s2: Add block decode fuzzer #1044
Conversation
📝 WalkthroughWalkthroughThe pull request introduces a new fuzz testing function Changes
Sequence DiagramsequenceDiagram
participant F as Fuzzing Framework
participant FD as FuzzDecodeBlock
participant Encoder as S2 Encoder
participant Decoder as S2 Decoder
F->>FD: Provide input data
FD->>Encoder: Encode data (Encode)
FD->>Encoder: Encode data (EncodeBetter)
FD->>Decoder: Attempt decoding
FD->>FD: Validate decoded data
FD->>FD: Check data integrity
FD->>FD: Verify buffer limits
The sequence diagram illustrates the key steps in the ✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
s2/fuzz_test.go (4)
204-205: Define magic number as a constant.The size limit
8<<20should be defined as a named constant at the package level for better maintainability and documentation.+ const maxDecodedSize = 8 << 20 // 8MB - if dlen > 8<<20 { + if dlen > maxDecodedSize {
212-215: Define buffer padding size as a constant.The padding size
1024and padding pattern0xffshould be defined as named constants for better maintainability.+ const ( + paddingSize = 1024 + paddingByte = 0xff + ) dataCapped := make([]byte, 0, len(data)+1024) dataCapped = append(dataCapped, data...) - dataCapped = append(dataCapped, bytes.Repeat([]byte{0xff, 0xff, 0xff, 0xff}, 1024/4)...) + dataCapped = append(dataCapped, bytes.Repeat([]byte{paddingByte}, paddingSize)...)
219-220: Define sentinel value as a constant.The sentinel value
0xfeused for overflow detection should be defined as a named constant.+ const overflowSentinel = 0xfe - dst2 := bytes.Repeat([]byte{0xfe}, dlen+1024) + dst2 := bytes.Repeat([]byte{overflowSentinel}, dlen+paddingSize)
224-226: Improve error message clarity.The error message could be more descriptive to help with debugging.
- t.Fatalf("base err: %v, capped: %v", baseErr, err) + t.Fatalf("unexpected error with capped buffer - base decode: %v, capped decode: %v", baseErr, err)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
s2/fuzz_test.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: fuzz-other ("noasm,nounsafe")
- GitHub Check: fuzz-other (nounsafe)
- GitHub Check: build-special
- GitHub Check: fuzz-s2 ("noasm,nounsafe")
- GitHub Check: fuzz-s2 (nounsafe)
- GitHub Check: Analyze (go)
🔇 Additional comments (2)
s2/fuzz_test.go (2)
186-196: Well-structured fuzzing setup with comprehensive test corpus!The function is well-organized with:
- Multiple encoding methods to increase coverage
- Both regression and block corpus test data
- Proper separation of corpus loading and fuzzing logic
197-245: Excellent fuzzing implementation!The fuzzing logic is thorough and well-implemented with:
- Proper input data integrity checks
- Comprehensive buffer boundary testing
- Good error validation
- Effective testing of edge cases
Summary by CodeRabbit