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

YAML file fails to load if JVM character set is MS932 (common on Windows) #995

Merged
merged 2 commits into from Sep 9, 2013

Conversation

Projects
None yet
3 participants
@yousuketto
Copy link
Contributor

yousuketto commented Sep 9, 2013

I fail to load yaml file in windows7.

"InputStreamReader" is using "Charset.defaultCharset"
https://github.com/jruby/jruby/blob/master/core/src/main/java/org/jruby/ext/psych/PsychParser.java#L150
I expect that this is causing the problem.

env

c:\workspace\yaml171>jruby -v
jruby 1.7.5.dev (1.9.3p392) 2013-09-09 f87088b on Java HotSpot(TM) Client VM 1.7.0_17-b02 [Windows 7-x86]
c:\workspace\yaml171>jruby -e "import java.nio.charset.Charset; p Charset.defaultCharset()"
#<Java::SunNioCsExt::MS932:0x181af25>

case

#sample.rb

# -*- coding: utf-8 -*-
require 'yaml'
path = File.expand_path "../sample.yaml", __FILE__
p YAML.load_file(path)
# sample.yaml
ja:
  hoge: "テストほげ"
c:\workspace\yaml171>jruby sample.rb
Psych::SyntaxError: (c:/workspace/yaml171/sample.yaml): found unexpected end of stream while scanning a quoted scalar at line 3 column 1
         parse at org/jruby/ext/psych/PsychParser.java:212
  parse_stream at C:/workspace/sandbox/jruby/lib/ruby/shared/psych.rb:205
         parse at C:/workspace/sandbox/jruby/lib/ruby/shared/psych.rb:153
          load at C:/workspace/sandbox/jruby/lib/ruby/shared/psych.rb:129
     load_file at C:/workspace/sandbox/jruby/lib/ruby/shared/psych.rb:299
          open at org/jruby/RubyIO.java:1185
     load_file at C:/workspace/sandbox/jruby/lib/ruby/shared/psych.rb:299
        (root) at sample.rb:4
@BanzaiMan

This comment has been minimized.

Copy link
Member

BanzaiMan commented Sep 9, 2013

Please remove white space changes. Also, the commit subject doesn't seem quite right.

The issue appears to be the default character set on Windows, MS932. To reproduce this, I had to change the default character set on OS X:

$ jruby -J-Dfile.encoding=MS932 sample.rb
Psych::SyntaxError: (/Users/asari/Development/src/jruby/tmp/gh-995/sample.yaml): found unexpected end of stream while scanning a quoted scalar at line 4 column 1
         parse at org/jruby/ext/psych/PsychParser.java:212
  parse_stream at /Users/asari/Development/src/jruby/lib/ruby/shared/psych.rb:205
         parse at /Users/asari/Development/src/jruby/lib/ruby/shared/psych.rb:153
          load at /Users/asari/Development/src/jruby/lib/ruby/shared/psych.rb:129
     load_file at /Users/asari/Development/src/jruby/lib/ruby/shared/psych.rb:299
          open at org/jruby/RubyIO.java:1185
     load_file at /Users/asari/Development/src/jruby/lib/ruby/shared/psych.rb:299
        (root) at sample.rb:4

If this is the real culprit, is it sufficient to just check if yaml is an instance of RubyIO?

@yousuketto

This comment has been minimized.

Copy link
Contributor Author

yousuketto commented Sep 9, 2013

Thanks for your quick reply. Thank you for pointing it out.

I think that does not use encode of RubyIO to read Yaml is a problem.
parse_file method set 'r:bom|utf-8'.
https://github.com/jruby/jruby/blob/master/lib/ruby/shared/psych.rb#L163

I have no confidence that is enough RubyIO.
but, I think it's enough in RubyIO.

BanzaiMan added a commit that referenced this pull request Sep 9, 2013

Merge pull request #995 from yousuketto/fail_to_load_yaml_file
YAML file fails to load if JVM character set is MS932 (common on Windows)

@jrubyci jrubyci merged commit f0c56d6 into jruby:master Sep 9, 2013

1 check failed

default The Travis CI build could not complete due to an error
Details
@yousuketto

This comment has been minimized.

Copy link
Contributor Author

yousuketto commented Sep 10, 2013

Thank you.
Sorry,i am not good at english.

@BanzaiMan

This comment has been minimized.

Copy link
Member

BanzaiMan commented Sep 10, 2013

@yousuketto 気になさらないで下さい。これからも PR 宜しくお願いします。

@yousuketto yousuketto deleted the yousuketto:fail_to_load_yaml_file branch Sep 13, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.