readpartial must not return more data than requested

[headius]

The contract of readpartial is that it will return up to the amount requested and no more. However the implementation of readpartial in WEBrick's HTTPRequest does not honor this contract:

webrick/lib/webrick/httprequest.rb

Lines 276 to 284 in 6b6990e

	# for IO.copy_stream. Note: we may return a larger string than +size+
	# here; but IO.copy_stream does not care.
	def readpartial(size, buf = ''.b) # :nodoc
	res = @body_tmp.shift or raise EOFError, 'end of file reached'
	buf.replace(res)
	res.clear
	@body_rd.resume # get more chunks
	buf
	end

The assumption described here may hold in CRuby, but in JRuby's implementation of copy_stream we may use an intermediate buffer to hold data on its way, and if readpartial returns more data than requested we will overflow that buffer. I will be fixing JRuby to raise a more Ruby-friendly error, but it's still going to be an error if we readpartial N bytes and get N+1.

https://github.com/jruby/jruby/blob/a612f900243926e45a3d3465bc8ae5d9e5258cce/core/src/main/java/org/jruby/RubyIO.java#L4433-L4454

This is the root cause of a failure in test_big_bodies from test_httpproxy.rb as described in jruby/jruby#6246. The copy_stream call deep inside the proxy test fails due to this buffer overflow, which causes the request's piped content to be prematurely closed and the proxy test to error out while trying to write request data.

webrick/test/webrick/test_httpproxy.rb

Line 189 in ec5372f

th = Thread.new { nr.times { wr.write(rand_str) }; wr.close }

[headius]

The relevant bit of the JVM exception trace from JRuby is shown below:

java.nio.BufferOverflowException
	at java.nio.HeapByteBuffer.put(HeapByteBuffer.java:189)
	at org.jruby.util.IOChannel.read(IOChannel.java:93)
	at org.jruby.util.IOChannel$IOReadableByteChannel.read(IOChannel.java:151)
	at org.jruby.RubyIO.transfer(RubyIO.java:4454)
	at org.jruby.RubyIO.copy_stream(RubyIO.java:4345)
	at org.jruby.RubyIO$INVOKER$s$0$2$copy_stream.call(RubyIO$INVOKER$s$0$2$copy_stream.gen)
	at org.jruby.internal.runtime.methods.JavaMethod$JavaMethodN.call(JavaMethod.java:819)
	at org.jruby.internal.runtime.methods.DynamicMethod.call(DynamicMethod.java:211)
	at org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:203)
	at Users.headius.projects.jruby.lib.ruby.stdlib.net.http.generic_request.invokeOther21:copy_stream(/Users/headius/projects/jruby/lib/ruby/stdlib/net/http/generic_request.rb:212)
...

Because readpartial returned more data than requested, we are forced to raise an error from our IOReadableByteChannel (which wraps an IO-like object with a JVM IO Channel) since there's simply nowhere in the buffer to put it.

https://github.com/jruby/jruby/blob/30e9fa45cc13e97767c64d18410af8547c10ff5d/core/src/main/java/org/jruby/util/IOChannel.java#L86-L95

[headius]

FWIW the new error that JRuby will raise in this case:

(IOError) error calling WEBrick::HTTPRequest#readpartial: requested 8192 but received 65536

This error ends up getting swallowed by net/http plumbing, but the result is the same.

[headius]

The change in #45 does appear to fix this issue.