Issue Details (XML | Word | Printable)

Key: DERBY-4060
Type: Bug Bug
Status: Resolved Resolved
Resolution: Fixed
Priority: Major Major
Assignee: Kristian Waagan
Reporter: Trejkaz
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
Derby

Blob.getBinaryStream(long,long) is off by one for the pos+len check

Created: 16/Feb/09 10:00 PM   Updated: 04/May/09 06:21 PM
Component/s: JDBC, Network Client
Affects Version/s: 10.3.3.1, 10.4.2.0, 10.5.1.1
Fix Version/s: 10.3.3.1, 10.4.2.1, 10.5.1.1

Time Tracking:
Not Specified

File Attachments:
  Size
File Licensed for inclusion in ASF works derby-4060-1a-sub_stream_fix.diff 2009-02-26 11:44 AM Kristian Waagan 15 kB
File Licensed for inclusion in ASF works derby-4060-1a-sub_stream_fix.stat 2009-02-26 11:44 AM Kristian Waagan 0.5 kB
File Licensed for inclusion in ASF works derby-4060-1b-sub_stream_fix.diff 2009-03-02 08:48 AM Kristian Waagan 15 kB
File Licensed for inclusion in ASF works derby-4060-2a-doc_changes.diff 2009-03-04 11:02 AM Kristian Waagan 4 kB

Urgency: Normal
Resolution Date: 09/Mar/09 09:54 AM
Labels:


 Description  « Hide
If you have a BLOB of length 20, and call blob.getBinaryStream(11,10), it will give you an error:

    java.sql.SQLException: Sum of position('11') and length('10') is greater than the size of the LOB.

This is following word for word an error in the JDBC Javadoc:

        SQLException - if pos is less than 1 or if pos is greater than the number of bytes in the Blob or if pos + length is greater than the number of bytes in the Blob

So it's checking 11 + 10 > 20, but it should check 11 + 10 > 21 (pos + len > blob.length() + 1) to allow reading the last byte.

The Javadoc for Clob.getCharacterStream(long,long) has similar wording so it may have the same issue.

Likewise, the client driver may have the same issue -- I haven't yet checked.



 All   Comments   Work Log   Change History   Subversion Commits      Sort Order: Ascending order - Click to sort in descending order
Kristian Waagan added a comment - 17/Feb/09 08:15 AM
This issue is present for both Blob and Clob, and also in the client driver.

Kristian Waagan made changes - 17/Feb/09 08:15 AM
Field Original Value New Value
Component/s Network Client [ 11690 ]
Affects Version/s 10.5.0.0 [ 12313010 ]
Kristian Waagan made changes - 20/Feb/09 08:38 AM
Assignee Kristian Waagan [ kristwaa ]
Kristian Waagan made changes - 26/Feb/09 09:16 AM
Status Open [ 1 ] In Progress [ 3 ]
Kristian Waagan added a comment - 26/Feb/09 11:44 AM
The patch 'derby-4060-1a-sub_stream_fix.diff' fixes the reported bug.
The changes in the production code consist of subtracting one from the position when checking against the length.
Using the example from the report;
 o current: 11 + 10 > 20 = true (exception thrown)
 o with patch: (11 -1) + 10 > 20 = false (check passes)

I made some more changes to the test code. Three tests were added, one for Blob and two for Clob (one Clob loaded with latin chars only, one loaded with CJK chars).
In addition, CharAlphabet-objects cannot be shared between readers. I dealt with this by adding getClone method to the CharAlphabet-class, and getting a clone when creating the LoopingAlphabetReader. This should maybe be changed in a more robust way.
It should also be noted that the method draining the stream and returning the last byte/char, performs more checks than usual.

Patch ready for review.

Kristian Waagan made changes - 26/Feb/09 11:44 AM
Attachment derby-4060-1a-sub_stream_fix.stat [ 12401024 ]
Attachment derby-4060-1a-sub_stream_fix.diff [ 12401023 ]
Kristian Waagan made changes - 26/Feb/09 11:54 AM
Derby Info [Patch Available]
Repository Revision Date User Message
ASF #749235 Mon Mar 02 08:47:52 UTC 2009 kristwaa DERBY-4060: Blob.getBinaryStream(long,long) is off by one for the pos+len check.
Changed the pos/length checks to allow obtaining a stream reading the last
byte/char from the LOB.
The JavaDoc for Blob.getBinaryStream(long,long) and
Clob.getCharacterStream(long,long) (JDBC 4.0) incorrectly states that the
position plus the requested length of the stream cannot be larger than the
length of the LOB. Since positions in JDBC are 1-based, this makes it impossible
to read the last byte/char in the LOB. Derby adhered to the spec.

The changes to CharAlphabet/LoopingAlphabetReader were done to allow passing
an alphabet object around for constructing streams.
Patch file: DERBY-4060-1b-sub_stream_fix.diff
Files Changed
MODIFY /db/derby/code/trunk/java/client/org/apache/derby/client/am/Lob.java
MODIFY /db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/util/streams/LoopingAlphabetReader.java
MODIFY /db/derby/code/trunk/java/engine/org/apache/derby/impl/jdbc/EmbedBlob.java
MODIFY /db/derby/code/trunk/java/engine/org/apache/derby/impl/jdbc/EmbedClob.java
MODIFY /db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/util/streams/CharAlphabet.java
MODIFY /db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbc4/BlobTest.java
MODIFY /db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbc4/ClobTest.java

Kristian Waagan added a comment - 02/Mar/09 08:48 AM
Attached an updated patch, adding two more comments.

Kristian Waagan made changes - 02/Mar/09 08:48 AM
Attachment derby-4060-1b-sub_stream_fix.diff [ 12401223 ]
Kristian Waagan added a comment - 02/Mar/09 08:49 AM
Committed patch 1b to trunk with revision 749235.

Kristian Waagan made changes - 02/Mar/09 08:49 AM
Derby Info [Patch Available]
Fix Version/s 10.5.0.0 [ 12313010 ]
Kristian Waagan added a comment - 02/Mar/09 09:05 AM
The fix seems to merge cleanly with both 10.4 and 10.3 (I haven't run tests yet).
I'll wait for a few days to see if any comments or problems show up, and if not I'll backport the fix.

Kristian Waagan made changes - 02/Mar/09 09:05 AM
Affects Version/s 10.3.3.1 [ 12313143 ]
Knut Anders Hatlen added a comment - 03/Mar/09 09:25 AM
Should we also update the javadoc comments in EmbedBlob and EmbedClob (they still say that we throw SQLException if pos+length is greater than the number of bytes/characters)?

Same goes for the error message (XJ087.S) which says "Sum of position('{0}') and length('{1}') is greater than the size of the LOB."

Kristian Waagan added a comment - 04/Mar/09 11:02 AM
Thank you for the comment, Knut Anders.
Patch 2a changes the JavaDoc and the error message.
Feel free to comment on the wording.

I plan to commit this on Monday.
Patch ready for review.

Kristian Waagan made changes - 04/Mar/09 11:02 AM
Attachment derby-4060-2a-doc_changes.diff [ 12401398 ]
Knut Anders Hatlen added a comment - 04/Mar/09 12:46 PM
The patch looks good to me. (I was wondering what we should do with the translations, but it appears that we are only required to do something if the message parameters are changed, see comment near the top of messages.xml. For now it's probably better to leave the (possibly inaccurate) translated messages than to remove them.)

Repository Revision Date User Message
ASF #750409 Thu Mar 05 10:23:18 UTC 2009 kristwaa DERBY-4060: Blob.getBinaryStream(long,long) is off by one for the pos+len check.
Merged fix and tests from trunk (revision 749235).
Files Changed
MODIFY /db/derby/code/branches/10.4/java/client/org/apache/derby/client/am/Lob.java
MODIFY /db/derby/code/branches/10.4/java/testing/org/apache/derbyTesting/functionTests/util/streams/LoopingAlphabetReader.java
MODIFY /db/derby/code/branches/10.4/java/engine/org/apache/derby/impl/jdbc/EmbedBlob.java
MODIFY /db/derby/code/branches/10.4/java/engine/org/apache/derby/impl/jdbc/EmbedClob.java
MODIFY /db/derby/code/branches/10.4/java/testing/org/apache/derbyTesting/functionTests/util/streams/CharAlphabet.java
MODIFY /db/derby/code/branches/10.4/java/testing/org/apache/derbyTesting/functionTests/tests/jdbc4/BlobTest.java
MODIFY /db/derby/code/branches/10.4/java/testing/org/apache/derbyTesting/functionTests/tests/jdbc4/ClobTest.java

Repository Revision Date User Message
ASF #750410 Thu Mar 05 10:23:49 UTC 2009 kristwaa DERBY-4060: Blob.getBinaryStream(long,long) is off by one for the pos+len check.
Merged fix and tests from trunk (revision 749235).
Files Changed
MODIFY /db/derby/code/branches/10.3/java/testing/org/apache/derbyTesting/functionTests/tests/jdbc4/ClobTest.java
MODIFY /db/derby/code/branches/10.3/java/client/org/apache/derby/client/am/Lob.java
MODIFY /db/derby/code/branches/10.3/java/testing/org/apache/derbyTesting/functionTests/util/streams/LoopingAlphabetReader.java
MODIFY /db/derby/code/branches/10.3/java/engine/org/apache/derby/impl/jdbc/EmbedBlob.java
MODIFY /db/derby/code/branches/10.3/java/engine/org/apache/derby/impl/jdbc/EmbedClob.java
MODIFY /db/derby/code/branches/10.3/java/testing/org/apache/derbyTesting/functionTests/util/streams/CharAlphabet.java
MODIFY /db/derby/code/branches/10.3/java/testing/org/apache/derbyTesting/functionTests/tests/jdbc4/BlobTest.java

Kristian Waagan added a comment - 05/Mar/09 10:29 AM
Backported patch 1b to 10.4 with revision 750409 and to 10.3 with revision 750410.

Kristian Waagan made changes - 05/Mar/09 10:29 AM
Fix Version/s 10.4.2.1 [ 12313401 ]
Fix Version/s 10.3.3.1 [ 12313143 ]
Derby Info [Patch Available]
Repository Revision Date User Message
ASF #751637 Mon Mar 09 09:44:44 UTC 2009 kristwaa DERBY-4060: Blob.getBinaryStream(long,long) is off by one for the pos+len check.
Changes to the JavaDoc and one error message, to better describe the error situation.
Patch file: DERBY-4060-2a-doc_changes.diff
Files Changed
MODIFY /db/derby/code/trunk/java/client/org/apache/derby/client/am/Blob.java
MODIFY /db/derby/code/trunk/java/client/org/apache/derby/client/am/Clob.java
MODIFY /db/derby/code/trunk/java/engine/org/apache/derby/impl/jdbc/EmbedBlob.java
MODIFY /db/derby/code/trunk/java/engine/org/apache/derby/impl/jdbc/EmbedClob.java
MODIFY /db/derby/code/trunk/java/engine/org/apache/derby/loc/messages.xml

Repository Revision Date User Message
ASF #751639 Mon Mar 09 09:52:11 UTC 2009 kristwaa DERBY-4060: Blob.getBinaryStream(long,long) is off by one for the pos+len check.
Backported doc fix from trunk (revision 751637).
Files Changed
MODIFY /db/derby/code/branches/10.4/java/client/org/apache/derby/client/am/Blob.java
MODIFY /db/derby/code/branches/10.4/java/client/org/apache/derby/client/am/Clob.java
MODIFY /db/derby/code/branches/10.4/java/engine/org/apache/derby/impl/jdbc/EmbedBlob.java
MODIFY /db/derby/code/branches/10.4/java/engine/org/apache/derby/impl/jdbc/EmbedClob.java
MODIFY /db/derby/code/branches/10.4/java/engine/org/apache/derby/loc/messages.xml

Repository Revision Date User Message
ASF #751640 Mon Mar 09 09:52:49 UTC 2009 kristwaa DERBY-4060: Blob.getBinaryStream(long,long) is off by one for the pos+len check.
Backported doc fix from trunk (revision 751637).
Files Changed
MODIFY /db/derby/code/branches/10.3/java/client/org/apache/derby/client/am/Blob.java
MODIFY /db/derby/code/branches/10.3/java/client/org/apache/derby/client/am/Clob.java
MODIFY /db/derby/code/branches/10.3/java/engine/org/apache/derby/impl/jdbc/EmbedBlob.java
MODIFY /db/derby/code/branches/10.3/java/engine/org/apache/derby/impl/jdbc/EmbedClob.java
MODIFY /db/derby/code/branches/10.3/java/engine/org/apache/derby/loc/messages.xml

Kristian Waagan added a comment - 09/Mar/09 09:54 AM
Committed patch 2a to trunk with revision 751637.
Backported to 10.4 and 10.3 with revisions 751639 and 751640.

This concludes my work on this issue.

Kristian Waagan made changes - 09/Mar/09 09:54 AM
Status In Progress [ 3 ] Resolved [ 5 ]
Derby Info [Patch Available]
Resolution Fixed [ 1 ]
Myrna van Lunteren made changes - 04/May/09 06:21 PM
Affects Version/s 10.5.1.1 [ 12313771 ]
Affects Version/s 10.5.0.0 [ 12313010 ]
Fix Version/s 10.5.1.1 [ 12313771 ]
Fix Version/s 10.5.0.0 [ 12313010 ]