[#1074] Keep what says the connection is gone through the redaction of a co...

Valery Kharseko

12 hours ago 3fe8fc5280bcb0ced976a698fcae2dc977450ce6

[#1074] Keep what says the connection is gone through the redaction of a connect failure (#1107)

 opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/CachedConnection.java

@@ -258,7 +258,8 @@
     * How many links of the cause and getNextException() chains of a failure the two walks that
     * have a reason to stop look at: {@link #holdsCredentials} answers "yes" past it, since the
     * cost of the other answer is the password of the backend in the log, and
     * {@link #redactedCopy} rebuilds that far and names the rest in one link. The walk whose verdict
     * {@link #redactedCopy} rebuilds that far and names the rest in one link - carrying, where the
     * rest says the connection is gone, what says it (#1074). The walk whose verdict
     * decides something, {@link #isWorthRetrying}, is not one of them: a count does not leave that
     * question unanswered, it answers it with the verdict of a failure that carries nothing
     * (issue #1076), and the visited set of every walk here terminates it on its own.
@@ -2135,8 +2136,11 @@
     * prints a failure prints the causes along with it - a debug build of
     * stackTraceToSingleLineString walks them, the config manager traces them, and
     * RootContainer.open() makes the message of the cause the message of what it throws - so a
     * link left as it stands would carry the password past the wrapper. The SQLState and the
     * vendor code of every link survive it: they are what tells a caller what happened.
     * link left as it stands would carry the password past the wrapper. The SQLState, the vendor
     * code and the standard JDBC type of every link survive it: they are what tells a caller what
     * happened, and JDBCStorage.write() asks the type before the state whether the connection is
     * gone. Past the budget of the rebuild the one link standing for the rest says that much of it
     * where the rest says so (#1074).
     * <p>
     * The whole chain is the causes, the further exceptions of {@code getNextException()} and what
     * was suppressed on each of them - the last being where this class puts the failure of a close
@@ -2223,40 +2227,110 @@
    private static SQLException redactedCopy(SQLException e, String connectionString, int[] budget) {
        budget[0]--;
        final SQLException copy =
            new SQLException(redact(e.getMessage(), connectionString), e.getSQLState(), e.getErrorCode());
            sameKind(e, redact(e.getMessage(), connectionString), e.getSQLState(), e.getErrorCode());
        copy.setStackTrace(e.getStackTrace());
        if (e.getNextException() != null) {
            copy.setNextException(budget[0] > 0
                ? redactedCopy(e.getNextException(), connectionString, budget) : droppedTail());
                ? redactedCopy(e.getNextException(), connectionString, budget)
                : droppedTail(Collections.<Throwable>singletonList(e.getNextException())));
        }
        if (e.getCause() != null) {
            copy.initCause(budget[0] > 0 ? redactedLink(e.getCause(), connectionString, budget) : droppedTail());
            copy.initCause(budget[0] > 0 ? redactedLink(e.getCause(), connectionString, budget)
                : droppedTail(Collections.singletonList(e.getCause())));
        }
        copySuppressed(e, copy, connectionString, budget);
        return copy;
    }

    /**
     * A new exception of the kind JDBC names the given one as. Not the class of the driver itself,
     * which this cannot be sure of building, but the standard type it extends: that type is what
     * the JDBC contract gives a driver to say what happened beside the SQLState, and JDBCStorage
     * reads it first - a SQLRecoverableException says the connection is gone whatever state it
     * carries, and a copy made as a plain SQLException says nothing of the kind (#1074).
     */
    private static SQLException sameKind(SQLException e, String message, String sqlState, int vendorCode) {
        if (e instanceof SQLRecoverableException) {
            return new SQLRecoverableException(message, sqlState, vendorCode);
        }
        if (e instanceof SQLNonTransientConnectionException) {
            return new SQLNonTransientConnectionException(message, sqlState, vendorCode);
        }
        if (e instanceof SQLTransientConnectionException) {
            return new SQLTransientConnectionException(message, sqlState, vendorCode);
        }
        if (e instanceof SQLTimeoutException) {
            return new SQLTimeoutException(message, sqlState, vendorCode);
        }
        if (e instanceof SQLTransactionRollbackException) {
            return new SQLTransactionRollbackException(message, sqlState, vendorCode);
        }
        if (e instanceof SQLFeatureNotSupportedException) {
            return new SQLFeatureNotSupportedException(message, sqlState, vendorCode);
        }
        if (e instanceof SQLIntegrityConstraintViolationException) {
            return new SQLIntegrityConstraintViolationException(message, sqlState, vendorCode);
        }
        if (e instanceof SQLInvalidAuthorizationSpecException) {
            return new SQLInvalidAuthorizationSpecException(message, sqlState, vendorCode);
        }
        if (e instanceof SQLSyntaxErrorException) {
            return new SQLSyntaxErrorException(message, sqlState, vendorCode);
        }
        if (e instanceof SQLDataException) {
            return new SQLDataException(message, sqlState, vendorCode);
        }
        if (e instanceof SQLTransientException) {
            return new SQLTransientException(message, sqlState, vendorCode);
        }
        if (e instanceof SQLNonTransientException) {
            return new SQLNonTransientException(message, sqlState, vendorCode);
        }
        return new SQLException(message, sqlState, vendorCode);
    }

    // The suppressed links of a failure are rebuilt with the rest of it and counted against the
    // same budget. They are not decoration here: the failure of a close that could not be made
    // rides on the failure being unwound (establish(), #929), and a rebuild that dropped them
    // would lose it at the one deployment whose log is redacted - which is the deployment whose
    // url has a password in it, the log that is worth reading.
    private static void copySuppressed(Throwable from, Throwable to, String connectionString, int[] budget) {
        for (final Throwable suppressed : from.getSuppressed()) {
        final Throwable[] suppressed = from.getSuppressed();
        for (int i = 0; i < suppressed.length; i++) {
            if (budget[0] <= 0) {
                to.addSuppressed(droppedTail());
                to.addSuppressed(droppedTail(Arrays.asList(suppressed).subList(i, suppressed.length)));
                return;
            }
            to.addSuppressed(redactedLink(suppressed, connectionString, budget));
            to.addSuppressed(redactedLink(suppressed[i], connectionString, budget));
        }
    }

    // What stands where the budget ran out. Without it the same failure logs its root cause when
    // the url of the backend has no password in it and loses it without a word when it has, which
    // is a report of a connect nobody can read against a report of one they can.
    private static SQLException droppedTail() {
        return new SQLException("the rest of this failure was left out: a chain of more than "
            + MAX_CHAIN_LENGTH + " links is rebuilt only that far");
    // It also says what the rest says about the connection, where the rest says it is gone: that
    // is a verdict JDBCStorage.write() reads off every link of the failure - it replays the attempt
    // and distrusts the pool on it - and a tail saying nothing would answer it with the "no" of a
    // failure that carries nothing (#1074, the #961 thesis on the budget of this rebuild). Only
    // that, and only where the rest does say it, so that the cut makes no failure say it either.
    // The rest is read by the very walk isConnectionFailure() answers from, to its end, rather than
    // by a copy of it here: the two would read the same edges today and drift the day one of them
    // changes. It builds nothing, so what it costs is a walk of the links the driver has already
    // allocated.
    private static SQLException droppedTail(List<Throwable> rest) {
        final String message = "the rest of this failure was left out: a chain of more than "
            + MAX_CHAIN_LENGTH + " links is rebuilt only that far";
        SQLException gone = null;
        for (final Throwable t : rest) {
            gone = JDBCStorage.connectionFailureLink(t);
            if (gone != null) {
                break;
            }
        }
        return gone == null
            ? new SQLException(message)
            : sameKind(gone, message + "; a link of it says the connection is gone", gone.getSQLState(),
                gone.getErrorCode());
    }

    // A link that is no SQLException keeps its class name in the message: its type is not one this
@@ -2270,7 +2344,8 @@
            + (t.getMessage() == null ? "" : ": " + redact(t.getMessage(), connectionString)));
        copy.setStackTrace(t.getStackTrace());
        if (t.getCause() != null) {
            copy.initCause(budget[0] > 0 ? redactedLink(t.getCause(), connectionString, budget) : droppedTail());
            copy.initCause(budget[0] > 0 ? redactedLink(t.getCause(), connectionString, budget)
                : droppedTail(Collections.singletonList(t.getCause())));
        }
        copySuppressed(t, copy, connectionString, budget);
        return copy;

 opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/JDBCStorage.java

@@ -4842,7 +4842,20 @@
     * failover took, and {@link #replayReason} fails an attempt that was worth replaying (issue #961).
     */
    static boolean isConnectionFailure(Throwable failure) {
        return firstLinkMatching(failure, WITH_THE_RELEASE, EVERY_LINK, JDBCStorage::saysTheConnectionIsGone)!=null;
        return connectionFailureLink(failure)!=null;
    }

    /**
     * The link of a failure that says the connection is gone, found by the walk {@link #isConnectionFailure} answers
     * from, or null where none says so.
     * <p>
     * Package-private for the one other reader it has, the redaction of a connect failure in {@link
     * CachedConnection}: what that rebuild leaves in place of a chain past its budget has to say what this reads
     * off the links it cuts (#1074). It asks for the walk rather than for the question alone, so that a change to
     * the edges this walks changes what the tail carries along with what {@link #write} reads.
     */
    static SQLException connectionFailureLink(Throwable failure) {
        return firstLinkMatching(failure, WITH_THE_RELEASE, EVERY_LINK, JDBCStorage::saysTheConnectionIsGone);
    }

    /**
@@ -7071,8 +7084,9 @@
         * database that its dialect table does not recognize is raised at once instead: a mysql
         * account with a {@code MAX_USER_CONNECTIONS} of its own answers 1226 on SQLState 42000, and
         * a driver of no known dialect has no vendor code read at all (issue #1011). The type is no
         * rule either: a failure whose chain names the credentials of the backend is rebuilt as a
         * plain {@code SQLException} whatever the driver threw. Sorting them here would be that
         * rule either: a driver need not raise the standard JDBC type that names what happened, and a
         * failure whose chain names the credentials of the backend keeps only that standard type, not
         * the class of the driver (#1074). Sorting them here would be that
         * classification written out a second time, with the failure of a multi-hour import as the
         * cost of getting it wrong (issue #1013).
         * <p>

 opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/CachedConnectionTestCase.java

@@ -39,8 +39,19 @@
import java.sql.DriverManager;
import java.sql.DriverPropertyInfo;
import java.sql.PreparedStatement;
import java.sql.SQLDataException;
import java.sql.SQLException;
import java.sql.SQLFeatureNotSupportedException;
import java.sql.SQLIntegrityConstraintViolationException;
import java.sql.SQLInvalidAuthorizationSpecException;
import java.sql.SQLNonTransientConnectionException;
import java.sql.SQLNonTransientException;
import java.sql.SQLRecoverableException;
import java.sql.SQLSyntaxErrorException;
import java.sql.SQLTimeoutException;
import java.sql.SQLTransactionRollbackException;
import java.sql.SQLTransientConnectionException;
import java.sql.SQLTransientException;
import java.util.ArrayDeque;
import java.util.Collections;
import java.util.Deque;
@@ -942,6 +953,157 @@
    }

    /**
     * A link that says the connection is gone by its type alone still says so once it is rebuilt
     * (#1074). The type is what the JDBC contract gives a driver to say it - write() asks it before
     * the SQLState - so a copy made as a plain SQLException reads as a failure that says nothing of
     * the connection, and only at the deployment whose url has a password in it.
     */
    @Test(timeOut = 60000)
    public void testALinkThatSaysTheConnectionIsGoneByItsTypeKeepsItThroughRedaction() throws Exception {
        final String url = "jdbc:postgresql://opendj:S3cretOfTheBackend@127.0.0.1:5432/opendj";
        for (final SQLException gone : new SQLException[] { new SQLRecoverableException("io error"),
                new SQLNonTransientConnectionException("closed"), new SQLTransientConnectionException("reset") }) {
            final SQLException failure = new SQLException("login to " + url + " failed", "S0001", 18456);
            failure.setNextException(gone);

            final SQLException reported = CachedConnection.reported(failure, url);

            assertNoCredentials(reported);
            assertTrue(JDBCStorage.isConnectionFailure(reported),
                "a rebuilt " + gone.getClass().getSimpleName() + " no longer says the connection is gone");
        }
    }

    /**
     * ... and so does one standing past the budget of the rebuild, whatever it says it by: the link
     * that stands for the rest of the chain there is all that the classification of write() gets to
     * read of it. The url has no password, and the chain is rebuilt all the same - it is longer than
     * the walk looking for credentials, which answers "yes" past it.
     */
    @Test(timeOut = 60000)
    public void testALinkThatSaysTheConnectionIsGonePastTheBudgetOfARebuildStillSaysSo() throws Exception {
        final String url = "jdbc:postgresql://127.0.0.1:5432/opendj";
        for (final SQLException gone : new SQLException[] { new SQLException("connection reset", "08S01", 10054),
                new SQLRecoverableException("io error") }) {
            final SQLException failure = plainChain(40);
            failure.setNextException(gone);

            final SQLException reported = CachedConnection.reported(failure, url);

            assertTrue(reported != failure, "a chain this long is expected to be rebuilt");
            assertTrue(JDBCStorage.isConnectionFailure(reported),
                gone + " past the budget of the rebuild no longer says the connection is gone");
        }
    }

    /**
     * ... and so does one past the budget on any other edge the rebuild cuts: the cause of a link, what
     * was suppressed on it - where establish() puts the close that failed (#929) - and the cause of a
     * link that is no SQLException, which is rebuilt on a road of its own. Each chain spends the budget
     * before the rebuild reaches the edge, so the link that says the connection is gone is behind the
     * one that stands for the rest; the last one has it on what was suppressed on a link that is cut,
     * which the rest is read for as well.
     */
    @Test(timeOut = 60000)
    public void testALinkThatSaysTheConnectionIsGoneOnACauseOrASuppressedEdgePastTheBudgetStillSaysSo()
            throws Exception {
        final String url = "jdbc:postgresql://127.0.0.1:5432/opendj";
        final SQLException byCause = plainChain(40);
        byCause.initCause(new IOException("socket closed", new SQLException("connection reset", "08S01", 10054)));
        final SQLException bySuppressed = plainChain(40);
        bySuppressed.addSuppressed(new SQLException("the connection is closed", "08003"));
        final SQLException byTheCauseOfALinkThatIsNoSQLException = new SQLException("login failed", "S0001", 18456);
        Throwable wrapper = new SQLException("connection reset", "08S01", 10054);
        for (int i = 0; i < 40; i++) {
            wrapper = new IOException("wrapper " + i, wrapper);
        }
        byTheCauseOfALinkThatIsNoSQLException.initCause(wrapper);
        final SQLException byTheSuppressedOfALinkCut = plainChain(40);
        lastOf(byTheSuppressedOfALinkCut).addSuppressed(new SQLException("the connection is closed", "08003"));
        final Object[][] cases = {
            { "the cause", byCause },
            { "the suppressed", bySuppressed },
            { "the cause of a link that is no SQLException", byTheCauseOfALinkThatIsNoSQLException },
            { "what was suppressed on a link cut", byTheSuppressedOfALinkCut } };
        for (final Object[] edge : cases) {
            final SQLException failure = (SQLException) edge[1];
            assertTrue(JDBCStorage.isConnectionFailure(failure), edge[0] + ": the failure itself says the connection is gone");

            final SQLException reported = CachedConnection.reported(failure, url);

            assertNotSame(reported, failure, edge[0] + ": a chain this long is expected to be rebuilt");
            assertTrue(JDBCStorage.isConnectionFailure(reported),
                edge[0] + " cut past the budget no longer says the connection is gone");
        }
    }

    /**
     * Every standard type of JDBC a link can be of survives its rebuild, not only the three that say
     * the connection is gone: each is read somewhere - a SQLTimeoutException is what sends a borrow of
     * an import to the debug log rather than the warn one (borrowedOrShared()) - and a rebuild that
     * turned one into its supertype would classify the same failure one way where the url of the
     * backend has no password and the other where it does. Each type is asked for before its
     * supertype, so a check out of order fails here as well.
     */
    @Test(timeOut = 60000)
    public void testEveryStandardTypeOfALinkIsKeptThroughRedaction() throws Exception {
        final String url = "jdbc:postgresql://opendj:S3cretOfTheBackend@127.0.0.1:5432/opendj";
        final String message = "login to " + url + " failed";
        for (final SQLException original : new SQLException[] {
                new SQLRecoverableException(message, "08006", 17002),
                new SQLNonTransientConnectionException(message, "08001", 0),
                new SQLTransientConnectionException(message, "08001", 0),
                new SQLTimeoutException(message, "HYT00", 0),
                new SQLTransactionRollbackException(message, "40001", 1205),
                new SQLFeatureNotSupportedException(message, "0A000", 0),
                new SQLIntegrityConstraintViolationException(message, "23000", 2627),
                new SQLInvalidAuthorizationSpecException(message, "28000", 18456),
                new SQLSyntaxErrorException(message, "42000", 0),
                new SQLDataException(message, "22000", 0),
                new SQLTransientException(message, "S1000", 0),
                new SQLNonTransientException(message, "S1000", 0),
                new SQLException(message, "S1000", 0) }) {
            final SQLException reported = CachedConnection.reported(original, url);

            assertNoCredentials(reported);
            assertEquals(reported.getClass(), original.getClass(), "the rebuild changed the type of the link");
            assertEquals(reported.getSQLState(), original.getSQLState(), "the rebuild changed the SQLState of the link");
            assertEquals(reported.getErrorCode(), original.getErrorCode(), "the rebuild changed the vendor code of the link");
        }
    }

    /**
     * The link standing for the rest says only what the rest says: a chain that says nothing of the
     * connection is not made to say it is gone by being cut. That would replay the write and distrust
     * the pool over a database that refused a connection for a reason of its own.
     */
    @Test(timeOut = 60000)
    public void testALongChainThatSaysNothingOfTheConnectionIsNotMadeToSayItByTheRebuild() throws Exception {
        final SQLException reported = CachedConnection.reported(plainChain(40), "jdbc:postgresql://127.0.0.1:5432/opendj");

        assertFalse(JDBCStorage.isConnectionFailure(reported),
            "the rebuild made a failure that says nothing of the connection say it is gone");
    }

    /** That many plain links of the next exception chain, none of which says the connection is gone. */
    private static SQLException plainChain(int links) {
        final SQLException head = new SQLException("error 0", "S0001", 1);
        for (int i = 1; i < links; i++) {
            head.setNextException(new SQLException("error " + i, "S0001", 1));
        }
        return head;
    }

    /** The last link of the next exception chain of a failure. */
    private static SQLException lastOf(SQLException failure) {
        SQLException last = failure;
        while (last.getNextException() != null) {
            last = last.getNextException();
        }
        return last;
    }

    /**
     * A credential named by a suppressed link alone is redacted like any other. The close of a
     * connection whose set-up failed rides there (establish(), #929) and an interrupt that ended a
     * wait for a catalog connect does, and a driver names the url it could not close as readily as

 opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/CatalogConnectionTestCase.java

@@ -685,6 +685,69 @@
        verify(failing).close();
    }

    /**
     * A refusal that says the connection is gone reaches {@code write()} saying so, however deep in its
     * chain the driver put the link that says it (#1074). The failure leaves this connect redacted, and
     * a chain longer than the walk looking for credentials is rebuilt whether or not anything in it names
     * them - this url has no password at all - so what the rebuild leaves past its budget is what {@code
     * isConnectionFailure()} reads: the attempt is replayed where the same refusal a few links shorter is.
     */
    @Test
    public void testARefusalThatSaysTheConnectionIsGoneDeepInItsChainStillSaysSo() throws Exception {
        final SQLException refusal = deepChain(40, new SQLException("connection reset", "08S01", 10054));
        probeDriver.refusal = refusal;
        probeDriver.refusalsLeft.set(Integer.MAX_VALUE);
        try {
            storageFor(ProbeDriver.URL).newCatalogConnection(NO_REPLAY_WINDOW, DEFAULT_BOUND, 60);
            fail("a connect that will not clear was retried instead of being reported");
        } catch (SQLException expected) {
            // the road this case is about: the timeout of a refusal waited out carries the same tail
            // as its cause and would pass the rest of this case just as well
            assertFalse(expected instanceof SQLTimeoutException,
                "the refusal was retried to the deadline instead of being reported at once");
            assertEquals(probeDriver.attempts.get(), 1, "the refusal was retried instead of being reported at once");
            assertTrue(JDBCStorage.isConnectionFailure(refusal), "the refusal of the driver says the connection is gone");
            assertTrue(JDBCStorage.isConnectionFailure(expected),
                "the refusal as it left the connect lost what said the connection is gone: " + expected);
        }
    }

    /**
     * ... and so does the timeout a refusal worth waiting out ends in, whose cause is that refusal
     * redacted the same way: the retry changes no classification.
     */
    @Test
    public void testATimedOutRefusalThatSaysTheConnectionIsGoneDeepInItsChainStillSaysSo() throws Exception {
        final SQLException refusal = new SQLException("too many clients already", "53300");
        refusal.setNextException(deepChain(40, new SQLException("connection reset", "08S01", 10054)));
        probeDriver.refusal = refusal;
        probeDriver.refusalsLeft.set(Integer.MAX_VALUE);
        try {
            storageFor(ProbeDriver.URL).newCatalogConnection(NO_REPLAY_WINDOW, DEFAULT_BOUND, 1);
            fail("a connect refused for the whole deadline was not given up on");
        } catch (SQLTimeoutException expected) {
            assertTrue(JDBCStorage.isConnectionFailure(refusal), "the refusal of the driver says the connection is gone");
            assertTrue(JDBCStorage.isConnectionFailure(expected),
                "the timeout lost what its last refusal said about the connection: " + expected);
        }
    }

    /**
     * A driver's failure the way mssql-jdbc chains every error of a message it received: that many
     * plain links of the next exception chain, and the given one last.
     */
    private static SQLException deepChain(int links, SQLException last) {
        final SQLException head = new SQLException("error 0 of the login", "S0001", 1);
        SQLException tail = head;
        for (int i = 1; i < links; i++) {
            final SQLException next = new SQLException("error " + i + " of the login", "S0001", 1);
            tail.setNextException(next);
            tail = next;
        }
        tail.setNextException(last);
        return head;
    }

    /** What the dialect of this url declares, at the given number of seconds. */
    private static void assertBoundedAt(Properties handed, long seconds) {
        assertNotNull(handed, "no properties were handed to the driver at all");