Return clear HTTP 413 when request body exceeds max_request_body_size#2841
Closed
arpitjain099 wants to merge 1 commit into
Closed
Return clear HTTP 413 when request body exceeds max_request_body_size#2841arpitjain099 wants to merge 1 commit into
arpitjain099 wants to merge 1 commit into
Conversation
When max_request_body_size is set, the maxBodySize middleware wraps the request body with http.MaxBytesReader. The resulting read error only surfaced to clients as a clear 413 in one wrapping shape (a CompositeError holding a ParseError); a bare *http.MaxBytesError fell through to an opaque 500 and a top-level ParseError produced a 400 that leaked the internal 'http: request body too large' text. Clients (for example cosign) saw only a generic retry failure with no indication that the body size limit was hit. Detect the *http.MaxBytesError across all of these shapes via a single helper and respond with 413 Request Entity Too Large, naming the configured limit in bytes so operators and clients can see why the request was rejected. ParseError is handled explicitly because it does not implement Unwrap, so errors.As cannot reach its Reason on its own. Add unit tests covering each error shape, an under-limit request, and the detection helper directly. Fixes #2808 Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
Contributor
|
There is already an open PR for this - #2813 |
Author
@Hayden-IO I see. Noted. Thanks! |
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.
Summary
Fixes #2808.
When
max_request_body_sizeis configured, themaxBodySizemiddleware inpkg/generated/restapi/configure_rekor_server.gowraps the request body withhttp.MaxBytesReader. Once the limit is exceeded the read fails, but the way that failure was surfaced to clients depended entirely on how the error happened to be wrapped before it reachedlogAndServeError:*http.MaxBytesErrorfell through to an opaque500with bodyhttp: request body too large*errors.ParseErrorproduced a400that leaked the same internalhttp: request body too largetextCompositeErrorholding aParseErrorwas converted to413, and even then the message was a genericRequest Entity Too Largethat never named the limitThat is why the reporter (uploading with cosign against the public instance) saw only a generic
giving up after N attempt(s)retry failure with no hint that the body size limit was the cause, and why the limit itself was effectively undiscoverable.This change detects the underlying
*http.MaxBytesErroracross all of those shapes through a single helper (asMaxBytesError) and responds with413 Request Entity Too Large, including the configured limit in bytes (read straight fromMaxBytesError.Limit) so operators and clients can see exactly why the request was rejected.*errors.ParseErroris handled explicitly because it does not implementUnwrap, soerrors.Ascannot reach itsReasonon its own; everything else (bare error, or one nested in aCompositeError, which does unwrap) is caught byerrors.As. The detailed error is still logged server-side, and it is now logged at warn rather than error since it is a client error.This edit is in the hand-maintained region of
configure_rekor_server.go(the file carries the go-swaggerThis file is safe to edit. Once it exists it will not be overwrittenmarker, and the prior 413 handling lived in this same region).How to verify:
Behaviour for an over-limit body with the fix applied:
500 {"code":500,"message":"http: request body too large"}(or a400leaking the same text, depending on the consumer)413 {"code":413,"message":"request body exceeds max_request_body_size limit of 1024 bytes"}The new tests cover each error shape, an under-limit request (must stay a success, not a false-positive 413), and the detection helper directly.
Release Note
Rekor now returns a clear
413 Request Entity Too Largeresponse that names the configured byte limit when a request body exceedsmax_request_body_size, instead of an opaque500/400with no explanation. The detailed error continues to be logged server-side only.Documentation
None.