diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/JDBCStorage.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/JDBCStorage.java index d1d8055314..c94088c361 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/JDBCStorage.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/JDBCStorage.java @@ -1043,6 +1043,16 @@ public long getRecordCount(TreeName treeName) { } } } + /** + * A transaction able to write, unless the storage was opened read-only: then it may open an existing tree and + * read it, and every mutating operation throws {@link ReadOnlyStorageException} instead. + *

+ * The mode is checked per operation rather than refused here, because {@code RootContainer.open(AccessMode)} + * asks for a write transaction even in read-only mode - that is where it opens the compressed schema and the + * entry containers - so refusing to hand one out failed the offline {@code export-ldif}, {@code verify-index} + * and {@code backendstat} before they read anything (#874). Both other storages of this server already have + * this shape: {@code PDBStorage.ReadOnlyStorageImpl} and {@code CASStorage.TransactionImpl.checkReadOnly()}. + */ private final class WriteableTransactionTransactionImpl extends ReadableTransactionImpl implements WriteableTransaction { // Shared by every table this transaction stamps: opening a backend opens all its trees, @@ -1052,10 +1062,17 @@ private final class WriteableTransactionTransactionImpl extends ReadableTransact public WriteableTransactionTransactionImpl(Connection con) { super(con); - if (!accessMode.isWriteable()) { + //captured once rather than read per operation: the access mode of the storage is mutable state - + //ImporterImpl reopens the storage READ_WRITE under its caller - and a transaction has to keep the mode + //it was created with. It also drives isReadOnly, so that a cursor this transaction opens refuses + //delete() as well. + isReadOnly = !accessMode.isWriteable(); + } + + void checkReadOnly() { + if (isReadOnly) { throw new ReadOnlyStorageException(); } - isReadOnly = false; } boolean isExistsTable(TreeName treeName) { @@ -1096,6 +1113,7 @@ String getTableDialect() { @Override public void openTree(TreeName treeName, boolean createOnDemand) { if (createOnDemand) { + checkReadOnly(); if (!isExistsTable(treeName)) { try (final PreparedStatement statement=con.prepareStatement("create table "+getTableName(treeName)+" ("+getTableDialect()+")")){ execute(statement); @@ -1158,6 +1176,7 @@ boolean isExistsIndex(String tableName, String indexName) throws SQLException { } public void clearTree(TreeName treeName) { + checkReadOnly(); try (final PreparedStatement statement=con.prepareStatement("delete from "+getTableName(treeName))){ execute(statement); con.commit(); @@ -1168,6 +1187,7 @@ public void clearTree(TreeName treeName) { @Override public void deleteTree(TreeName treeName) { + checkReadOnly(); if (isExistsTable(treeName)) { try (final PreparedStatement statement = con.prepareStatement("drop table " + getTableName(treeName))) { execute(statement); @@ -1183,6 +1203,7 @@ public void deleteTree(TreeName treeName) { @Override public void put(TreeName treeName, ByteSequence key, ByteSequence value) { + checkReadOnly(); try { upsert(treeName, key, value); } catch (SQLException e) { @@ -1250,6 +1271,9 @@ boolean update(TreeName treeName, ByteSequence key, ByteSequence value) throws S @Override public boolean update(TreeName treeName, ByteSequence key, UpdateFunction f) { + //checked before the read, so that a read-only transaction reports the mode rather than the value it + //computed being equal to the stored one + checkReadOnly(); final ByteString oldValue=read(treeName,key); final ByteSequence newValue=f.computeNewValue(oldValue); if (Objects.equals(newValue, oldValue)) @@ -1266,6 +1290,7 @@ public boolean update(TreeName treeName, ByteSequence key, UpdateFunction f) { @Override public boolean delete(TreeName treeName, ByteSequence key) { + checkReadOnly(); try (final PreparedStatement statement=con.prepareStatement("delete from "+getTableName(treeName)+" where h="+hashParam(con)+" and k=?")){ statement.setString(1,key2hash.get(ByteBuffer.wrap(key.toByteArray()))); statement.setBytes(2,real2db(key.toByteArray())); diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VLVIndex.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VLVIndex.java index 757aa99e16..ca84641b19 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VLVIndex.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VLVIndex.java @@ -105,6 +105,7 @@ class VLVIndex extends AbstractTree implements ConfigurationChangeListener cursor = txn.openCursor(tree)) { + assertTrue(cursor.next()); + assertEquals(cursor.getKey(), key(0)); + try { + cursor.delete(); + fail("delete() through a cursor of a read-only transaction must fail"); + } catch (UnsupportedOperationException expected) {} + } + + assertReadOnly("openTree(createOnDemand)", () -> txn.openTree(absent, true)); + assertReadOnly("put", () -> txn.put(tree, key(2), value(2))); + assertReadOnly("update", () -> txn.update(tree, key(0), old -> value(3))); + assertReadOnly("delete", () -> txn.delete(tree, key(0))); + assertReadOnly("deleteTree", () -> txn.deleteTree(tree)); + } + }); + + // nothing above reached the database + storage.close(); + storage.open(AccessMode.READ_WRITE); + storage.read(new ReadOperation() { + @Override + public Void run(ReadableTransaction txn) throws Exception { + assertEquals(txn.getRecordCount(tree), 2); + assertEquals(txn.read(tree, key(0)), value(0)); + return null; + } + }); + } finally { + try { + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + txn.deleteTree(tree); + } + }); + } catch (Exception ignored) {} + storage.close(); + } + } + + private static void assertReadOnly(String operation, Runnable mutation) { + try { + mutation.run(); + fail(operation + " must fail on a read-only storage"); + } catch (ReadOnlyStorageException expected) {} + } + /** Buffer-served repositioning relies on the database collating keys in unsigned byte order. */ @Test public void testCursorKeyOrderIsUnsigned() throws Exception { diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/PluggableBackendImplTestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/PluggableBackendImplTestCase.java index eef3190788..6706f7b402 100644 --- a/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/PluggableBackendImplTestCase.java +++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/PluggableBackendImplTestCase.java @@ -1251,6 +1251,40 @@ public void run(WriteableTransaction txn) throws Exception } } + /** + * export-ldif, verify-index and backendstat open a root container of their own, in READ_ONLY mode, when the + * backend is not already open - a stopped server or a disabled backend. RootContainer.open() asks the storage + * for a write transaction even in that mode, since that is where it opens the compressed schema and the entry + * containers, so a storage refusing to hand one out fails the three tools before they read anything (#874). + *

+ * testReadOnly() above does not cover this: it expects a ReadOnlyStorageException and a storage that fails the + * open throws one too, from RootContainer.open() rather than from the write it is meant to be checking. + */ + @Test + public void testOfflineToolsOpenBackendReadOnly() throws Exception + { + // Put the backend offline, so that the tools open a read-only root container of their own + backend.finalizeBackend(); + try + { + final ByteArrayOutputStream exported = new ByteArrayOutputStream(); + try (final LDIFExportConfig exportConfig = new LDIFExportConfig(exported)) + { + backend.exportLDIF(exportConfig); + } + assertThat(exported.toString(StandardCharsets.UTF_8.name())).contains(testBaseDN.toString()); + + final VerifyConfig verifyConfig = new VerifyConfig(); + verifyConfig.setBaseDN(testBaseDN); + verifyConfig.addCompleteIndex("dn2id"); + assertThat(backend.verifyBackend(verifyConfig)).isEqualTo(0); + } + finally + { + backend.openBackend(); + } + } + @Test public void test_issue_496() throws Exception { int resultCode = TestCaseUtils.applyModifications(true,