Fix source error handling across reopen - #555
Closed
carrerasdarren-cell wants to merge 1 commit into
Closed
Conversation
dillof
added a commit
that referenced
this pull request
Jul 22, 2026
Do not query ZIP_SOURCE_ERROR after a successful close callback. Callback failures are already captured when they return -1; querying after success can resurrect an error from an earlier open cycle. Make the WinZip AES source report deferred integrity failures from close, clear its private error on reopen, and release its crypto context on close. Keep regression coverage for the lookahead reset now landed in 4cd7310 from nih-at#555, plus the still-distinct stale-error reopen case.
carrerasdarren-cell
force-pushed
the
fix/source-reopen-state
branch
from
July 23, 2026 22:30
42fae9e to
8f085c7
Compare
carrerasdarren-cell
changed the title
Member
|
Thanks for the bug report and test program. We found a solution that more closely resembles how errors are handled in other zip_source functions: Only propagate the error from the source if it returns -1. Your extended tests pass with our change. |
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
2 participants
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.
Follow-up to #552 and bf51731.
This PR originally covered two reopen regressions. Upstream commit 4cd7310 has now adopted the lookahead reset from this PR; this refresh rebases on current
mainand keeps regression coverage for it.The remaining functional changes are:
zip_source_close()no longer queriesZIP_SOURCE_ERRORafter a callback returned success. Callback failures are already captured when they return-1; querying after success can resurrect an error from an earlier open cycle.ZIP_SOURCE_CLOSE, clears its private error on reopen, and releases its crypto context on close. These changes preserve HMAC error propagation after removing the unconditional error query and avoid leaking the prior context across a partial-read reopen.Regression coverage includes the lookahead case fixed by 4cd7310 and the still-distinct stale-error case. On current unmodified
mainplus the regression test, the latter fails withclosing reopened source failed: Invalid argument; this patch passes it.Validation:
ctest --test-dir build -R "^exact-read-integrity\\.test$" --output-on-failure: passed.ctest --test-dir build --parallel 4 --output-on-failure: 187/187 passed.mainbuild 1.0.1028 fails in the same jobs for the same reason.