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

CGI.escapeHTML crashes on invalid byte sequence where CRuby does not #6093

Closed
PragTob opened this issue Feb 22, 2020 · 2 comments · Fixed by #6097
Closed

CGI.escapeHTML crashes on invalid byte sequence where CRuby does not #6093

PragTob opened this issue Feb 22, 2020 · 2 comments · Fixed by #6097

Comments

@PragTob
Copy link

@PragTob PragTob commented Feb 22, 2020

Environment Information

tobi@speedy:~/github/simplecov/tmp/aruba/project(convert-utf8)$ jruby -v
jruby 9.2.10.0 (2.5.7) 2020-02-18 fffffff OpenJDK 64-Bit Server VM 12.0.1+12 on 12.0.1+12 +jit [linux-x86_64]
tobi@speedy:~/github/simplecov/tmp/aruba/project(convert-utf8)$ uname -a
Linux speedy 5.3.0-40-generic #32~18.04.1-Ubuntu SMP Mon Feb 3 14:05:59 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux

Expected Behavior

tobi@speedy:~/github/simplecov/tmp/aruba/project(convert-utf8)$ ruby -v
ruby 2.7.0p0 (2019-12-25 revision 647ee6f091) [x86_64-linux]
tobi@speedy:~/github/simplecov/tmp/aruba/project(convert-utf8)$ irb
irb(main):001:0> require 'cgi'
=> true
irb(main):002:0> CGI.escapeHTML "\xA4\xAA\xA4Ϥ褦"
=> "\xA4\xAA\xA4Ϥ褦"
irb(main):003:0> 

Actual Behavior

tobi@speedy:~/github/simplecov/tmp/aruba/project(convert-utf8)$ ruby -v
jruby 9.2.10.0 (2.5.7) 2020-02-18 fffffff OpenJDK 64-Bit Server VM 12.0.1+12 on 12.0.1+12 +jit [linux-x86_64]
tobi@speedy:~/github/simplecov/tmp/aruba/project(convert-utf8)$ irb
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by com.headius.backport9.modules.Modules to method sun.nio.ch.NativeThread.signal(long)
WARNING: Please consider reporting this to the maintainers of com.headius.backport9.modules.Modules
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release
irb(main):001:0> require 'cgi'
=> true
irb(main):002:0> CGI.escapeHTML "\xA4\xAA\xA4Ϥ褦"
Traceback (most recent call last):
        8: from /home/tobi/.asdf/installs/ruby/jruby-9.2.10.0/bin/irb:13:in `<main>'
        7: from org/jruby/RubyKernel.java:1189:in `catch'
        6: from org/jruby/RubyKernel.java:1189:in `catch'
        5: from org/jruby/RubyKernel.java:1442:in `loop'
        4: from org/jruby/RubyKernel.java:1048:in `eval'
        3: from (irb):2:in `evaluate'
        2: from /home/tobi/.asdf/installs/ruby/jruby-9.2.10.0/lib/ruby/stdlib/cgi/util.rb:54:in `escapeHTML'
        1: from org/jruby/RubyString.java:3132:in `gsub'
ArgumentError (invalid byte sequence in UTF-8)

I didn't check against master due to #6092 and it being late. However, this popped up in simplecov and despite that the build says green (woops?) it errors on our jruby-head build there: https://github.com/colszowka/simplecov/pull/866/checks?check_run_id=462430018

Context/why the invalid byte: This is おはよう but read from an EUC-JP encoded file. The source file isn't declared as such so even CRuby won't execute it, however in simplecov you can track even not loaded files leading to this potential error.

Workaround
I don't need a fix. I'll just need to keep a workaround which can be done through something like .encode('UTF-8', invalid: :replace, undef: :replace):

irb(main):003:0> CGI.escapeHTML "\xA4\xAA\xA4Ϥ褦".encode('UTF-8', invalid: :replace, undef: :replace)
=> "���Ϥ褦"

Thanks and have a wonderful day y'all! 💚

edit: SimpleCov PR is simplecov-ruby/simplecov#866

@ahorek
Copy link
Contributor

@ahorek ahorek commented Feb 25, 2020

this is actually a bug in the CRuby's stdlib, but CRuby use a native extension to do the job (in most cases)

the reason why JRuby dosn't use a CGI extension #5949
+ a quick workaround / fix? #6097

@headius
Copy link
Member

@headius headius commented Mar 2, 2020

@ahorek Thanks! Merged your PR for 9.2.11.

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

Successfully merging a pull request may close this issue.

3 participants