Skip to content

fix(middleware): reset ContentLength after gzip decompression#3000

Open
shblue21 wants to merge 1 commit into
labstack:masterfrom
shblue21:fix-decompress-request-metadata
Open

fix(middleware): reset ContentLength after gzip decompression#3000
shblue21 wants to merge 1 commit into
labstack:masterfrom
shblue21:fix-decompress-request-metadata

Conversation

@shblue21

@shblue21 shblue21 commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix Decompress so middleware after it does not keep using the compressed request size after the body has been replaced with the decoded gzip stream.

For example:

e.Use(middleware.Decompress())
e.Use(middleware.BodyLimit(4))

A gzipped request whose decoded body is ok should pass:

printf "ok" | gzip | curl -X POST \
  -H "Content-Type: text/plain" \
  -H "Content-Encoding: gzip" \
  --data-binary @- \
  http://localhost:8080/

Before this change, Decompress replaced req.Body but left req.ContentLength set to the compressed size, so BodyLimit could reject the request before reading it:

{"message":"Request Entity Too Large"}

With this change, Decompress sets req.ContentLength = -1 after gzip decompression is set up, so downstream middleware enforces limits while reading the decoded body:

ok

This intentionally does not change Content-Encoding, GetBody, or multiple content-coding behavior.

Test plan

  • go test -count=1 ./...
  • go test -race -count=1 ./...
  • go vet ./...
  • staticcheck ./...
  • Manual curl check with the same handler returned ok and HTTP 200

@codecov

codecov Bot commented Jun 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.18%. Comparing base (01b45be) to head (918b1a5).
⚠️ Report is 9 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3000   +/-   ##
=======================================
  Coverage   93.17%   93.18%           
=======================================
  Files          43       43           
  Lines        4501     4504    +3     
=======================================
+ Hits         4194     4197    +3     
  Misses        189      189           
  Partials      118      118           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@shblue21 shblue21 force-pushed the fix-decompress-request-metadata branch from 3fe50ad to 0a25d94 Compare June 10, 2026 18:54
Comment thread middleware/decompress.go Outdated
req.Header.Del(echo.HeaderContentEncoding)
req.Header.Del(echo.HeaderContentLength)
req.ContentLength = -1
req.GetBody = nil

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GetBody doc comments says that For server requests, it is unused.

@aldas

aldas commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

The description is little bit cryptic on first clance. It would be better if the situation/use-case would be explained in a "caveman manner"

I am using Decompress middleware along with the body limit middleware. This snippet

func main() {
	e := echo.New()

	e.Use(middleware.Decompress())
	e.Use(middleware.BodyLimit(4))

	e.POST("/", func(c *echo.Context) error {
		body, readErr := io.ReadAll(c.Request().Body)
		if readErr != nil {
			return readErr
		}
		return c.String(http.StatusOK, string(body))
	})

	if err := e.Start(":8080"); err != nil {
		slog.Error("Failed to start server", "error", err)
	}
}

does not work because the Decompress middleware leaves req.ContentLength unchanged after decompressing and this will trigger BodyLimit middleware to deny the request, although the compressed body is actually smaller than ContentLength reports.

At the moment and tested also with

printf "ok" | gzip | curl   -X POST   -H "Content-Type: text/plain"   -H "Content-Encoding: gzip"   --data-binary @-   http://localhost:8080/

Currently the result is incorrect:

{"message":"Request Entity Too Large"}

Expected would be:

ok

@aldas

aldas commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

just nitpicking here as it is not easy sometimes to understand what the actual use-case is, and fixing (accepting PRs) something that you do not understand can break things on some other place.

Comment thread middleware/decompress.go Outdated
if c.Request().Header.Get(echo.HeaderContentEncoding) != GZIPEncoding {
req := c.Request()
contentEncoding := req.Header.Values(echo.HeaderContentEncoding)
if len(contentEncoding) != 1 || strings.TrimSpace(contentEncoding[0]) != GZIPEncoding {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this change necessary? It does not help if the request has been compressed multiple times. For example:

Content-Encoding: gzip, br

or

Content-Encoding: gzip
Content-Encoding: br

meaning that decompression is in reverse order:

  1. decode br
  2. decode gzip
  3. obtain the original content

Comment thread middleware/decompress.go Outdated
c.Request().Body = gr
req.Body = gr
}
req.Header.Del(echo.HeaderContentEncoding)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above - if are to fix headers then the plain "deletion" is wrong approach as there could be multiple Content-Encoding values and we are deleting all of them now - not only gzip part.

Is this complexity necessary and valuable for any realworld use-case?

@aldas

aldas commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Please not to take this as me being mean. I am trying keep things simple and avoid unessesary complexity if possible.

@shblue21 shblue21 force-pushed the fix-decompress-request-metadata branch from 0a25d94 to 30b16e7 Compare June 11, 2026 01:56
@shblue21 shblue21 changed the title fix(middleware): clear request metadata after decompressing gzip body fix(middleware): reset ContentLength after gzip decompression Jun 11, 2026
@shblue21 shblue21 force-pushed the fix-decompress-request-metadata branch from 30b16e7 to 918b1a5 Compare June 11, 2026 02:14
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.

2 participants