[java-plugin-storage-jdbc] branch main updated: JJDBC-33 Enforce limits on context and key size
Codeberg
noreply at shibboleth.net
Thu Jul 9 18:28:03 UTC 2026
This is an automated email from the git hooks/post-receive script.
codeberg pushed a commit to branch main
in repository java-plugin-storage-jdbc.
View the commit online:
https://codeberg.org/Shibboleth/java-plugin-storage-jdbc/commit/d3282fe02649a2aecd73bf7fb15a54464fbe60d2
The following commit(s) were added to refs/heads/main by this push:
new d3282fe JJDBC-33 Enforce limits on context and key size
d3282fe is described below
commit d3282fe02649a2aecd73bf7fb15a54464fbe60d2
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Thu Jul 9 19:28:08 2026 +0100
JJDBC-33 Enforce limits on context and key size
https://shibboleth.atlassian.net/browse/JJDBC-33
Convert the strings into a byte array and check the length of that
---
.../storage/jdbc/impl/JDBCStorageService.java | 47 +++++++++++--
.../storage/jdbc/impl/JDBCStorageServiceTest.java | 78 +++++++++++++++-------
2 files changed, 95 insertions(+), 30 deletions(-)
diff --git a/jdbc-storage-impl/src/main/java/net/shibboleth/plugin/storage/jdbc/impl/JDBCStorageService.java b/jdbc-storage-impl/src/main/java/net/shibboleth/plugin/storage/jdbc/impl/JDBCStorageService.java
index 489b4ae..e18ca5a 100644
--- a/jdbc-storage-impl/src/main/java/net/shibboleth/plugin/storage/jdbc/impl/JDBCStorageService.java
+++ b/jdbc-storage-impl/src/main/java/net/shibboleth/plugin/storage/jdbc/impl/JDBCStorageService.java
@@ -50,7 +50,6 @@ import net.shibboleth.shared.collection.CollectionSupport;
import net.shibboleth.shared.collection.Pair;
import net.shibboleth.shared.component.ComponentInitializationException;
import net.shibboleth.shared.logic.Constraint;
-import net.shibboleth.shared.logic.ConstraintViolationException;
import net.shibboleth.shared.primitive.LoggerFactory;
import net.shibboleth.shared.primitive.StringSupport;
@@ -126,7 +125,6 @@ public final class JDBCStorageService extends AbstractStorageService
"SELECT id FROM StorageRecords WHERE context=? AND id like ? AND (expires IS NULL OR expires > ?)";
/** Default timeout of SQL queries. */
- @SuppressWarnings("null")
@Nonnull static final Duration DEFAULT_QUERY_TIMEOUT = Duration.ofSeconds(5);
/** Class logger. */
@@ -270,10 +268,7 @@ public final class JDBCStorageService extends AbstractStorageService
* @param count how many time to try before we bail.
*/
public void setTransactionRetries(@Positive final int count) {
- transactionRetries = count;
- if (count < 0) {
- throw new ConstraintViolationException("transaction retry must be positive");
- }
+ transactionRetries = Constraint.isGreaterThanOrEqual(0, count, "transaction retry must be positive");
}
/** Will we do thread level locking or delegate to the Database?
@@ -474,6 +469,28 @@ public final class JDBCStorageService extends AbstractStorageService
}
}
+ /** Check that the strings fit within the constrained size, when converted into byte arrays
+ * @param context the context
+ * @param key the key (if relevant for the operation being policed)
+ * @throws IoException if either are too large.
+ */
+ private void checkContextAndKeySize(@Nonnull final String context, @Nullable String key) throws IOException
+ {
+ int size = context.getBytes().length;
+ if (size > getContextSize()) {
+ log.error("Context {} was too large ({{} > {})", context, size, getContextSize());
+ throw new IOException("Supplied Context was too large");
+ }
+
+ if (key != null) {
+ size = key.getBytes().length;
+ if (size > getKeySize()) {
+ log.error("Key {} was too large ({{} > {})", key, size, getKeySize());
+ throw new IOException("Supplied Key was too large");
+ }
+ }
+ }
+
/**
* Returns all contexts from the store (for testing only).
*
@@ -543,6 +560,8 @@ public final class JDBCStorageService extends AbstractStorageService
final List<JDBCStorageRecord<?>> result = new ArrayList<>();
Constraint.isNotEmpty(Constraint.isNotNull(context, "ReadAll(String): context must not be null"),
"ReadAll(String): context must not be empty");
+ checkContextAndKeySize(context, null);
+
try (final ConnectionWithLock connection = new ConnectionWithLock(true, false);
final PreparedStatement query = connection.prepareStatement(readAllByContextSQL)) {
log.trace("ReadAll:: '{}' 1: '{}' ", readAllByContextSQL, context);
@@ -576,6 +595,8 @@ public final class JDBCStorageService extends AbstractStorageService
"create: context must not be empty");
Constraint.isNotEmpty(Constraint.isNotNull(value, "create: value must not be null"),
"create: value must not be empty");
+ checkContextAndKeySize(context, key);
+
int retries = transactionRetries;
while(true) {
try (final ConnectionWithLock connection = new ConnectionWithLock(false, true)) {
@@ -677,6 +698,7 @@ public final class JDBCStorageService extends AbstractStorageService
"read: context must not be empty");
Constraint.isNotEmpty(Constraint.isNotNull(key, "read: key must not be null"),
"read: key must not be empty");
+ checkContextAndKeySize(context, key);
int retries = transactionRetries;
while(true) {
@@ -794,6 +816,8 @@ public final class JDBCStorageService extends AbstractStorageService
"update: context must not be empty");
Constraint.isNotEmpty(Constraint.isNotNull(key, "update: key must not be null"),
"update: key must not be empty");
+ checkContextAndKeySize(context, key);
+
int retries = transactionRetries;
while (true) {
try (final ConnectionWithLock connection = new ConnectionWithLock(false, true);
@@ -902,6 +926,8 @@ public final class JDBCStorageService extends AbstractStorageService
Constraint.isNotEmpty(Constraint.isNotNull(context, "delete: key must not be null"),
"delete: key must not be empty");
Constraint.isTrue(version == null || version > 0, "delete: version should be null of > 0");
+ checkContextAndKeySize(context, key);
+
int retries = transactionRetries;
while (true) {
try (final ConnectionWithLock connection= new ConnectionWithLock(false, true);
@@ -999,6 +1025,8 @@ public final class JDBCStorageService extends AbstractStorageService
public void reap(@Nonnull @NotEmpty final String context) throws IOException {
Constraint.isNotEmpty(Constraint.isNotNull(context, "reap: context must not be null"),
"reap: context must not be empty");
+ checkContextAndKeySize(context, null);
+
int retries = transactionRetries;
while (true) {
try (final ConnectionWithLock connection = new ConnectionWithLock(true, true);
@@ -1037,6 +1065,8 @@ public final class JDBCStorageService extends AbstractStorageService
throws IOException {
Constraint.isNotEmpty(Constraint.isNotNull(context, "updateContextExpiration: context must not be null"),
"updateContextExpiration: context must not be empty");
+ checkContextAndKeySize(context, null);
+
int retries = transactionRetries;
while (true) {
try (final ConnectionWithLock connection = new ConnectionWithLock(true, true);
@@ -1077,6 +1107,8 @@ public final class JDBCStorageService extends AbstractStorageService
public void deleteContext(@Nonnull @NotEmpty final String context) throws IOException {
Constraint.isNotEmpty(Constraint.isNotNull(context, "deleteContext: context must not be null"),
"deleteContext: context must not be empty");
+ checkContextAndKeySize(context, null);
+
int retries = transactionRetries;
while (true) {
try (final ConnectionWithLock connection = new ConnectionWithLock(true, true);
@@ -1114,6 +1146,7 @@ public final class JDBCStorageService extends AbstractStorageService
@Nonnull public Iterable<String> getContextKeys(@Nonnull @NotEmpty final String context,
@Nullable final String prefix) throws IOException {
+ checkContextAndKeySize(context, null);
final List<String> result = new ArrayList<>();
try (final ConnectionWithLock connection= new ConnectionWithLock(true, false);
@@ -1331,4 +1364,4 @@ public final class JDBCStorageService extends AbstractStorageService
}
}
-}
\ No newline at end of file
+}
diff --git a/jdbc-storage-impl/src/test/java/net/shibboleth/plugin/storage/jdbc/impl/JDBCStorageServiceTest.java b/jdbc-storage-impl/src/test/java/net/shibboleth/plugin/storage/jdbc/impl/JDBCStorageServiceTest.java
index 56e9e42..c47d904 100644
--- a/jdbc-storage-impl/src/test/java/net/shibboleth/plugin/storage/jdbc/impl/JDBCStorageServiceTest.java
+++ b/jdbc-storage-impl/src/test/java/net/shibboleth/plugin/storage/jdbc/impl/JDBCStorageServiceTest.java
@@ -17,6 +17,7 @@ package net.shibboleth.plugin.storage.jdbc.impl;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNotEquals;
import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.fail;
import java.io.IOException;
import java.security.SecureRandom;
@@ -56,32 +57,32 @@ public class JDBCStorageServiceTest extends StorageServiceTest {
private final boolean USE_SQLSERVER = false;
@Nonnull private static final String INIT_SQL_SQLSERVER="CREATE TABLE StorageRecords (\r\n"
- + " context varchar(255) COLLATE Latin1_General_100_CS_AS NOT NULL,\n"
- + " id varchar(255) COLLATE Latin1_General_100_CS_AS NOT NULL,\n"
- + " expires bigint DEFAULT NULL,\n"
- + " value varchar(255) NOT NULL,\n"
- + " version bigint NOT NULL,\n"
- + " PRIMARY KEY (context,id)\n"
- + ")";
-
+ + " context varchar(255) COLLATE Latin1_General_100_CS_AS NOT NULL,\n"
+ + " id varchar(255) COLLATE Latin1_General_100_CS_AS NOT NULL,\n"
+ + " expires bigint DEFAULT NULL,\n"
+ + " value varchar(255) NOT NULL,\n"
+ + " version bigint NOT NULL,\n"
+ + " PRIMARY KEY (context,id)\n"
+ + ")";
+
@Nonnull private static final String INIT_SQL_HSQLDB="CREATE TABLE StorageRecords (\r\n"
- + " context varchar(255) NOT NULL,\n"
- + " id varchar(255) NOT NULL,\n"
- + " expires bigint DEFAULT NULL,\n"
- + " value varchar(255) NOT NULL,\n"
- + " version bigint NOT NULL,\n"
- + " PRIMARY KEY (context,id)\n"
- + ")";
+ + " context varchar(255) NOT NULL,\n"
+ + " id varchar(255) NOT NULL,\n"
+ + " expires bigint DEFAULT NULL,\n"
+ + " value varchar(255) NOT NULL,\n"
+ + " version bigint NOT NULL,\n"
+ + " PRIMARY KEY (context,id)\n"
+ + ")";
@Nonnull private static final String CLEANUP_SQL = "DROP TABLE StorageRecords;";
/** Contexts used for testing. */
private Object[][] contexts;
-
+
@NonnullBeforeTest private BasicDataSource dataSource;
/** Constructor. */
- public JDBCStorageServiceTest() {
+ public JDBCStorageServiceTest() {
final SecureRandom random1 = new SecureRandom();
contexts = new Object[10][1];
for (int i = 0; i < 10; i++) {
@@ -108,9 +109,9 @@ public class JDBCStorageServiceTest extends StorageServiceTest {
try (final Connection dbConn = dataSource.getConnection()) {
final Statement statement = dbConn.createStatement();
try {
- statement.executeUpdate(CLEANUP_SQL);
+ statement.executeUpdate(CLEANUP_SQL);
} catch (final SQLException e) {
- System.out.println(e);
+ System.out.println(e);
}
statement.executeUpdate(INIT_SQL_SQLSERVER);
}
@@ -143,7 +144,7 @@ public class JDBCStorageServiceTest extends StorageServiceTest {
}
super.setUp();
}
-
+
@AfterClass
protected void tearDown() {
try {
@@ -154,7 +155,7 @@ public class JDBCStorageServiceTest extends StorageServiceTest {
}
List<?> recs = storageService.readAll();
Assert.assertEquals(recs.size(), 0);
- } catch (IOException e){
+ } catch (IOException e){
throw new RuntimeException(e);
}
super.tearDown();
@@ -350,7 +351,7 @@ public class JDBCStorageServiceTest extends StorageServiceTest {
@Test(enabled = false)
public void largeValue() throws IOException {
- // hsqldb defaults LOB length to 255 chars; disabled for now
+ // hsqldb defaults BLOB length to 255 chars; disabled for now
final StringBuilder sb = new StringBuilder(1000 * 36);
for (int i = 0; i < 1000; i++) {
sb.append(UUID.randomUUID());
@@ -362,5 +363,36 @@ public class JDBCStorageServiceTest extends StorageServiceTest {
assert rec !=null;
Assert.assertEquals(sb.toString(), rec.getValue());
}
-
+
+ @Test
+ public void veryLongContextAndKey() {
+
+ final StringBuilder builder = new StringBuilder();
+ Character foo = '\u1000';
+
+ for (int i = 0; i < 180; i++) {
+ builder.append(foo++);
+ }
+ final String longString = builder.toString();
+ assertTrue(longString.length() < 255);
+ assertTrue(longString.getBytes().length > 255);
+
+ final Long then = System.currentTimeMillis() + 300000;
+
+ try {
+ shared.create(longString, "key", longString, then);
+ fail("Long Context worked");
+ } catch (IOException e) {
+ // OK
+ }
+
+ try {
+ shared.create("context", longString, longString, then);
+ fail("Long Context worked");
+ } catch (IOException e) {
+ // OK
+ }
+
+ }
+
}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list