Multipart data corrupt under jRuby 1.7.*

[digitalextremist]

After struggling with celluloid/reel#40 for some time, I switched to rubinius to test and the problem disappears. @tmm1 stepped into that issue on Reel, and the details are there. Figured I would refocus the issue to http_parser.rb specifically since it is not Reel related.

[digitalextremist]

gulp under rubinius/rubinius, mongodb/mongo performance is down 300% to 1000% ... save me, have mercy!

[headius]

Do you have an example case? We can get this fixed up.

[digitalextremist]

@headius, @tmm1 left details on celluloid/reel#40 of the failing tests on this java port. If there is no time on your end to mend this essential piece of my stack, please enlighten me on what the break points are and I will slowly repair the port somehow and return to jRuby compatibility. But if you can find time or guide a java coder around you, I would be very, very grateful.

[digitalextremist]

Another instance in the wild: https://groups.google.com/d/msg/celluloid-ruby/rjrjtKcdc_E/Ksmx3h6YqO4J

[doxavore]

This one isn't so easy for my poor Java skills.

As near as I can tell, all of the tests that fail on my machine under JRuby 1.7.2 are because we're trying to access requestUrl at https://github.com/tmm1/http_parser.rb/blob/master/ext/ruby_http_parser/org/ruby_http_parser/RubyHttpParser.java#L110 in settings.on_url before the callback for settings.on_message_begin are ever called at https://github.com/tmm1/http_parser.rb/blob/master/ext/ruby_http_parser/org/ruby_http_parser/RubyHttpParser.java#L188

If we can get over that issue, the upstream may have already fixed the underlying issue causing our request bodies to become malformed... Anyone? :-/

[doxavore]

Digging in a little more, this commit is the one that changed it so that the settings.on_message_begin callback isn't always being called before our on_url method: http-parser/http-parser.java@79b495a

I am wholly out of my league now as I have no idea how the original http_parser is intended to work. If I copy the following initialization code from on_message_begin into RubyHttpParser.java's init method, I only get 7 failures in the 68 tests (most of which it seems to be validating that we have the null state that I blew away with init). Is this a bug in the upstream, perhaps? (I think this bug has made its rounds now...)

I gisted the original and changes at https://gist.github.com/doxavore/5446909/revisions

[digitalextremist]

^^ @headius

[digitalextremist]

@tmm1 there is a test case demonstrating this, as requested by @headius:

digitalextremist/http_parser_multipart_corruption

[digitalextremist]

Any word on how this is going @enebo or @headius? I really appreciate you guys helping through this issue.

[enebo]

@digitalextremist Sorry I had caught a cold and was out of action on thursday/friday. I am feeling better today :)

[digitalextremist]

Glad to hear you're feeling better! Thanks for letting me know.

[digitalextremist]

@enebo you've said on Twitter you found the issue to be encoding oriented, and that http_parser.rb is not encoding aware.

What is your recommendation? /cc: @tmm1 @headius

[enebo]

I will write something up this afternoon. As it stands I will mention a couple of the problems I see (hopefully this is a nice qualitative description):

1. new String(bytes) will create a string using the default charset. If you are reading from a multipart section of an encoding which is not your locale charset of the JVM (file.encoding) then this would corrupt the string. This however is just an aside and not the main problem.
2. runtime.newString(new String(bytes)) will create a Ruby String which is US-ASCII in 1.9 mode. In 1.8 mode all strings were buckets of bits so this was appropriate. In 1.9 mode we need to know what encoding those bits are and construct a string with the appropriate encoding object (part of what I plan on writing a recommendation for).

I think 2 is causing your main issue but since it uses new String(bytes) (1) it is asking for trouble since if you ever run this on a machine with a different locale encoding from what is in multipart envelopes it will get corrupted.