Repository navigation
Conversation
2 tasks done
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Author
|
Closing this PR because I am continuing the same fix from my new account, @hainuoli, in #5879. The replacement preserves both commits and links back to this PR for the original discussion and CI results. The companion JSON parsing fix continues in #5880 (replacing #5815). Please continue review on the replacement PRs. |
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.
MaxBytesHandlerchecks onlyContentLength, so unknown-length requests can bypass a configured body limit. For example, with a limit of 8 bytes, a 16-byte JSON request can reach the handler and be parsed successfully.Wrap accepted request bodies with
http.MaxBytesReaderto enforce the limit while reading. Keep the existing early rejection for known oversized requests and the disabled-limit behavior. In the defaulthttpx.Errorandhttpx.ErrorCtxpaths, map direct or wrapped*http.MaxBytesErrorvalues to HTTP 413. Custom error handlers keep their existing precedence.Companion to #5815, following #5815 (comment). This PR can be reviewed independently and does not eagerly read or buffer the complete request body.
Regression coverage:
httptest.NewServer, using dynamically allocated ports and checking the server's actual content length and transfer encoding.Error/ErrorCtxlayout.The JSON tests use
jsonx.UnmarshalFromReader, the same decoder path used by the parser, so they do not depend on #5815. The two fixes were also tested together with a temporary Go overlay and real HTTP requests, including empty-body defaults and HTTP 413 for oversized chunked JSON.Validation:
go test -mod=readonly -race ./rest/handler ./rest/httpx -count=1go vet -mod=readonly ./rest/handler ./rest/httpxgo test -mod=readonly -race -coverprofile=coverage.txt -covermode=atomic ./... -skip '^TestRpcServer$' -timeout 3mgofmtandgit diff --checkThe local root-module run skips only the unchanged
zrpc/internal.TestRpcServer, whose fixedlocalhost:54321port is occupied by an existing application. All remaining packages and tests passed. The earlier upstream Linux failure was in the unchangedgateway.TestHttpToHttp(listen tcp :45678: bind: address already in use); the relevant REST packages passed that run. The follow-up commit adds regression coverage and does not modify gateway tests or CI configuration.Upstream CI on
d1ef79d972dc350b66aacc0b7b392b7670e0c8e0is now green: Go (Linux, Windows, and RPC generation), staticcheck, and CodeQL. The Linux run includes the full test suite without the local skip and a successful Codecov upload.