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.

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

YAML parses okay on CRuby, raises exception on JRuby #4847

Closed
0x1eef opened this issue Nov 10, 2017 · 8 comments
Closed

YAML parses okay on CRuby, raises exception on JRuby #4847

0x1eef opened this issue Nov 10, 2017 · 8 comments
Labels
stdlib
Milestone
Won't Fix

Comments

@0x1eef
Copy link

0x1eef commented Nov 10, 2017

testcase.yml

---
  !!seq [
    !!str "https://www.youtube.com/watch?v=DzpKasJJtRs",
    !!str "2Pac - Dont Care What Ya\'ll Think Remix Music Video 2017",
  ]

JRuby

$ jruby -v -ryaml -e "YAML.load_file('./testcase.yml')"
jruby 9.1.13.0 (2.3.3) 2017-09-06 8e1c115 Java HotSpot(TM) 64-Bit Server VM 25.144-b01 on 1.8.0_144-b01 +jit [darwin-x86_64]
Psych::SyntaxError: (./testcase.yml): found unknown escape character '(39) while scanning a double-quoted scalar at line 4 column 37

CRuby

ruby -v -ryaml -e "YAML.load_file('./testcase.yml')"
ruby 2.4.2p198 (2017-09-14 revision 59899) [x86_64-darwin15]
@headius
Copy link
Member

headius commented Nov 10, 2017

Huh, this is pretty simple YAML too. I've confirmed it's valid YAML (the parser we use can be finicky) so it definitely should be ok.

The only escape character is \\ which is ASCII 39, so the error actually just doesn't like that the single quote is escaped, perhaps because it doesn't need to be in a double-quoted string. This may be a grey area in YAML but more and more I suspect the library we use.

@headius headius added this to the JRuby 9.1.15.0 milestone Nov 10, 2017
@headius
Copy link
Member

headius commented Nov 10, 2017

Well I've confirmed it's not fixed in the latest SnakeYAML (the library we use). I'll bring it to them.

@headius
Copy link
Member

headius commented Nov 10, 2017

So it turns out I've dealt with this before, but never followed up to get it fixed in SnakeYAML 😞

https://bitbucket.org/asomov/snakeyaml/issues/350/failure-to-parse-escaped-single-quote-in-a

This was originally reported in #2199 where @asomov pointed out that the single quote is not a valid character in the YAML spec, and I confirmed that neither YAML 1.1 nor 1.2 list it as valid.

http://www.yaml.org/spec/1.2/spec.html#id2776092

I do not know why MRI passes this. I had not noticed the online YAML tester I used (http://www.yamllint.com/) is "optimized for ruby" so that explains in why it said it was ok. Another tester, based on Python, rejects it (http://yaml-online-parser.appspot.com/).

So, I think maybe we need to pull in @hsbt and @tenderlove. This does not appear to be valid YAML according to any spec, and yet MRI accepts it. Why?

@headius
Copy link
Member

headius commented Nov 10, 2017

I will close the older bug in favor of this new active bug.

@headius headius mentioned this issue Nov 10, 2017
Closed
@headius headius added the stdlib label Nov 10, 2017
@headius headius mentioned this issue Nov 29, 2017
Closed
@headius
Copy link
Member

headius commented Nov 29, 2017

I've filed an additional bug with libyaml. Hopefully I won't have to also file a bug with Psych to get some movement on this! (cc @hsbt, @tenderlove)

yaml/libyaml#68

@headius
Copy link
Member

headius commented Nov 29, 2017

If we fix this at all, it won't be in 9.1.15.0.

@headius
Copy link
Member

headius commented Dec 5, 2017

A PR has been created for libyaml (which seems very likely to be merged), which means this is going to start percolating back into MRI.

Given that fact I'm going to close this as Won't Fix and let MRI know what's happening.

@headius headius closed this as completed Dec 5, 2017
@headius headius modified the milestones: JRuby 9.1.16.0, Won't Fix Dec 5, 2017
@0x1eef
Copy link
Author

0x1eef commented Dec 5, 2017

cheers, great @headius

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
stdlib
Projects
None yet
Milestone
Won't Fix
Development

No branches or pull requests
2 participants
@headius @0x1eef