Skip to content

fix: parse YAML 1.2 octal scalars (0o prefix) as integers#1451

Merged
SGSSGene merged 1 commit into
jbeder:masterfrom
raphaelroshan:fix/1251-octal-scalars
Jul 2, 2026
Merged

fix: parse YAML 1.2 octal scalars (0o prefix) as integers#1451
SGSSGene merged 1 commit into
jbeder:masterfrom
raphaelroshan:fix/1251-octal-scalars

Conversation

@raphaelroshan

@raphaelroshan raphaelroshan commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Fixes #1251.

YAML 1.2 writes octal as 0o17, but as<int>() failed on it because std::stringstream's octal parsing only understands the C-style 0 prefix.

Normalize 0o/0O to a leading 0 before the stream conversion, so both the 1.1 (0123) and 1.2 (0o123) spellings decode. Hex and decimal are unchanged. As per the discussion in the issue, adding a function NormalizeOctalPrefix that should help to support backward compatibility. Added a test covering octal, signed/unsigned, hex and decimal.

Do let me know if i misinterpreted the behavior required!

@raphaelroshan raphaelroshan force-pushed the fix/1251-octal-scalars branch from 3331445 to 2ca3378 Compare June 23, 2026 15:44
@raphaelroshan raphaelroshan marked this pull request as ready for review June 23, 2026 16:30
@SGSSGene

Copy link
Copy Markdown
Collaborator

@raphaelroshan thank you so much for tackling this issue!

Comments:

  1. Could you change the signature to inline std::string NormalizeOctalPrefix(std::string s) I believe this fits the style better.
  2. I am not sure if you shuld additionally check that all characters are digits. Otherwise it will accept stuff like 0oxff and convert it to 0xff.
  3. Sadly the yaml spec is offline, but ane statements like -0o12 or +0o12 even valid in yaml? (this needs confirmation)

@SGSSGene SGSSGene left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this PR, see comment above.

@raphaelroshan raphaelroshan force-pushed the fix/1251-octal-scalars branch from 2ca3378 to f7b1554 Compare June 25, 2026 15:03
@raphaelroshan raphaelroshan force-pushed the fix/1251-octal-scalars branch from f7b1554 to 79d9250 Compare June 25, 2026 15:38
@raphaelroshan

Copy link
Copy Markdown
Contributor Author

Thank you @SGSSGene for the review, have updated the signature, added a check to ensure its an octal digit and based on yaml 1.2 - leading +/- signs arent valid so did not add that in the regex check

@SGSSGene SGSSGene left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! looks great

@SGSSGene SGSSGene merged commit 847020a into jbeder:master Jul 2, 2026
46 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

YAML octal scalars not converted to ints

2 participants