Bug with body reader/Buf.Utf8 in finch 0.9.2

[matteosb]

This is a bug that hit us in a unit test when upgrading to finch 0.9.2. It though it may not have an impact on real usage:

When setting the com.twitter.finagle.http.Request body to a com.twitter.io.Buf.Utf8 the body request reader adds an extra byte at the end of the byte array. This only appears to be the case with certain string, though JSON seems to be affected. The problem looks to be an off-by-one error and is not present when using com.twitter.io.Buf.ByteArray.

I added a simple project with an example of the bug here:
https://github.com/matteobanerjee/finch-buffer-bug

I might try to resolve this in PR myself if I have time this week.

[vkostyukov]

Thanks @matteobanerjee! This is indeed a good bug report. There was a recent change for body readers, although I'm not sure how it my cause this.

[vkostyukov]

It turns out to be a Finagle bug:

scala> import com.twitter.io.{Charsets, Buf}; import com.twitter.finagle.netty3.ChannelBufferBuf
import com.twitter.io.{Charsets, Buf}
import com.twitter.finagle.netty3.ChannelBufferBuf

scala> val foo = "{\"foos\": [1, 2]}"
foo: String = {"foos": [1, 2]}

scala> val buf1 = Buf.Utf8(foo)
buf1: com.twitter.io.Buf = ByteBuffer(16)

scala> val buf2 = Buf.ByteArray.Owned(foo.getBytes(Charsets.Utf8))
buf2: com.twitter.io.Buf = ByteArray(16)

scala> val cb = ChannelBufferBuf.Owned.extract(buf1)
cb: org.jboss.netty.buffer.ChannelBuffer = TruncatedChannelBuffer(ridx=0, widx=16, cap=16)

scala> val ba = Buf.ByteArray.Shared.extract(buf1)
ba: Array[Byte] = Array(123, 34, 102, 111, 111, 115, 34, 58, 32, 91, 49, 44, 32, 50, 93, 125)

scala> cb.array()
res0: Array[Byte] = Array(123, 34, 102, 111, 111, 115, 34, 58, 32, 91, 49, 44, 32, 50, 93, 125, 0)

I'm going to file an internal issue so we could fix it. Although this doesn't mean we shouldn't apply a workaround in Finch until the next version of Finagle is released.

[vkostyukov]

I'm wondering if we could do something like w/o allocating much. Let me run some benchamrks.

buffer.slice(0, n).array()

[matteosb]

Great, thanks!

[vkostyukov]

So I put together a fix and I'd say that this bug is completely Finch's fault given that ButeBuffer.array() doesn't guarantee to be the exact version user sees from different API. So, the fix I proposed fixes that.

[vkostyukov]

Fixed by #485. Thanks @matteobanerjee for reporting! You can grab this fix as part of 0.9.3-SNASPSHOT.