snappy/s2: Limit length of varint in decodedLen

[eustas]

To match C++ implementation.

Summary by CodeRabbit

• Bug Fixes

  • Stricter validation of compressed-data headers to better detect and reject malformed/overlong encodings, reducing risk of silent corruption.

• Tests

  • Added test cases covering malformed length encodings to ensure decoding surfaces corruption errors consistently.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR tightens varint length validation in the snapref and s2 decoders to reject overlong varint preambles (consuming >5 bytes). decodedLen now returns ErrCorrupt when binary.Uvarint reports n <= 0 or n > 5. Tests for overlong 8-byte varints were added.

Changes

Varint Length Validation

Layer / File(s)	Summary
Validation Logic internal/snapref/decode.go, s2/decode.go	decodedLen now treats varint results with n <= 0 or n > 5 as corrupt and returns ErrCorrupt; existing v > 0xffffffff check remains.
Test Coverage s2/s2_test.go, snappy/snappy_test.go	TestInvalidVarint table extended with an overlong 8-byte varint case; tests assert DecodedLen and Decode return ErrCorrupt.

---

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and specifically describes the main change: adding stricter validation for varint length in the decodedLen function across snappy and s2 packages.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

• Generate code and open pull requests
• Plan features and break down work
• Investigate incidents and troubleshoot customer tickets together
• Automate recurring tasks and respond to alerts with triggers
• Summarize progress and report instantly

Built for teams:

• Shared memory across your entire org—no repeating context
• Per-thread sandboxes to safely plan and execute work
• Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[klauspost]

The go version uses the same mechanism.

I don't really see any reason to change it. If people want to waste bytes IMO they are free to do so.

[eustas]

I'll update go version soon (need to unarchive repo first).

The purpose is consistency: we want all decoders to produce the same output from the same input.

The "golden" behavior is the one from C++ implementation. There we decode only until "offset" is less than 32 (given 7-bit increment that is up to 5 bytes). In brotli we have even more strict check that disallows leading zero nibs, but I can imagine some hardware encoder might prefer to have fixed size preamble if the stream length is unknown upfront, so checking for leading zeros would be harmful for snappy.

Yes, that is not exactly what spec says; going to update it to clarify that as well.

[klauspost]

OK. I will revert at the first sign of problems.

[thaJeztah]

I'll update go version soon (need to unarchive repo first).

To save others arriving here from finding the related changes; PR (pending) is here;

• Limit length of varint in decodedLen golang/snappy#77

[eustas]

Will poke the owner soon.

[klauspost]

It has been released as part of https://github.com/klauspost/compress/releases/tag/v1.19.0