Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
After upgrading core to Symfony 5.1 the following deprecation is triggered in Drupal\Tests\Component\Serialization\YamlTest:
Since symfony/yaml 5.1: Support for parsing numbers prefixed with 0 as octal numbers. They will be parsed as strings as of 6.0.
The root cause of this is the octal type in core/modules/config/tests/config_test/config/install/config_test.types.yml:
octal: 0775
In the YAML 1.1 spec octal numbers started with an 0 and then any number of digits between 0 and 7.
In YAML 1.2 this has been changed and octal numbers must start with 0o.
Symfony 5.1 now respects YAML 1.2 and has deprecated the older format, but the PHP PECL parser only seems to respect the older format - I can't get it to parse 0o755 for example.
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | 3199691-11.patch | 1.65 KB | catch |
| #11 | 3199691-interdiff.txt | 891 bytes | catch |
| #9 | interdiff.3199691.6-9.txt | 925 bytes | abhisekmazumdar |
| #9 | 3199691-9.patch | 799 bytes | abhisekmazumdar |
| #6 | 3199691.patch | 800 bytes | catch |
Comments
Comment #2
andypostPecl yaml is 1.1 spec that's why core stoped to promote it, there's a set of related issues
Comment #3
catchPECL YAML is based on libYAML, and libYAML is slowly moving towards YAML 1.2 support: https://github.com/yaml/libyaml/issues/20
However, it looks unlikely that it'll get there in time for Drupal 10.
Given this is literally only used in test content, could we remove the test coverage and open an issue to add it back once both implementations support octal again?
Comment #4
longwaveI think removing this - or commenting it out, with an extra comment as to why and a @todo to put it back - is probably the best solution. Parsing octal in YAML is surely a niche case, and as far as I can tell we will never deliberately write octal to YAML in any case, so reading existing hardcoded files is the only place this might be an issue. The only real world use case I know of for octal is as a readable way of storing POSIX file permissions (e.g. 0644 for rw-r--r--).
If we go to Symfony 5 then users who use the Symfony YAML reader will receive the warning. Users who use PECL YAML won't receive any warning, but it won't affect them until PECL upgrades anyway.
If we go straight to Symfony 6 the warning will be skipped, but we can't do anything anyway; if SF6 parses differently from PECL YAML and we support both parsers that is unfortunate but there doesn't seem to be much we can do about it.
Comment #5
andypostBtw libyaml already has release of 1.2 as 0.2.3 2020-04-11 so makes sense to check main distributions about shipped version. If pecl extension is not yet ready we can use ffi shim to lib
Comment #6
catchHere's a start on a comment, and created the follow-up so I could link to it #3205480: Drop PECL YAML library support in favor of only Symfony YAML.
Comment #7
longwaveThis looks good enough to me except for nits:
YAMl -> YAML
octals -> octal (this will satisfy cspell as well!)
Comment #8
longwaveComment #9
abhisekmazumdarUpdated the suggested changes by @longwave. Kindly review.
Comment #11
catchOK need to change the test too. Copied the comment over there as well.
Comment #12
longwaveI think this the best we can do for now.
Comment #13
alexpottThe problem with Synfony Yaml and Pecl yaml diverging is that we write all yaml with Symfony and only read with Pecl. So this is going to introduce some awkwardness if there are any octals in contrib... I wonder what will happen to some of the NumericCode's in http://grep.xnddx.ru/node/30888953 - it has stuff like;
Comment #14
longwaveAren't those wrong at the moment though? For example, Belize is 084 which isn't valid octal. This would seem to be a good argument for moving to the 0o prefix!
Comment #15
catchSeems like three things could happen:
1. Running a version of libYAML which updates the octal parsing to YAML 1.2 while running Drupal 9 - so it gets ahead of Symfony 4's. (this would also break the current test coverage).
2. Running an older version of libYAML with Drupal 10, which is behind Symfony 6.
3. Running a YAML 1.2 compliant version of libYAML with Drupal 10, so parity with Symfony 6.
Since PECL YAML only has minimal requirements for libYAML (and I'm not sure if it exposes the version in use), we could probably only do feature detection in a hook_requirements() or similar to flag any issues. However, I'm really not sure that we want a system status message for YAML octal parsing - and it's one which people are fairly unlikely to have control over. This seems like more of a discussion to have on #3161889: [META] Symfony 6 compatibility though.
Comment #17
SpokjeComment #18
catchBumping to critical since this blocks Symfony 6 adoption.
Comment #19
alexpottOne of the problems we have here is if we go from SF4 -> SF6 then Drupal project (custom, contrib, sites) never get the benefit of Symfony's deprecation of
0775<code> octal support - they have to go immediately to <code>0onotation. And if D9 is SF4 and D10 is SF6 then there is no way a project can support both if they are using octal notation. Yes this possibly has zero cases in known contrib projects but still the point is relevant for any other deprecations in SF5.Should we be adding a deprecation test to allow Sf5 testing and then somehow marking the octal test skipped on Sf6?
Comment #20
longwave@alexpott That is a wider question than just this issue, that is being discussed in #3196027: Contrib support solution needed for potential Symfony 4 to 6 upgrade, Symfony 5 only deprecations are not available in Symfony 4
Comment #21
catchWe have #3196027: Contrib support solution needed for potential Symfony 4 to 6 upgrade, Symfony 5 only deprecations are not available in Symfony 4 open to discuss the jump between Symfony 4 to 6 for contrib. I'm not sure what a test would gain us here - we can't test the deprecation on Symfony 4 or Symfony 6, and Symfony itself should be testing the Symfony 5 deprecation.
Comment #22
alexpottGreat to see that #3196027: Contrib support solution needed for potential Symfony 4 to 6 upgrade, Symfony 5 only deprecations are not available in Symfony 4 is open. As this case for octal being used seems v small and not really seen in contrib let's go forward here to make things simpler.
Committed 12c6bfe231 and pushed to 9.2.x. Thanks!
Comment #23
longwaveIt doesn't look like this actually got pushed to git.
Comment #25
alexpottDid now - thanks @longwave!