This branch is for debugging purposes only, and will be deleted once we identity the issue#102
This branch is for debugging purposes only, and will be deleted once we identity the issue#102rosygmiki wants to merge 4 commits into
Conversation
Signed-off-by: Rosy-Glorious Miki <rmiki@redhat.com>
…ion for broader compatibility Signed-off-by: Rosy-Glorious Miki <rmiki@redhat.com>
…tching the delta Signed-off-by: Rosy-Glorious Miki <rmiki@redhat.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #102 +/- ##
==========================================
- Coverage 79.19% 78.34% -0.85%
==========================================
Files 10 10
Lines 1115 1210 +95
==========================================
+ Hits 883 948 +65
- Misses 134 153 +19
- Partials 98 109 +11 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the Go version to 1.26, fixes an error-handling bug in SetCurrentFile, and introduces a validateRequiredFiles helper to pre-validate that all required source files exist before applying a patch. It also modernizes a loop in Apply using Go's range-over-integer feature. Feedback on these changes highlights three main areas of improvement in validateRequiredFiles: a potential Denial of Service (DoS) vulnerability due to unvalidated filename size allocation, a resource leak where the zstd.Reader is not closed on error paths, and a performance concern regarding the double decompression and scanning of the delta file.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| case protocol.DeltaOpOpen: | ||
| opCount++ | ||
| nameBytes := make([]byte, size) |
There was a problem hiding this comment.
The size of the filename is read directly from the delta file and used to allocate nameBytes without validation. A maliciously crafted delta file with an extremely large size could cause an out-of-memory (OOM) panic, leading to a Denial of Service (DoS). Please validate size against maxFilenameSize before allocating memory, similar to the check performed in Apply.
case protocol.DeltaOpOpen:
opCount++
if size > maxFilenameSize {
return fmt.Errorf("filename size %d exceeds maximum allowed %d", size, maxFilenameSize)
}
nameBytes := make([]byte, size)| decoder, err := zstd.NewReader(delta) | ||
| if err != nil { | ||
| fmt.Fprintf(os.Stderr, "[DEBUG] validateRequiredFiles: failed to create zstd decoder: %v\n", err) | ||
| return err | ||
| } |
There was a problem hiding this comment.
The zstd.Reader is not closed if an error occurs during the delta scanning loop, leading to a resource leak. Deferring decoder.Close() immediately after its successful creation ensures that resources are always released, even on error paths. Note that the manual call to decoder.Close() later in the function can then be removed.
| decoder, err := zstd.NewReader(delta) | |
| if err != nil { | |
| fmt.Fprintf(os.Stderr, "[DEBUG] validateRequiredFiles: failed to create zstd decoder: %v\n", err) | |
| return err | |
| } | |
| decoder, err := zstd.NewReader(delta) | |
| if err != nil { | |
| fmt.Fprintf(os.Stderr, "[DEBUG] validateRequiredFiles: failed to create zstd decoder: %v\n", err) | |
| return err | |
| } | |
| defer decoder.Close() |
| if seeker, ok := delta.(io.ReadSeeker); ok { | ||
| if fsDataSource, ok := dataSource.(*FilesystemDataSource); ok { | ||
| if err := validateRequiredFiles(seeker, fsDataSource); err != nil { | ||
| return err | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Calling validateRequiredFiles decompresses and scans the entire delta file to check for required files, and then Apply decompresses and scans it a second time to perform the actual patching. For large delta files, decompressing the zstd stream twice will double the CPU and I/O overhead. Consider if this pre-validation is strictly necessary, or if validation can be performed on-the-fly during the main patch application to avoid the performance penalty.
rosygmiki
force-pushed
the
DELETE_ME
branch
3 times, most recently
from
cbcc5a3 to
b9c5f8d
Compare
Signed-off-by: Rosy-Glorious Miki <rmiki@redhat.com>
1 participant
The missing source file test in
tests/test-tar-errors.shcontinues to fail on Windows, so this debug PR adds logging tovalidateRequiredFilesto identify where and why the validation is failing.