[internal/wclayer]: clear currentIsDir in reset deferred cleanup#2787
Merged
anmaxvl merged 1 commit intoJun 22, 2026
Merged
Conversation
When reset returns an error from inside the currentIsDir block (e.g. safefile.RemoveRelative or br.Next fails), the deferred cleanup closes currentFile and sets it to nil, but previously left currentIsDir true. A subsequent call to reset (e.g. from Close after a failed Add) would then enter the currentIsDir block, assign r := w.currentFile (nil), and panic at r.Seek. Fix: set w.currentIsDir = false in the deferred cleanup alongside the other currentFile fields so all file-related state is consistently cleared even on the error path. Adds a test covering this exact scenario: error inside the currentIsDir block, followed by a second reset call that must not panic. Assisted-by: Github-Copilot Signed-off-by: Maksim An <maksiman@microsoft.com>
helsaawy
approved these changes
Jun 22, 2026
msscotb
approved these changes
Jun 22, 2026
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
3 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.
When reset returns an error from inside the currentIsDir block (e.g.
safefile.RemoveRelative or br.Next fails), the deferred cleanup closes
currentFile and sets it to nil, but previously left currentIsDir true.
A subsequent call to reset (e.g. from Close after a failed Add) would
then enter the currentIsDir block, assign r := w.currentFile (nil),
and panic at r.Seek.
Fix: set w.currentIsDir = false in the deferred cleanup alongside the
other currentFile fields so all file-related state is consistently
cleared even on the error path.
Adds a test covering this exact scenario: error inside the currentIsDir
block, followed by a second reset call that must not panic.
Assisted-by: Github-Copilot