"V.Narayanan (JIRA)" <[email protected]> writes:
> V.Narayanan commented on DERBY-796:
> -----------------------------------
> Issue 5
> -------------
>
> --- java/client/org/apache/derby/client/am/Blob.java (revision 377622)
> +++ java/client/org/apache/derby/client/am/Blob.java (working copy)
> @@ -267,7 +267,10 @@
>
> public int setBytesX(long pos, byte[] bytes, int offset, int len)
> throws SqlException {
> int length = 0;
> - if ((int) pos <= 0) {
> + boolean insertIntoEmpty = false;
> + if(pos==1 && binaryString_.length==0)
> + insertIntoEmpty = true;
> + if ((int) pos <= 0 || ((!insertIntoEmpty) && (pos >
> binaryString_.length - dataOffset_))) {
> throw new SqlException(agent_.logWriter_,
> new MessageId(SQLState.BLOB_BAD_POSITION), new Long(pos));
> }
>
> It seemed like the essence of this change was something like:
>
> "It's usually an error to pass a 'pos' value greater than the
> length of the blob, but if the blob is currently empty then
> there is a special case where the caller passes 1, not 0, as
> you might expect."
>
> The interpretation above is exactly what I tried to do. I will try
> to explain what I intended and also restructure the code to make it
> more understandable.
>
> There are two cases when exception needs to be thrown
>
> a) when pos <=0
> b) when pos > binaryString_.length - dataOffset_
> b.1) This case arises when your insert into a empty Blob. so here
> pos = 1 and (binaryString_.length - dataOffset_)=0.
> this should not result in a SQL exception being thrown.
Is the empty blob really a special case? I think you get the same
problem when the blob is not empty.
Let's say you want to append a byte array to a blob with size 1. Then
you have:
pos = 2
(binaryString_.length - dataOffset_) = 1
In this case, (pos > binaryString_ - dataOffset_) is true and an
exception is thrown. But there's no reason to throw an exception in
this case, is it?
I think you have to change this code:
public int setBytesX(long pos, byte[] bytes, int offset, int len) throws
SqlException {
int length = 0;
boolean insertIntoEmpty = false;
if(pos==1 && binaryString_.length==0)
insertIntoEmpty = true;
if ((int) pos <= 0 || ((!insertIntoEmpty) && (pos > binaryString_.length
- dataOffset_))) {
throw new SqlException(agent_.logWriter_,
new MessageId(SQLState.BLOB_BAD_POSITION), new Long(pos));
}
if ( pos > binaryString_.length - dataOffset_) {
throw new SqlException(agent_.logWriter_,
new MessageId(SQLState.BLOB_POSITION_TOO_LARGE), new Long(pos));
}
to
public int setBytesX(long pos, byte[] bytes, int offset, int len) throws
SqlException {
int length = 0;
if ((int) pos <= 0) {
throw new SqlException(agent_.logWriter_,
new MessageId(SQLState.BLOB_BAD_POSITION), new Long(pos));
}
if (pos - 1 > binaryString_.length - dataOffset_) {
throw new SqlException(agent_.logWriter_,
new MessageId(SQLState.BLOB_POSITION_TOO_LARGE), new Long(pos));
}
This will solve both the special case of inserting into an empty Blob,
and the more general case of inserting at the end.
> Issue - 6
> -----------------
>
> it is a cleaner approach to do the conversion from one-based index
> to zero-based index on the highest possible level
> -------------------------------------------------
>
> Would it be acceptable if I add a proper comment explaining my
> change for now and raise this as a seperate issue and fix it at the
> earliest?
It seems like the Blob/Clob code has a lot of potential
off-by-ones. Since this is somewhat outside the scope of your patch, I
think it is OK that you just include the simple (offset_ - 1) fixes
for now (they seem to be sufficient to fix the bugs you have
encountered), and address the cleanup of the offset/position confusion
in a separate JIRA issue.
--
Knut Anders
