Skip to content

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

Open
vharseko wants to merge 1 commit into
OpenIdentityPlatform:masterfrom
vharseko:issue-1074-redacted-chain-keeps-verdict
Open

vharseko wants to merge 1 commit into
OpenIdentityPlatform:masterfrom
vharseko:issue-1074-redacted-chain-keeps-verdict

Conversation

@vharseko

Copy link
Copy Markdown
Member

Fixes #1074

The problem

CachedConnection.reported() rebuilds a connect failure link by link, and JDBCStorage.write() reads the rebuilt chain through isConnectionFailure() whenever the catalog connect fails inside a write (newCatalogConnection(): the direct throw, the interrupted wait, and catalogConnectTimedOut(), whose cause is reported(last)). Two things in that rebuild answered the question with the false of a failure that says nothing about the connection:

  1. The tail past the budget. Where redactedCopy() runs out of its 32 links it puts droppedTail() in place of the rest - a bare SQLException with no SQLState, which saysTheConnectionIsGone() can never accept. A class 08 link past the 32nd was lost.
  2. The type of every link, inside the budget too. Each copy was built as new SQLException(msg, state, code), so a SQLRecoverableException / SQLNonTransientConnectionException / SQLTransientConnectionException that says the connection is gone by its type alone - which saysTheConnectionIsGone() asks before the SQLState - lost it. The reviewer of [#961] Walk every link of a failure where the verdict decides something #1004 noted this in the thought that became JDBC backend: reported() drops the SQLState of a connection-is-gone link past MAX_CHAIN_LENGTH, so a redacted catalog-connect failure reads false through isConnectionFailure() #1074; the issue text kept only the tail.

Two corrections to the issue text, both pinned by the tests below:

  • No credential is needed. holdsCredentials() answers "yes" past its own 32-link walk, so every chain longer than that is rebuilt whether the url has a password or not. The case the issue needs - a deciding link past 32 others - is therefore rebuilt on every deployment; the "only where the failure names the credentials" narrowing does not hold. (I made the same mistake in my reply on [#961] Walk every link of a failure where the verdict decides something #1004.)
  • The type loss needs no long chain, only a password in a message: the same refusal classified one way where the url has no password and the other where it does.

What was lost: replayReason() did not replay the attempt as "a connection the database dropped", and the pool was not told - where the same refusal a few links shorter, or on a url without a password, is.

The fix

  • sameKind(): every rebuilt link keeps the standard JDBC type it extends (the three connection types, SQLTimeoutException, SQLTransactionRollbackException, SQLFeatureNotSupportedException, the other standard SQLNonTransient* / SQLTransient* subtypes, else SQLException). Not the driver's own class, which this cannot be sure of building.
  • droppedTail(rest): walks the links it stands for to their end - by the edges isConnectionFailure() walks (cause, next exception, suppressed), identity-visited, building nothing - and where one of them says the connection is gone, the tail takes that link's type, SQLState and vendor code. Where none does, the tail stays the bare SQLException it was, so the cut makes no failure a drop either (the concern catalogConnectTimedOut()'s javadoc spells out about manufacturing class 08).
  • JDBCStorage.saysTheConnectionIsGone() becomes package-private so that the rebuild asks the very question the classifier asks, instead of a second copy of it.

Kept as they were: the 32-link budget of the rebuild and of holdsCredentials(), and the "left out" tail message (testTheTailOfALongChainIsNamedRatherThanDropped is untouched and green). A memoised rebuild with no budget was considered and not taken: it recurses as deep as the chain, and the budget is also what bounds that.

Tests

CachedConnectionTestCase:

  • testALinkThatSaysTheConnectionIsGoneByItsTypeKeepsItThroughRedaction - each of the three connection types, no SQLState, behind a link naming the password.
  • testALinkThatSaysTheConnectionIsGonePastTheBudgetOfARebuildStillSaysSo - 08S01, and a stateless SQLRecoverableException, as the 41st link; url without a password.
  • testALongChainThatSaysNothingOfTheConnectionIsNotMadeToSayItByTheRebuild - guard: 40 plain links stay false.

CatalogConnectionTestCase (end to end through newCatalogConnection()):

  • testARefusalThatSaysTheConnectionIsGoneDeepInItsChainStillSaysSo - direct throw.
  • testATimedOutRefusalThatSaysTheConnectionIsGoneDeepInItsChainStillSaysSo - 53300 retried to the deadline, 08S01 deep in its next chain.

Before the fix: 4 of the 5 red (the guard green, as a guard should be). After: CachedConnectionTestCase 120/120, CatalogConnectionTestCase 20/20, JDBCStorageRetryTest 113/113, StampConnectionTestCase 6/6, Skipped: 0.

Mutants, each red on its own case:

mutant red
every copy a plain SQLException again …ByItsTypeKeepsItThroughRedaction
the tail says nothing of the rest (the old behaviour) …PastTheBudgetOfARebuildStillSaysSo + both CatalogConnectionTestCase cases
the tail always says the connection is gone …IsNotMadeToSayItByTheRebuild
the tail keeps the SQLState but not the type …PastTheBudgetOfARebuildStillSaysSo

…ough the redaction of a connect failure

reported() rebuilds a connect failure link by link wherever it may carry the
credentials of the backend - and on every chain longer than the walk looking
for them, which answers "yes" past its 32 links whether the url has a password
or not. The rebuild made every link a plain SQLException and put a bare one
with no SQLState where its own budget ran out, so a link saying the connection
is gone by its type, or standing past the 32nd, read false through
isConnectionFailure(): write() neither replayed the attempt nor distrusted the
pool over a catalog connect refused the way a shorter chain is replayed.

Every rebuilt link now keeps the standard JDBC type it extends, and the link
standing for what the budget cut carries the type, SQLState and vendor code of
the first link of the rest that says the connection is gone - and says nothing
of it where the rest does not, so the cut makes no failure a drop either.

Fixes OpenIdentityPlatform#1074
@vharseko vharseko added bug jdbc java Changes to Java sources tests Test suites: fixing, enabling, un-disabling labels Sep 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug java Changes to Java sources jdbc tests Test suites: fixing, enabling, un-disabling

Projects

None yet

1 participant