Skip to content

Commit

Permalink
#764 Do not reject attempts to read blob id 0
Browse files Browse the repository at this point in the history
  • Loading branch information
mrotteveel committed Sep 15, 2023
1 parent 620e7ba commit b5949a0
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,42 @@ public class JnaBlob extends AbstractFbBlob implements FbBlob, DatabaseListener
private final FbClientLibrary clientLibrary;
private ByteBuffer byteBuffer;

/**
* Creates a blob for output (writing to the database).
*
* @param database
* database
* @param transaction
* transaction
* @param blobParameterBuffer
* blob parameter buffer
*/
public JnaBlob(JnaDatabase database, JnaTransaction transaction, BlobParameterBuffer blobParameterBuffer) {
this(database, transaction, blobParameterBuffer, NO_BLOB_ID);
this(database, transaction, blobParameterBuffer, NO_BLOB_ID, true);
}

/**
* Creates a blob for input (reading from the database).
*
* @param database
* database
* @param transaction
* transaction
* @param blobParameterBuffer
* blob parameter buffer
* @param blobId
* blob id
*/
public JnaBlob(JnaDatabase database, JnaTransaction transaction, BlobParameterBuffer blobParameterBuffer,
long blobId) {
this(database, transaction, blobParameterBuffer, blobId, false);
}

private JnaBlob(JnaDatabase database, JnaTransaction transaction, BlobParameterBuffer blobParameterBuffer,
long blobId, boolean outputBlob) {
super(database, transaction, blobParameterBuffer);
this.blobId = new LongByReference(blobId);
outputBlob = blobId == NO_BLOB_ID;
this.outputBlob = outputBlob;
clientLibrary = database.getClientLibrary();
}

Expand Down
8 changes: 8 additions & 0 deletions src/docs/asciidoc/release_notes.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -812,6 +812,14 @@ Previously almost all types returned `true` (including -- for example -- `BOOLEA
* Added methods `List<String> getTypeAliasList()` and `List<String> getSupportedProtocolList()` to `GDSFactoryPlugin`, and deprecated `String[] getTypeAliases()` and `String[] getSupportedProtocols()` for removal in Jaybird 7 or later
* Fixed formatting of `isc_formatted_exception` to not repeat the original parameters of the exception (https://github.com/FirebirdSQL/jaybird/issues/749[jaybird#749])
* Added aliases `ApplicationName` and `applicationName` for connection property `processName` (https://github.com/FirebirdSQL/jaybird/issues/751[jaybird#751])
* Improvement: Do not reject attempts to read blob id 0 (https://github.com/FirebirdSQL/jaybird/issues/764[jaybird#764])
+
Previously, Jaybird rejected attempts to read blobs with blob id `0` (not on all code paths, for some only when assertions are enabled).
Formally, blob id `0` is not a valid blob id, but in practice they can occur (e.g. due to bugs, or access components/drivers explicitly setting a blob column to id `0`).
Other drivers and tools simply send the requests for blob id `0` to the server, which then treats it as an empty blob.
For consistency, we decided to let Jaybird handle it the same.
+
This change was also backported to Jaybird 5.0.3.
[#removal-of-deprecated-classes-and-packages]
=== Removal of deprecated classes and packages
Expand Down
16 changes: 16 additions & 0 deletions src/jna-test/org/firebirdsql/gds/ng/jna/JnaBlobTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,22 @@ void testOutputBlobDoubleOpen() throws Exception {
}
}

@Test
void readBlobIdZero() throws Exception {
try (JnaDatabase db = createDatabaseConnection()) {
try {
transaction = getTransaction(db);
FbBlob blob = db.createBlobForInput(transaction, 0);
blob.open();
assertEquals(0, blob.length());
byte[] segment = blob.getSegment(500);
assertEquals(0, segment.length, "expected empty segment");
} finally {
if (transaction != null) transaction.commit();
}
}
}

@Override
protected JnaDatabase createFbDatabase(FbConnectionProperties connectionInfo) throws SQLException {
final JnaDatabase db = factory.connect(connectionInfo);
Expand Down
4 changes: 2 additions & 2 deletions src/main/org/firebirdsql/gds/ng/AbstractFbBlob.java
Original file line number Diff line number Diff line change
Expand Up @@ -353,10 +353,10 @@ public <T> T getBlobInfo(final byte[] requestItems, final int bufferLength, fina
public long length() throws SQLException {
try (LockCloseable ignored = withLock()) {
checkDatabaseAttached();
if (getBlobId() == FbBlob.NO_BLOB_ID) {
if (isOutput() && getBlobId() == FbBlob.NO_BLOB_ID) {
throw FbExceptionBuilder.forException(ISCConstants.isc_bad_segstr_id).toSQLException();
}
final BlobLengthProcessor blobLengthProcessor = createBlobLengthProcessor();
BlobLengthProcessor blobLengthProcessor = createBlobLengthProcessor();
return getBlobInfo(blobLengthProcessor.getBlobLengthItems(), 20, blobLengthProcessor);
} catch (SQLException e) {
exceptionListenerDispatcher.errorOccurred(e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@

import org.firebirdsql.gds.BlobParameterBuffer;
import org.firebirdsql.gds.ISCConstants;
import org.firebirdsql.gds.ng.FbBlob;
import org.firebirdsql.gds.ng.FbExceptionBuilder;

import java.sql.SQLException;
Expand All @@ -34,9 +33,8 @@ public abstract class AbstractFbWireInputBlob extends AbstractFbWireBlob {
private final long blobId;

protected AbstractFbWireInputBlob(FbWireDatabase database, FbWireTransaction transaction,
BlobParameterBuffer blobParameterBuffer, long blobId) {
BlobParameterBuffer blobParameterBuffer, long blobId) {
super(database, transaction, blobParameterBuffer);
assert blobId != FbBlob.NO_BLOB_ID : "blobId must be non-zero for an input blob";
this.blobId = blobId;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,4 +226,21 @@ public void testDoubleOpen() throws Exception {
}
}
}

@Test
public void readBlobIdZero() throws Exception {
try (FbWireDatabase db = createDatabaseConnection()) {
try {
transaction = getTransaction(db);
FbBlob blob = db.createBlobForInput(transaction, 0);
blob.open();
assertEquals(0, blob.length());
byte[] segment = blob.getSegment(500);
assertEquals(0, segment.length, "expected empty segment");
} finally {
if (transaction != null) transaction.commit();
}
}
}

}

0 comments on commit b5949a0

Please sign in to comment.