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

Grammar appears to allow character forbidden by docs #1013

Closed
Lucretiel opened this issue Feb 10, 2024 · 4 comments
Closed

Grammar appears to allow character forbidden by docs #1013

Lucretiel opened this issue Feb 10, 2024 · 4 comments
Labels

Comments

@Lucretiel
Copy link

toml/toml.abnf

Line 39 in 8eae5e1

non-eol = %x09 / %x20-7F / non-ascii

The grammar for valid comment characters includes the class U+0020 - U+007F. The docs, however, forbid the use of U+007F in comments.

@eksortso
Copy link
Contributor

eksortso commented Feb 11, 2024

You're comparing different versions of the standard.

If you look at the current version:

and the current ABNF:

you will see that %7f is now allowed in comments.

This was a change that we made explicit in #924. It could be reverted if we merged #996, which I'm personally opposed to doing. So keep an eye out for this. By the time v1.1.0rc1 is released, this matter will be resolved, one way or another.

@Lucretiel
Copy link
Author

Am I? I'm comparing the specification 1.0.0:

Control characters other than tab (U+0000 to U+0008, U+000A to U+001F, U+007F) are not permitted in comments.

To the grammar tagged 1.0.0

toml/toml.abnf

Line 39 in 8eae5e1

non-eol = %x09 / %x20-7F / non-ascii

If it's going to be fixed in a later version then it's not a big deal, but it does seem to be a real mistake in one of the two documents.

@ChristianSi
Copy link
Contributor

ChristianSi commented Feb 16, 2024

Indeed! I'm pretty sure the intent was to forbid "Control characters other than tab", like the written spec says, so the ABNF is wrong in allowing U+007F.

But in any case it's a bug in one of the spec documents and the next version must fix it. So it's good that the current mainline already fixed it by allowing nearly all control chars.

@pradyunsg
Copy link
Member

1.0 was fixed in #812. We can discuss reverting #924 in the above PR.

Closing this out since that has been changed on main and will be resolved once 1.1.0 is out "soon". :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants