Repository navigation
Fix unsigned underflow in BrotliBinaryDiff::Diff() reserve size. - #274
Merged
Merged
Conversation
When derived font is smaller than base font, the unsigned subtraction wraps around 2^32, requesting a ~4 GiB allocation. This fails on memory-constrained environments like Cloud Run (4 GiB) with std::bad_alloc. Fix: reserve only when derived size exceeds base size. Promote sizes to size_t to avoid wrap-around during multiplication. Add regression test: DiffAndPatchWithSmallerDerived exercises the smaller-than-base code path. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
garretrieger
approved these changes
Oct 9, 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
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
BrotliBinaryDiff::Diff()reserves the output buffer based on the size difference:sink.reserve(2 * (font_derived.size() - font_base.size()));FontData::size()returnsunsigned int, so when the derived data is smaller than the base (e.g. a table-keyed patch that shrinks a table), the subtraction wraps around. A 103-byte shrink requestsreserve(4294967090)(~4 GiB). With memory overcommit this goes unnoticed because the pages are never touched. Under a hard memory limit (e.g. a 4 GiB container)font2iftaborts withstd::bad_alloc.Fix
Reserve only when the derived data is larger, and compute in
size_t:The reserve is only a capacity hint, so the generated patch bytes are unchanged.
Test
Added
BrotliPatchingTest.DiffAndPatchWithSmallerDerived, which diffs full Roboto against a subset and checks the patch round-trips. On hosts with overcommit the old code doesn't fail this test (the oversized reservation succeeds), so it covers the shrinking path rather than reproducing the crash.🤖 Generated with Claude Code