Skip to content

Fix index out of bounds under the disallow_large_window_size feature - #267

Merged
danielrh merged 1 commit into
dropbox:masterfrom
lenamonj:feature-distance-symbols
Oct 25, 2026
Merged

danielrh merged 1 commit into
dropbox:masterfrom
lenamonj:feature-distance-symbols

Conversation

@lenamonj

@lenamonj lenamonj commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

cargo test --features disallow_large_window_size panics in BrotliOptimizeHuffmanCountsForRle with index out of bounds: the len is 520 but the index is 520 (integration_tests::test_ukkonooa), and so does compressing ordinary text with that feature on.

BROTLI_NUM_HISTOGRAM_DISTANCE_SYMBOLS was defined twice: pub and unconditionally 544 in enc/constants.rs, and privately in enc/histogram.rs, where only that copy shrank to 520 under the feature. HistogramDistance::data_ took the private value while every bound took the public one.

This keeps one feature-aware definition in constants.rs and imports it from histogram.rs. The default build still reads 544.

Tests: cargo test --features disallow_large_window_size now passes (42 + 101). cargo fmt --check, cargo test, cargo test --doc and cargo test --features portable-float pass as before.

BROTLI_NUM_HISTOGRAM_DISTANCE_SYMBOLS was defined twice: unconditionally 544 in enc/constants.rs and privately in enc/histogram.rs, where only that copy shrank to 520 under disallow_large_window_size. The histogram array took the private value while every bound took the public one, so the encoder indexed one past the end. Keep one feature-aware definition.
@CLAassistant

CLAassistant commented Sep 7, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@danielrh
danielrh merged commit c550341 into dropbox:master Oct 25, 2026
3 checks passed
@lenamonj
lenamonj deleted the feature-distance-symbols branch October 26, 2026 01:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants