Skip to content
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.

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

Ensure strings with only underscores are not processed as Integer #687

Conversation

zendesk-jmeade
Copy link
Contributor

A string similar to "0x____" should be treated as a string. Currently it is processed as an Integer.

This alters the regex specified by http://yaml.org/type/int.html to ensure at least one numerical symbol is present in the string before converting to Integer.

A string similar to "0x____" should be treated as a string.
Currently it is processed as an Integer.

This alters the regex specified by http://yaml.org/type/int.html
to ensure at least one numerical symbol is present in the string
before converting to Integer.
Copy link

@KJTsanaktsidis KJTsanaktsidis left a comment

Choose a reason for hiding this comment

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

❤️

I think the spec is also broken here. As you say, these regexes came from the spec.

---
foo: 0x_

Since the spec says that the regexes in question "may also be used for implicit tag resolution", that means that the plain-style 0x_ is expected to become an integer.... but... there's really no way to say what integer that's supposed to be 😅

The YAML 1.2 spec though actually deals with this issue already by not allowing _ in integer literals - https://yaml.org/spec/1.2.2/#1012-tag-resolution. So actually this is only a 1.1 issue. I don't know if there's a process for making errata for YAML 1.1.

I opened an issue about the spec here: yaml/yaml-spec#338

@KJTsanaktsidis
Copy link

cc @tenderlove @hsbt this change looks good to me 👍 but you're the owners of the gem so I'll leave merging & releasing up to you when you're happy to do it. Thanks!

@tenderlove tenderlove merged commit 786a8dd into ruby:master Sep 11, 2024
67 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants