Repository navigation
Ruby 2.1/2.2/2.3 yaml parsing anomaly for comma separated integers #273
Description
A string of digits separated by comma (without quotes) is evaluated as integer. Is it intentional?
irb(main):001:0> Psych.load("key: 123,456")
=> {"key"=>123456}
irb(main):002:0> Psych.load("key: 123456,7890")
=> {"key"=>1234567890}
I understand that the first example can be seen as a number in American notation but the 2nd example is not following any notation/standard that I'm aware of. I was expecting to have both of the values evaluated as string.
With the quotes it works as expected:
irb(main):003:0> Psych.load("key: \"123,456\"")
=> {"key"=>"123,456"}
irb(main):004:0> Psych.load("key: \"123456,7890\"")
=> {"key"=>"123456,7890"}
Thanks
Activity
tenderlove commentedon Mar 7, 2017
Apparently Japan and China will separate at four digits, but to be honest Psych internals don't care, they just remove all commas and cast to a number. The only reason for this is legacy support for Syck documents. I agree that both values should evaluate to strings, but I'm not sure how to do that and support old formats.
Revert loading back to the use-YAML way..
Jell commentedon Nov 4, 2020
I've hit this issue today after trying to parse some YAML output by another tool (kustomize build in my case, but not that relevant). I think as YAML becomes more and more ubiquitous it might be a good idea to provide an option for "strict" parsing according to the standard?
It seems to me that we use those regexp to decide whether or not to parse a Scalar as a Float or Integer:
psych/lib/psych/scalar_scanner.rb
Lines 11 to 20 in 9f8c365
I did this patch on my side as a workaround, which solved the issue I had:
(basically replacing the regexp with a subset of the standard one from from http://yaml.org/type/int.html)
Would it be possible to have say two constants:
And chose one or the other based on a flag to keep backward compatibility but offer a stricter/more standard alternative? I can offer to work on a patch if that can help.
sethboyles commentedon Apr 13, 2021
We would love to see this fixed as well. The current parsing logic does not conform to the YAML specification, so we believe something like @Jell's proposed solution should be considered. Would the maintainers (@tenderlove) of Psych be amenable to a PR implementing the above solution or similar?
sethboyles commentedon Aug 17, 2021
@tenderlove Do you have a stance on whether or not you would accept a PR that adds a flag to change the behavior to something stricter? We are willing to work on this, but don't want to get started if you don't think it would be worthwhile.
tenderlove commentedon Aug 19, 2021
Yes, I'm definitely open to it. There was a PR submitted here for strict hash keys. I made a commit here that demonstrates how to accomplish it.
Maybe this commit should just be extended to be "strict parsing" or something (rather than just specific to hash keys).
sethboyles commentedon Jan 14, 2022
@tenderlove I created a PR that implements this (#537). Let me know what you think!
hsbt commentedon Oct 20, 2022
#537 fixed this issue.