BinaryCodec.readBytesOrFewer fails if no data is requested AND no date is available in the underlying input stream #1188
Conversation
…am is a ByteArrayInputStream, the buffer has no more bytes, and the # of bytes to read is zero.
|
@lbergelson @jacarey or @tfenne care to take a look. |
|
Strange, input stream documentation says it should return 0 in that case.
|
|
Oh, I didn't read carefully enough, it's the combination of the end of stream AND requesting to read nothing that is the issue? |
Codecov Report
@@ Coverage Diff @@
## master #1188 +/- ##
==============================================
+ Coverage 68.397% 68.407% +0.01%
- Complexity 8014 8016 +2
==============================================
Files 542 542
Lines 32693 32700 +7
Branches 5529 5530 +1
==============================================
+ Hits 22361 22369 +8
Misses 8127 8127
+ Partials 2205 2204 -1
|
|
From
which doesn't agree with |
| @@ -415,7 +415,8 @@ public int readBytesOrFewer(final byte[] buffer, final int offset, final int len | |||
| throw new IllegalStateException("Calling read method on BinaryCodec open for write."); | |||
| } | |||
| try { | |||
| return inputStream.read(buffer, offset, length); | |||
| if (length == 0) return 0; | |||
There was a problem hiding this comment.
use { } around if body
| @@ -415,7 +415,8 @@ public int readBytesOrFewer(final byte[] buffer, final int offset, final int len | |||
| throw new IllegalStateException("Calling read method on BinaryCodec open for write."); | |||
| } | |||
| try { | |||
| return inputStream.read(buffer, offset, length); | |||
| if (length == 0) return 0; | |||
| else return inputStream.read(buffer, offset, length); | |||
There was a problem hiding this comment.
else unnecessary after return
There was a problem hiding this comment.
How about return (length == 0) ? 0 : inputStream.read(buffer, offset, length);
|
My interpretation is that |
|
Thanks for the note, I was looking at the source for |
| return inputStream.read(buffer, offset, length); | ||
| // Some implementations of InputStream do not behave well when the buffer is empty and length is zero, for | ||
| // example ByteArrayInputStream, so we must check for length equal to zero. | ||
| // See: https://bugs.java.com/view_bug.do?bug_id=6766844 |
There was a problem hiding this comment.
good find, thanks for adding this note.
|
I love the open java bugs from 2008 that no one has bothered responding too. |
|
On one of the linked bugs there's a comment
|
I ran into this bug when writing an empty string as the last element into a byte buffer (ex
binaryCodec.writeString("", true, false)) and then trying to read it back in (ex.binaryCodec.readLengthAndString(false)). The main issue is thatInputStream.readwill return-1even if the maximum number of bytes to read is zero (i.e.InputStream.read(buffer, 0, 0)will return-1).I made a commit with a test to show the bug and a commit to fix it.