On Mon, 17 May 2021 14:39:05 GMT, Mitsuru Kariya 
<github.com+2217224+kariya-mits...@openjdk.org> wrote:

>> Fix `SerialBlob.setBytes(long pos, byte[] bytes, int offset, int length)` in 
>> the following cases:
>> 
>> 1. `pos - 1 + bytes.length - offset > this.length() && pos - 1 + length <= 
>> this.length()`
>>    The original implementation throws `ArrayIndexOutOfBoundsException` but 
>> this case should end successfully.
>>    (test31)
>> 
>> 2. `pos - 1 + length > this.length()`
>>    The original implementation throws `ArrayIndexOutOfBoundsException` but 
>> this case should end successfully. *1
>>    (test32)
>> 
>> 3. `pos == this.length() + 1`
>>    The original implementation throws `SerialException` but this case should 
>> end successfully. *2
>>    (test33)
>> 
>> 4. `length < 0`
>>    The original implementation throws `ArrayIndexOutOfBoundsException` but 
>> this case should throw `SerialException`.
>>    (test34)
>> 
>> 5. `offset + length > Integer.MAX_VALUE`
>>    The original implementation throws `ArrayIndexOutOfBoundsException` (or 
>> `OutOfMemoryError` in most cases) but this case should throw 
>> `SerialException`.
>>    (test35)
>> 
>> Additionally, fix `SerialClob.setString(long pos, String str, int offset, 
>> int length)` in the following cases:
>> 
>> 1. `offset > str.length()`
>>    The original implementaion throws `StringIndexOutOfBoundsException` but 
>> this case should throw `SerialException`.
>>    (test39)
>> 
>> 2. `pos - 1 + str.length() - offset > this.length() && pos - 1 + length <= 
>> this.length()`
>>    The original implementation throws `ArrayIndexOutOfBoundsException` but 
>> this case should end successfully.
>>    (test32)
>> 
>> 3. `pos - 1 + length > this.length()`
>>    The original implementaion throws `SerialException` but this case should 
>> end successfully. *3
>>    (test40)
>> 
>> 4. `pos == this.length() + 1`
>>    The original implementaion throws `SerialException` but this case should 
>> end successfully. *4
>>    (test41)
>> 
>> 5. `length < 0`
>>    The original implementation throws `StringIndexOutOfBoundsException` but 
>> this case should throw `SerialException`.
>>    (test42)
>> 
>> 6. `offset + length > Integer.MAX_VALUE`
>>    The original implementation throws `ArrayIndexOutOfBoundsException` (or 
>> `OutOfMemoryError` in most cases) but this case should throw 
>> `SerialException`.
>>    (test43)
>> 
>> 
>> The javadoc has also been modified according to the above.
>> 
>> *1 The documentation of `Blob.setBytes()` says, "If the end of the Blob 
>> value is reached while writing the array of bytes, then the length of the 
>> Blob value will be increased to accommodate the extra bytes."
>> 
>> *2 The documentation of `Blob.setBytes()` says, "If the value specified for 
>> pos is greater than the length+1 of the BLOB value then the behavior is 
>> undefined."
>>    So, it should work correctly when pos == length+1 of the BLOB value.
>> 
>> *3 The documentation of `Clob.setString()` says, "If the end of the Clob 
>> value is eached while writing the given string, then the length of the Clob 
>> value will be increased to accommodate the extra characters."
>> 
>> *4 The documentation of `Clob.setString()` says, "If the value specified for 
>> pos is greater than the length+1 of the CLOB value then the behavior is 
>> undefined."
>>    So, it should work correctly when pos == length+1 of the CLOB value.
>
> Mitsuru Kariya has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix for length + offset > Integer.MAX_VALUE case

Overall the changes look reasonable.  As mentioned in the comments, a CSR will 
be required due to some of the wordsmithing cleanup

src/java.sql.rowset/share/classes/javax/sql/rowset/serial/SerialBlob.java line 
306:

> 304:      *
> 305:      * @param pos the position in the SQL <code>BLOB</code> value at 
> which
> 306:      *     to start writing. The first position is <code>1</code>;

When updating the javadoc to use @code, please update all instances for 
consistency

src/java.sql.rowset/share/classes/javax/sql/rowset/serial/SerialBlob.java line 
308:

> 306:      *     to start writing. The first position is <code>1</code>;
> 307:      *     must not be less than <code>1</code> nor greater than
> 308:      *     the length+1 of this {@code SerialBlob} object.

Changes such as this require a CSR.  I think I have convinced myself that it is 
OK to move forward with the CSR.

src/java.sql.rowset/share/classes/javax/sql/rowset/serial/SerialBlob.java line 
313:

> 311:      * @return the number of bytes written
> 312:      * @throws SerialException if there is an error accessing the
> 313:      *     {@code BLOB} value; or if an invalid position is set;

Even though this addresses a typo, this will require a CSR

-------------

Changes requested by lancea (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/4001

Reply via email to