Skip to content

[YAML] Parsing octal numbers: unexpected value conversion and PHP 7.4 notice #34807

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
cebe opened this issue Dec 4, 2019 · 4 comments · Fixed by #34813
Closed

[YAML] Parsing octal numbers: unexpected value conversion and PHP 7.4 notice #34807

cebe opened this issue Dec 4, 2019 · 4 comments · Fixed by #34813
Labels
DX DX = Developer eXperience (anything that improves the experience of using Symfony) Yaml

Comments

@cebe
Copy link

cebe commented Dec 4, 2019

Symfony version(s) affected: probably all (tried 4.3 and 4.4)

Description

Parsing YAML values that start with 0 may result in a deprecation warning in PHP 7.4:

Invalid characters passed for attempted conversion, these have been ignored

Related note in PHP 7.4 migration guide: https://www.php.net/manual/en/migration74.deprecated.php#migration74.deprecated.core.invalid-base-characters

Also unsure if the parsing happens correctly according to the YAML spec.

Having something like OwnerId: 0123456789 in YAML, it is being interpreted as an octal number and results in [OwnerId] => 342391 when parsed.

How to reproduce

<?php

require __DIR__ . '/vendor/autoload.php';

$yaml = \Symfony\Component\Yaml\Yaml::parse(<<<YAML
OwnerId: 0123456789
YAML
);

print_r($yaml);

results in:

Array
(
    [OwnerId] => 342391
)

342391 decimal, when converted back to ocal however is 1234567 in octal, not 0123456789, which is obvious, because 0123456789 is not a valid octal number.

Possible Solution

As far as I see, the yaml spec is not clear about how to handle this case. Many users might be unaware of this conversion and actually expect a string '0123456789' as the parseing result. So we should probably add a check and only convert octal numbers that are valid.

I also found that YAML 1.1 is showing octal number examples like 013 but YAML 1.2 they are 0o14...

Additional context

related code:

symfony/src/Symfony/Component/Yaml/Inline.php

Lines 625 to 634 in c732122

case ctype_digit($scalar):
$raw = $scalar;
$cast = (int) $scalar;
return '0' == $scalar[0] ? octdec($scalar) : (((string) $raw == (string) $cast) ? $cast : $raw);
case '-' === $scalar[0] && ctype_digit(substr($scalar, 1)):
$raw = $scalar;
$cast = (int) $scalar;
return '0' == $scalar[1] ? octdec($scalar) : (((string) $raw === (string) $cast) ? $cast : $raw);

@xabbuh xabbuh added DX DX = Developer eXperience (anything that improves the experience of using Symfony) Yaml labels Dec 4, 2019
cebe added a commit to cebe/php-openapi that referenced this issue Dec 4, 2019
@cebe
ignore deprecation warning in PHP 7.4
b4a47f4 
symfony/symfony#34807
@xabbuh xabbuh mentioned this issue Dec 4, 2019
Merged
@stof
Copy link
Member

stof commented Dec 4, 2019

According to https://yaml.org/spec/1.2/spec.html#id2805071, only values matching the regex 0o[0-7]+ should be parsed as octal. So it looks like our parser is not compliant there.

@xabbuh xabbuh mentioned this issue Dec 4, 2019
Merged
@stof
Copy link
Member

stof commented Dec 4, 2019

Yaml 1.1 has a similar restriction about 0-7 in the octal regex (see https://yaml.org/type/int.html), so our parser is not compliant either for 1.1 here. Fixing it will avoid the PHP 7.4 deprecation

@xabbuh xabbuh mentioned this issue May 4, 2020
Merged
@xabbuh
Copy link
Member

xabbuh commented May 4, 2020

PHP 7.4 compat will be fixed by #36690 on the 3.4 branch.

@xabbuh
Copy link
Member

xabbuh commented May 4, 2020

#34813 now deprecates the 1.1 notation and adds support for the 1.2 notation.

fabpot added a commit that referenced this issue May 5, 2020
@fabpot
bug #36690 [Yaml] prevent notice for invalid octal numbers on PHP 7.4…
6340e87 
… (xabbuh)

This PR was merged into the 3.4 branch.

Discussion
----------

[Yaml] prevent notice for invalid octal numbers on PHP 7.4

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #34807
| License       | MIT
| Doc PR        |

Commits
-------

92bc19f prevent notice for invalid octal numbers on PHP 7.4
@fabpot fabpot closed this as completed in 09d78cf May 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
DX DX = Developer eXperience (anything that improves the experience of using Symfony) Yaml
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Yaml] support YAML 1.2 octal notation, deprecate YAML 1.1 one xabbuh/symfony
3 participants
@cebe @stof @xabbuh