Skip to content

fix(middleware): return 400 for invalid gzip request bodies in Decompress - #3187

Open
nk-batman21 wants to merge 1 commit into
labstack:masterfrom
nk-batman21:fix-decompress-invalid-gzip-status
Open

nk-batman21 wants to merge 1 commit into
labstack:masterfrom
nk-batman21:fix-decompress-invalid-gzip-status

Conversation

@nk-batman21

Copy link
Copy Markdown

Decompress answers 500 when a request has Content-Encoding: gzip but the body is not a gzip stream. gzip.Reader.Reset fails with gzip.ErrHeader for a plain body or io.ErrUnexpectedEOF for a truncated header. The middleware returns that error unchanged, so the default error handler treats it as an internal error.

This happens with a client that sets the header but sends the body uncompressed (for example a retry path that resends the original buffer), with a truncated upload, or with a scanner. The input is malformed, but it is logged as a server error, it trips 5xx alerts, and clients that retry on 5xx will retry a request that can never succeed. Corrupt data after a valid header already gets 400, because the read error comes out of Bind. With e.Use(middleware.Decompress()) and a handler that binds JSON, on master:

plain JSON labelled gzip           -> 500 {"message":"Internal Server Error"}
truncated gzip header (4 bytes)    -> 500 {"message":"Internal Server Error"}
corrupt data after valid header    -> 400 {"message":"Bad Request"}
valid gzip                         -> 200 {"name":"echo"}

The change wraps the error in echo.ErrBadRequest the same way DefaultJSONSerializer.Deserialize handles body read errors. An error that already has a status is returned unchanged, so a BodyLimit 413 raised while the header is read stays 413. The original error is still available through errors.Is. This follows #2170 (fixed in #2191), where BasicAuth changed invalid base64 from 500 to 400.

Ran: go test ./middleware/ ./echotest/, go vet ./..., golint -set_exit_status ./..., staticcheck ./...
Checked: TestDecompressInvalidGzipBody fails on master (500 != 400). TestDecompressKeepsBodyLimitError fails if the status check is removed (400 != 413).

…ress

When a request has Content-Encoding: gzip but the body does not start with
a valid gzip header (a plain body, or a truncated one), gzip.Reader.Reset
fails with gzip.ErrHeader or io.ErrUnexpectedEOF. Decompress returned that
error as is, so the default error handler answered 500 for malformed
client input. Corrupt data after a valid header already ends up as 400
through Bind.

Wrap the error as 400 Bad Request, keeping errors that already carry a
status code (such as a BodyLimit 413), the same way the JSON serializer
handles body read errors.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant