Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gzip will compress the compressed body again #47

Open
LawyZheng opened this issue Jan 21, 2022 · 5 comments
Open

Gzip will compress the compressed body again #47

LawyZheng opened this issue Jan 21, 2022 · 5 comments

Comments

@LawyZheng
Copy link

Gzip will compress the compressed body again when I use third-party handlers to handle gin context,
like httputil.ReverseProxy, promhttp.

It seems if the target HTTP server returns a gzip-compressed body, the middleware will compress it again,
which could result in the client can't decompress the response correctly.

For example, Prometheus can't scrape the target response correctly or some responses will be garbled, due to the double compression.

Though I can solve this problem by setting the gzip.WithExcludedPaths or other ExcludedOptions, I don't think it is the best way. Because sometimes we want to solve this problem more wisely and elegantly, rather than coding the ExcludedOptions hardly.

I also tried to fix this problem by myself. And past all default unit tests.

Am I welcomed to make a pr?

@SinuxLee
Copy link

SinuxLee commented Oct 19, 2022

I had the same problem,Can we do like this ?

func (g *gzipHandler) shouldCompress(req *http.Request) bool {
	if !strings.Contains(req.Header.Get("Accept-Encoding"), "gzip") ||
		strings.Contains(req.Header.Get("Connection"), "Upgrade") ||
		strings.Contains(req.Header.Get("Accept"), "text/event-stream") {
		return false
	}

	extension := filepath.Ext(req.URL.Path)
	if g.ExcludedExtensions.Contains(extension) {
		return false
	}

	if g.ExcludedPaths.Contains(req.URL.Path) {
		return false
	}
	if g.ExcludedPathesRegexs.Contains(req.URL.Path) {
		return false
	}

        c.Request.Header.Del("Accept-Encoding") // remove the header key, then other middlewares don't compress

	return true
}

@LawyZheng
Copy link
Author

@SinuxLee I don't think it's a good idea.
I think we should skip the compress when the body is gzip-compressed, rather than change the request header. Because in my opinion, HTTP messages should be in-sensitive to other middlewares.

@SinuxLee
Copy link

@LawyZheng 👍 Looking forward to your PR. Maybe, I can help to review the code :)

@rleungx
Copy link

rleungx commented Apr 19, 2023

Any update on this issue?

@LawyZheng
Copy link
Author

Any update on this issue?

waiting for PR merge

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

No branches or pull requests

3 participants