Fix OCSP test flakiness on repeated in-process runs - #200
Merged
Conversation
TOcspTest cached its generated certificate chain and reference instant in instance fields behind a nil guard, and SetUp never cleared them. DUnit reuses a test-case instance for the life of the process, so a second run reused the first run's chain: its CRLs pin thisUpdate/nextUpdate to the original instant (a ten-minute window) while the path is validated against the live clock, so once the runs were more than a few minutes apart the cached CRLs were stale and the "clean CRLs accept the path" cases flipped to rejection, with the blame index shifting to the intermediate once the cached certificates also expired. Clear the cached chain fields in SetUp so EnsureChain rebuilds a fresh, time-consistent chain on every run.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
TOcspTestpasses on the first run but fails five tests on a second run in thesame process (as seen in the Delphi/DUnit GUI, running the suite twice without
closing the app):
Root cause
A test-isolation bug, not a library defect.
TOcspTestcaches its generatedroot -> intermediate -> end-entity chain and a single reference instant
(
FChainUtcNow) in instance fields behind aif FRootCert <> nil then Exitguard, and
SetUpnever clears them (there is noTearDown).DUnit builds the test-case instances once and reuses them for the life of the
process, so the second run finds the first run's chain still cached and skips
rebuilding. The CRLs are pinned to
FChainUtcNow(thisUpdate = -1 min,nextUpdate = +10 min), and the certificates to anow-5s .. now+30minwindow,but the path is validated against the live clock (
PkixParameters.Dateis neverset). Once the two runs are more than a few minutes apart, the cached CRLs'
nextUpdateis already in the past, so the "clean CRLs accept the path" casesflip to rejection; once past 30 minutes the cached certificates expire too, so
validation fails at the intermediate first, which is why the default-checker
test reports blame index 1 instead of 0.
The FPC console runner never reproduced this because it runs each suite in a
fresh process (fresh instances).
Fix
Clear the cached chain fields in
SetUpsoEnsureChainrebuilds a fresh,time-consistent chain on every run.