Add Reader/ReadBytes fuzz tests and limits#394
Merged
klauspost merged 8 commits intotinylib:masterfrom Jun 24, 2025
Merged
Conversation
* Fix crash in ReadTimeBytes * Limit map/array size on ReadIntfBytes * Expose `SetMaxRecursionDepth/GetMaxRecursionDepth/RecursiveCall` to manage max recursion depth. * Expose `SetMaxElements/GetMaxElements` to limit array, bin, map and extension sizes. * Expose `SetMaxStringLength/GetMaxStrLen` to limit string/map key sizes. All of these will allow stricter control over decoding memory usage. Bumps minimum version to Go 1.22
There was a problem hiding this comment.
Pull Request Overview
This PR adds configurable limits to prevent excessive memory usage in decoding, fixes an extension error in ReadTimeBytes, and introduces comprehensive fuzz tests for byte-reading functions.
- Expose and enforce max recursion depth, element counts, and string length limits on
Readerand related functions. - Fix crash in
ReadTimeBytesby using the correcttypvariable in error handling. - Add fuzz tests covering
Readerand standaloneRead*Bytesfunctions and bump minimum Go version to 1.22.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| msgp/read_bytes.go | Correct use of typ in extension error and add underflow checks in readMapStrIntfBytesDepth and readIntfBytesDepth. |
| msgp/read.go | Introduce Set/GetMaxRecursionDepth, Set/GetMaxElements, SetMaxStringLength/GetMaxStrLen, enforce limits, and rename recursiveCall. |
| msgp/json.go | Enforce string-length limits in JSON reader helpers. |
| msgp/extension.go | Enforce element-count limits in extension readers. |
| msgp/errors.go | Define new ErrLimitExceeded error. |
| msgp/fuzz_test.go | Add fuzz tests for both Reader methods and standalone byte readers. |
| go.mod | Bump minimum Go version from 1.20 to 1.22. |
| .github/workflows/validate.yml | Update linter matrix to Go 1.24. |
| .github/workflows/test.yml | Update test matrix to include Go 1.24. |
Comments suppressed due to low confidence (1)
msgp/read.go:257
- [nitpick] The getter name
GetMaxStrLenis inconsistent with the setterSetMaxStringLength. Consider renaming it toGetMaxStringLengthfor a clearer and more consistent API.
func (m *Reader) GetMaxStrLen() uint64 {
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
4c86b2b to
450c857
Compare
philhofer
reviewed
Jun 24, 2025
philhofer
approved these changes
Jun 24, 2025
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.
SetMaxRecursionDepth/GetMaxRecursionDepthto manage max recursion depth.SetMaxElements/GetMaxElementsto limit array, bin, map and extension sizes.SetMaxStringLength/GetMaxStrLento limit string/map key sizes.All of these will allow stricter control over decoding memory usage.