clj-commons / gloss

speaks in bytes, so you don't have to
Eclipse Public License 1.0
486 stars 57 forks source link

Using gloss.io/contiguous with a single non-zero position ByteBuffer loses data. #33

Open d-t-w opened 10 years ago

d-t-w commented 10 years ago

When using gloss.io/contiguous as a convenience for rolling a sequence of ByteBuffer into a single buffer I've found that:

then the output loses as many bytes from the end as the position is offset from the start.

e.g.

Two test buffers:

(def buff-a (gi/to-byte-buffer "some text "))
=> (var test/buff-a)
(def buff-b (gi/to-byte-buffer "more, then end!"))
=> (var test/buff-b)
buff-a
=> #<HeapByteBuffer java.nio.HeapByteBuffer[pos=0 lim=10 cap=10]>
buff-b
=> #<HeapByteBuffer java.nio.HeapByteBuffer[pos=0 lim=15 cap=15]>

Apply contiguous, result is as expected:

(gi/contiguous [buff-a buff-b])
=> #<HeapByteBuffer java.nio.HeapByteBuffer[pos=0 lim=25 cap=25]>
(gi/decode (gc/string :utf-8) *1)
=> "some text more, then end!"

Reset the buffers, set position on both, apply contiguous. Result is as expected:

(def buff-a (gi/to-byte-buffer "some text "))
=> (var test/buff-a)
(def buff-b (gi/to-byte-buffer "more, then end!"))
=> (var test/buff-b)
(.position buff-a 2)
=> #<HeapByteBuffer java.nio.HeapByteBuffer[pos=2 lim=10 cap=10]>
(.position buff-b 4)
=> #<HeapByteBuffer java.nio.HeapByteBuffer[pos=4 lim=15 cap=15]>
(gi/contiguous [buff-a buff-b])
=> #<HeapByteBuffer java.nio.HeapByteBuffer[pos=0 lim=19 cap=19]>
(gi/decode (gc/string :utf-8) *1)
=> "me text , then end!"

Reset buff-a, set position, apply contiguous. Result is two bytes short, missing from the end:

(def buff-a (gi/to-byte-buffer "some text"))
=> (var test/buff-a)
(.position buff-a 2)
=> #<HeapByteBuffer java.nio.HeapByteBuffer[pos=2 lim=9 cap=9]>
(gi/contiguous [buff-a])
=> #<HeapByteBuffer java.nio.HeapByteBuffer[pos=0 lim=5 cap=5]>
(gi/decode (gc/string :utf-8) *1)
=> "me te"

As long as there is a second buffer in the sequence, this isn't an issue:

(def buff-a (gi/to-byte-buffer "some text"))
=> (var test/buff-a)
(.position buff-a 2)
=> #<HeapByteBuffer java.nio.HeapByteBuffer[pos=2 lim=9 cap=9]>
(gi/contiguous [buff-a (gi/to-byte-buffer "")])
=> #<HeapByteBuffer java.nio.HeapByteBuffer[pos=0 lim=7 cap=7]>
(gi/decode (gc/string :utf-8) *1)
=> "me text"

TL:DR; If using gloss.io/contiguous with non-zero position ByteBuffer sequences you might lose data.

When I get a moment I'll see if I can figure out why and raise a PR.

danielcompton commented 10 years ago

I've looked into it and I think the problem is in https://github.com/ztellman/gloss/blob/0.2.2/src/gloss/data/bytes/core.clj#L236. On a SingleBufferSequence, the limit of the ByteBuffer is set to the minimum of the byte-count or n. I think it should be something like the limit being set to the position + byte-count or n. I'll look into this further to see if I can make a patch.