Skip to content

Commit

Permalink
feat: do not attempt to guard server-side large object limit
Browse files Browse the repository at this point in the history
BlobInputStream.seek will now throw if attempting to skip beyond the max size for a large object. This is a breaking change. The previous implementation was imperfect as it did not account for the server being compiled with a different limit.

The state of the stream after the exception is undefined, but hopefully sane. Further exceptions may be thrown on future operations.
  • Loading branch information
OrangeDog committed May 13, 2024
1 parent b044578 commit 046152a
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 23 deletions.
Expand Up @@ -18,8 +18,6 @@
* This is an implementation of an InputStream from a large object.
*/
public class BlobInputStream extends InputStream {
static final int DEFAULT_BLOCK_SIZE = 8 * 1024;
static final int LO_BLOCK_SIZE = DEFAULT_BLOCK_SIZE / 4;
static final int DEFAULT_MAX_BUFFER_SIZE = 512 * 1024;
static final int INITIAL_BUFFER_SIZE = 64 * 1024;

Expand Down Expand Up @@ -93,19 +91,13 @@ public BlobInputStream(LargeObject lo, int bsize, long limit) {
// the first read to be exactly the initial buffer size
this.lastBufferSize = INITIAL_BUFFER_SIZE / 2;

// Trying to seek past the max size throws an exception, which skip() should avoid
long maxBlobSize;
try {
// initialise current position for mark/reset
this.absolutePosition = lo.tell64();
// https://github.com/postgres/postgres/blob/REL9_3_0/src/include/storage/large_object.h#L76
maxBlobSize = (long) Integer.MAX_VALUE * LO_BLOCK_SIZE;
} catch (SQLException e1) {
try {
// the tell64 function does not exist before PostgreSQL 9.3
this.absolutePosition = lo.tell();
// 2GiB is the limit for these older versions
maxBlobSize = Integer.MAX_VALUE;
} catch (SQLException e2) {
RuntimeException e3 = new RuntimeException("Failed to create BlobInputStream", e1);
e3.addSuppressed(e2);
Expand All @@ -114,7 +106,7 @@ public BlobInputStream(LargeObject lo, int bsize, long limit) {
}

// Treat -1 as no limit for backward compatibility
this.limit = limit == -1 ? maxBlobSize : limit + this.absolutePosition;
this.limit = limit == -1 ? Long.MAX_VALUE : limit + this.absolutePosition;
this.markPosition = this.absolutePosition;
}

Expand Down Expand Up @@ -367,7 +359,9 @@ public boolean markSupported() {
*
* @param n the number of bytes to be skipped.
* @return the actual number of bytes skipped which might be zero.
* @throws IOException if an I/O error occurs.
* @throws IOException if an underlying driver error occurs.
* In particular, this will throw if attempting to skip beyond the maximum length
* of a large object, which by default is 4,398,046,509,056 bytes.
* @see java.io.InputStream#skip(long)
*/
@Override
Expand All @@ -387,12 +381,10 @@ public long skip(long n) throws IOException {
}
long currentPosition = absolutePosition;
long skipped = targetPosition - currentPosition;
absolutePosition = targetPosition;

if (buffer != null && buffer.length - bufferPosition > skipped) {
bufferPosition += (int) skipped;
} else {
buffer = null;
try {
if (targetPosition <= Integer.MAX_VALUE) {
lo.seek((int) targetPosition, LargeObject.SEEK_SET);
Expand All @@ -405,7 +397,9 @@ public long skip(long n) throws IOException {
loId, n, currentPosition),
e);
}
buffer = null;
}
absolutePosition = targetPosition;
return skipped;
}
}
Expand Down
96 changes: 85 additions & 11 deletions pgjdbc/src/test/java/org/postgresql/test/jdbc2/BlobTest.java
Expand Up @@ -11,12 +11,16 @@
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
import static org.junit.jupiter.api.Assumptions.assumeFalse;
import static org.junit.jupiter.api.Assumptions.assumeTrue;

import org.postgresql.PGConnection;
import org.postgresql.core.ServerVersion;
import org.postgresql.largeobject.LargeObject;
import org.postgresql.largeobject.LargeObjectManager;
import org.postgresql.test.TestUtil;
import org.postgresql.util.PSQLException;
import org.postgresql.util.PSQLState;

import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.AfterEach;
Expand All @@ -39,6 +43,7 @@
import java.sql.Types;
import java.util.Arrays;
import java.util.concurrent.ThreadLocalRandom;
import java.util.concurrent.atomic.AtomicBoolean;

import javax.sql.rowset.serial.SerialBlob;
import javax.sql.rowset.serial.SerialClob;
Expand Down Expand Up @@ -340,13 +345,21 @@ void markResetWithInitialOffset() throws Exception {

@Test
void skip() throws Exception {
long expectedBigSkip;
if (TestUtil.haveMinimumServerVersion(con, ServerVersion.v9_3)) {
expectedBigSkip = 4398046442492L;
} else {
expectedBigSkip = 2147417083;
LargeObjectManager lom = ((PGConnection) con).getLargeObjectAPI();
long loid = createMediumLargeObject();

try (LargeObject blob = lom.open(loid, LargeObjectManager.READ)) {
InputStream bis = blob.getInputStream();
assertEquals(0, bis.read());
assertEquals(1024L, bis.skip(1024));
assertEquals(1, bis.read());
assertEquals(64 * 1024L, bis.skip(64 * 1024));
assertEquals(65, bis.read());
}
}

@Test
void skipBackwards() throws Exception {
LargeObjectManager lom = ((PGConnection) con).getLargeObjectAPI();
long loid = createMediumLargeObject();

Expand All @@ -355,14 +368,75 @@ void skip() throws Exception {
assertEquals(0, bis.read());
assertEquals(1024L, bis.skip(1024));
assertEquals(1, bis.read());
assertEquals(0L, bis.skip(-1));
assertEquals(0L, bis.skip(-1024));
assertEquals(1, bis.read());
assertEquals(64 * 1024L, bis.skip(64 * 1024));
assertEquals(65, bis.read());
assertEquals(expectedBigSkip, bis.skip(Long.MAX_VALUE));
}
}

@Test
void skipToEnd() throws Exception {
LargeObjectManager lom = ((PGConnection) con).getLargeObjectAPI();
long loid = createMediumLargeObject();

try (LargeObject blob = lom.open(loid, LargeObjectManager.READ)) {
InputStream bis = blob.getInputStream();
assertEquals(96 * 1024, bis.skip(96 * 1024));
assertEquals(-1, bis.read());
}
}

@Test
void skipPastEnd() throws Exception {
LargeObjectManager lom = ((PGConnection) con).getLargeObjectAPI();
long loid = createMediumLargeObject();

try (LargeObject blob = lom.open(loid, LargeObjectManager.READ)) {
InputStream bis = blob.getInputStream();
assertEquals(1024 * 1024, bis.skip(1024 * 1024));
assertEquals(-1, bis.read());
assertEquals(1024, bis.skip(1024));
assertEquals(-1, bis.read());
}
}

@Test
void skipPastMaxAfter9_3() throws Exception {
assumeTrue(TestUtil.haveMinimumServerVersion(con, ServerVersion.v9_3));

LargeObjectManager lom = ((PGConnection) con).getLargeObjectAPI();
long loid = createMediumLargeObject();

AtomicBoolean completedTest = new AtomicBoolean(false);
PSQLException ex = assertThrows(PSQLException.class, () -> {
try (LargeObject blob = lom.open(loid, LargeObjectManager.READ)) {
InputStream bis = blob.getInputStream();
assertEquals(0, bis.read());
assertThrows(IOException.class, () -> bis.skip(Long.MAX_VALUE / 2));
assertEquals(0, bis.read());
assertThrows(IOException.class, () -> { while (bis.read() != -1) {} });

Check failure on line 416 in pgjdbc/src/test/java/org/postgresql/test/jdbc2/BlobTest.java

View workflow job for this annotation

GitHub Actions / Code style

[Task :postgresql:checkstyleTest] [LeftCurly] '{' at column 47 should have line break after.

Check failure on line 416 in pgjdbc/src/test/java/org/postgresql/test/jdbc2/BlobTest.java

View workflow job for this annotation

GitHub Actions / Code style

[Task :postgresql:checkstyleTest] [RightCurly] '}' at column 75 should be alone on a line.
assertThrows(IOException.class, bis::close);
completedTest.set(true);
}
});

assertEquals(PSQLState.IN_FAILED_SQL_TRANSACTION.getState(), ex.getSQLState());
assertTrue(completedTest.get());
}

@Test
void skipPastMaxBefore9_3() throws Exception {
assumeFalse(TestUtil.haveMinimumServerVersion(con, ServerVersion.v9_3));

LargeObjectManager lom = ((PGConnection) con).getLargeObjectAPI();
long loid = createMediumLargeObject();

try (LargeObject blob = lom.open(loid, LargeObjectManager.READ)) {
InputStream bis = blob.getInputStream();
assertEquals(0, bis.read());
assertThrows(IOException.class, () -> bis.skip(Integer.MAX_VALUE));
assertEquals(0, bis.read());
while (bis.read() != -1) {}

Check failure on line 438 in pgjdbc/src/test/java/org/postgresql/test/jdbc2/BlobTest.java

View workflow job for this annotation

GitHub Actions / Code style

[Task :postgresql:checkstyleTest] [RightCurly] '}' at column 33 should be alone on a line.
bis.close();
assertThrows(IOException.class, () -> bis.skip(1));
}
}

Expand All @@ -376,7 +450,7 @@ void skipWithInitialOffset() throws Exception {

InputStream bis = blob.getInputStream();
assertEquals(1, bis.read());
assertEquals(1023L, bis.skip(1023));
assertEquals(1024L, bis.skip(1024));
assertEquals(2, bis.read());
assertEquals(64 * 1024L, bis.skip(64 * 1024));
assertEquals(66, bis.read());
Expand Down

0 comments on commit 046152a

Please sign in to comment.