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

CGI.unescapeHTML => Java::JavaLang::ArrayIndexOutOfBoundsException #4605

Closed
mmmries opened this issue May 11, 2017 · 4 comments
Closed

CGI.unescapeHTML => Java::JavaLang::ArrayIndexOutOfBoundsException #4605

mmmries opened this issue May 11, 2017 · 4 comments
Milestone
JRuby 9.1.9.0

Comments

@mmmries
Copy link

mmmries commented May 11, 2017

Related to #4556

Environment

  • jruby 9.1.7.0 (2.3.1) 2017-01-11 68056ae Java HotSpot(TM) 64-Bit Server VM 25.60-b23 on 1.8.0_60-b27 +jit [darwin-x86_64]
  • MacOS Sierra

Expected Behavior

jruby-9.1.7.0 :008 > ::CGI.unescapeHTML(::JSON.parse("{\"str\":\"&#8220GOOD LOVIN&#82\"}")["str"])
=> "&#8220GOOD LOVIN&#82"

Actual Behavior

jruby-9.1.7.0 :008 > ::CGI.unescapeHTML(::JSON.parse("{\"str\":\"&#8220GOOD LOVIN&#82\"}")["str"])
Java::JavaLang::ArrayIndexOutOfBoundsException: 20
	from org.jruby.ext.cgi.escape.CGIEscape.optimized_unescape_html(CGIEscape.java:181)
	from org.jruby.ext.cgi.escape.CGIEscape.cgiesc_unescape_html(CGIEscape.java:372)
	from org.jruby.ext.cgi.escape.CGIEscape$INVOKER$s$1$0$cgiesc_unescape_html.call(CGIEscape$INVOKER$s$1$0$cgiesc_unescape_html.gen)
	from org.jruby.runtime.callsite.CachingCallSite.cacheAndCall(CachingCallSite.java:338)
	from org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:163)
	from org.jruby.ir.interpreter.InterpreterEngine.processCall(InterpreterEngine.java:315)
	from org.jruby.ir.interpreter.StartupInterpreterEngine.interpret(StartupInterpreterEngine.java:73)
	from org.jruby.ir.interpreter.Interpreter.INTERPRET_EVAL(Interpreter.java:122)
	from org.jruby.ir.interpreter.Interpreter.evalCommon(Interpreter.java:176)
	from org.jruby.ir.interpreter.Interpreter.evalWithBinding(Interpreter.java:200)
	from org.jruby.RubyKernel.evalCommon(RubyKernel.java:1033)
	from org.jruby.RubyKernel.eval19(RubyKernel.java:1000)
	from org.jruby.RubyKernel$INVOKER$s$0$3$eval19.call(RubyKernel$INVOKER$s$0$3$eval19.gen)
	from org.jruby.runtime.callsite.CachingCallSite.callBlock(CachingCallSite.java:77)
	from org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:83)
	from org.jruby.ir.instructions.CallBase.interpret(CallBase.java:428)
... 154 levels...
	from org.jruby.internal.runtime.methods.JavaMethod$JavaMethodOneOrNBlock.call(JavaMethod.java:383)
	from org.jruby.internal.runtime.methods.AliasMethod.call(AliasMethod.java:61)
	from org.jruby.runtime.callsite.CachingCallSite.cacheAndCall(CachingCallSite.java:338)
	from org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:163)
	from bin.rails.invokeOther8:require(bin/rails:8)
	from bin.rails.RUBY$script(bin/rails:8)
	from java.lang.invoke.MethodHandle.invokeWithArguments(MethodHandle.java:627)
	from org.jruby.ir.Compiler$1.load(Compiler.java:90)
	from org.jruby.Ruby.runScript(Ruby.java:823)
	from org.jruby.Ruby.runNormally(Ruby.java:742)
	from org.jruby.Ruby.runNormally(Ruby.java:760)
	from org.jruby.Ruby.runFromMain(Ruby.java:573)
	from org.jruby.Main.doRunFromMain(Main.java:417)
	from org.jruby.Main.internalRun(Main.java:305)
	from org.jruby.Main.run(Main.java:232)
	from org.jruby.Main.main(Main.java:204)

Similar to the last issue I reported around this, I can't trigger the invalid behavior with a simple copy-paste of the string into IRB, but if I parse it out of a JSON structure (or a protobuf message in my application) then it fails to unescape cleanly.
@enebo enebo added this to the JRuby 9.1.9.0 milestone May 11, 2017
@enebo
Copy link
Member

enebo commented May 11, 2017

Ok seems like when I audited after the last fix I missed the continue clause which change 'i' (thus requiring one more i < len check). Fix coming up.

@enebo enebo closed this as completed in c7dfb2d May 11, 2017
@mmmries
Copy link
Author

mmmries commented May 11, 2017

👏 That was some 🏎 speed coding @enebo. I'll pull it down and run in it through several million strings in our production system to see if I can find any other edge cases.

@enebo
Copy link
Member

enebo commented May 11, 2017

@mmmries great current plan for 9.1.9 is Monday so please beat this up :)

@mmmries
Copy link
Author

mmmries commented May 11, 2017

I've pushed 942k strings through this so far (include all of the edge cases I had noticed previously) and everything looks good. I'll let you know if I find anything else, but I think it's probably good to release with 9.1.9.0

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

No branches or pull requests
2 participants
@enebo @mmmries