InputStream returned from Blob.getBinaryStream(long, long) terminates the stream by returning 0, should return -1

• Key: DERBY-4061
• Type: Bug
• Status: Resolved
• Resolution: Fixed
• Priority: Major
• Assignee: Kristian Waagan
• Reporter: Trejkaz
• Component/s: JDBC
• Affects Version/s: 10.3.3.0, 10.4.2.0, 10.5.1.1
• Fix Version/s: 10.3.3.1, 10.4.2.1, 10.5.1.1

Description

Take the following code to copy a sub-stream from a BLOB:

Blob blob = rs.getBlob(1);
InputStream stream = blob.getBinaryStream(1000, 2000);
try {
  IOUtils.copy(stream, out);
} finally {
  stream.close();
}

This will loop forever in IOUtils.copy, as the InputStream is returning 0 at end of stream instead of -1. It probably should return -1 to comply with the Javadoc for InputStream.

Notes:
I have not yet checked Blob.getBinaryStream(), or any of the Clob methods, or the client driver. So some of these may have a similar issue.

Comments

[Kristian Waagan]

I think the problem is restricted to UpdatableBlobStream only. The combinations I tried with Clob succeeded, and the client driver also succeeds with the repro for both Blob and Clob.
The problem in UpdatableBlobStream, is that the underlying source is asked to read zero bytes, which it does... The calling code fails to handle this scenario, which is a valid one seen from InputStream (from JavaDoc: "If len is zero, then no bytes are read and 0 is returned; otherwise, there is an attempt to read at least one byte.").

[Kristian Waagan]

Patch 1a fixes the bug and adds a regression test. I think there is at least one more bug in the class, I'll investigate later.
Note that the regression test will cause a hang if the fix is removed.

Patch ready for review.
Running the regression suite.

[Knut Anders Hatlen]

I think there is one tiny problem with the patch, but I haven't actually tested that it is a problem. I believe that in this code

+ if (remaining == 0 && len != 0) {
+ return -1;
+ }

(len != 0) should be replaced with (len > 0). Otherwise, we return -1 when len is negative instead of throwing an IndexOutOfBoundsException.

In the test there's a typo (infinit -> infinite), and the variable n is unused.

[Kristian Waagan]

Regression tests passed without failures on Solaris 10, Java SE 6.

[Kristian Waagan]

Thanks for the review and the comments, Knut.

Patch 1b addresses the comments.
Committed to trunk with revision 746236.

[Kristian Waagan]

Backported fix to 10.4 with revision 747393 and to 10.3 with revision 747395.

[Kristian Waagan]

Tinderbox tests for 10.4 and 10.3 ran successfully.
This issue can be verified and, if found to be fixed, closed by the reporter.