fix(command-runner): grow command string buffer instead of assuming capacity - #29
Open
ratazzi wants to merge 1 commit into
Open
fix(command-runner): grow command string buffer instead of assuming capacity#29ratazzi wants to merge 1 commit into
ratazzi wants to merge 1 commit into
Conversation
…apacity The display/logging command string was built with appendAssumeCapacity on a 256-byte buffer (capacity 0 if the initial allocation failed). Any argv longer than 256 bytes -- e.g. a routine 'apt-get install' or 'brew bundle' invocation with a dozen packages -- overflowed the buffer: an assertion panic in Debug/ReleaseSafe and heap corruption in ReleaseFast. Use the allocating append/appendSlice variants so the buffer grows as needed, extract the string building into joinArgs, and cover it with tests including an argv well past the initial capacity.
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
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.
Problem
executeCommandWithLoggingbuilt its display/logging command string withappendAssumeCapacity/appendSliceAssumeCapacityon a buffer allocated withinitCapacity(allocator, 256)(falling back to capacity 0 on allocation failure). The AssumeCapacity variants never grow the buffer, so any argv longer than 256 bytes wrote out of bounds.This triggers on routine paths:
apt_bootstrap.zigpassessudo <apt> install -y pkg1 pkg2 ...with the full package list, and thebrew bundlepath inapply.zigdoes the same. A dozen packages easily exceeds 256 bytes.Impact: assertion panic in Debug/ReleaseSafe; heap corruption in ReleaseFast.
Fix
append/appendSlicevariants so the buffer grows as needed.joinArgshelper and cover it with tests, including an argv well past the initial capacity.command_runner.zigto the test-discoverycomptimeblock inmain.zig(its tests were not being collected before).Testing
zig build test— 11 tests pass (2 new).