Skip to content

Unguarded Throwable.addSuppressed() calls risk self-suppression IllegalArgumentException, masking the real failure #982

Description

@jeffjensen

Throwable.addSuppressed(Throwable) throws IllegalArgumentException: Self-suppression not permitted when given the exact same instance as the receiver - documented JDK behavior. Several places in dbUnit call addSuppressed without guarding against that case, so if the "secondary" failure being attached ever turns out to be the identical instance as the "primary" one already being handled, the unguarded call replaces the real failure with an unrelated IllegalArgumentException and masks it.

This was found and fixed for real, reachable input in MultiDataSourcePrepAndExpectedTestCase.tearDownSuppressing() (#981, tracked under #968) - confirmed via a regression test that a delegate's own teardown/rollback failure can legitimately be the exact same instance as the failure it is already being attached to.

The identical unguarded pattern also exists at:

  • DefaultPrepAndExpectedTestCase.java:548 (e.addSuppressed(cleanupFailure))
  • DefaultPrepAndExpectedTestCase.java:621 (cleanupFailure.addSuppressed(verifyFailure))
  • DatabaseTestCase.java:245 (testFailure.addSuppressed(tearDownFailure))
  • database/connection/TestScopedConnection.java:233 (primary.addSuppressed(closeFailure))
  • annotation/runtime/TesterStateSnapshot.java:114 (primary.addSuppressed(restoreFailure))
  • annotation/runtime/AnnotatedTestExecutor.java:401 (primaryFailure.addSuppressed(closeFailure))
  • operation/TransactionOperation.java:126 (e.addSuppressed(rollbackFailure))

Two more call sites turned up in a grep for addSuppressed that may also be worth a look, though reachability was not traced: operation/RefreshOperation.java:161 and :172.

Suggested fix at each site, matching what landed in MultiDataSourcePrepAndExpectedTestCase:

if (secondaryFailure != primaryFailure)
{
    primaryFailure.addSuppressed(secondaryFailure);
}

Each site needs its own look at whether, and how, the two sides can legitimately end up being the same instance - the mechanism differs per class (e.g. a delegate rethrowing the identical exception object it was already handling).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

area: assertionDbUnitAssert, ValueComparer, Difference, DifferenceListener

Type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions