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

Fix handling of WriteString in gzip middleware #59

Closed
wants to merge 1 commit into from

Conversation

johnniedoe
Copy link

When using the gzip middleware, calls to WriteString were falling through to
using the embedded gin.ResponseWriter's WriteString. This would sidestep
the gzip encoder and inject uncompressed data into the output.

This change makes the gzip middleware handle the call itself.

Running the 'TestGzip' test in the existing test suite demonstrates the issue.

@lawlite
Copy link

lawlite commented Aug 28, 2015

Nice! Thank you.

@victorquinn
Copy link

Any updates here?

This is a very minor fix and being unable to use c.String() with gzip enabled is a pretty critical failure of this middleware. I'm pretty surprised this hasn't been merged in over 4 months.

@luca-moser
Copy link

Is this repo dead?

@mcastilho
Copy link
Contributor

Yeah. I would like to see this merged. Experiencing the same problem here.

@chrispassas
Copy link

This was also breaking gzip for me. Adding in the missing WriteString method fixed the issue.

chrispassas added a commit to chrispassas/contrib that referenced this pull request Jul 19, 2016
This is a fix in a fork. I'm making a copy so I can build my project off of my own repo until the code is merged into the main project.

The original issue was reported and a fixed provided here: gin-gonic#59
@kevineaton
Copy link

Is there any update on this? I cannot compress my output using gzip and some of my JSON will be large. I really don't want to proxy behind nginx if I don't have to.

@@ -42,6 +42,10 @@ type gzipWriter struct {
writer *gzip.Writer
}

func (g *gzipWriter) WriteString(s string) (n int, err error) {

Choose a reason for hiding this comment

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

Please drop the variable names from the return values

@tboerger
Copy link

Somebody can also create a new pr if the author doesn't update this :)

mcastilho added a commit to mcastilho/contrib that referenced this pull request Oct 12, 2016
This fixes code-review requests mentioned in gin-gonic#59
@mcastilho
Copy link
Contributor

@tboerger I have opened another PR as you suggested: #118

@tboerger
Copy link

So let's close this in favor of #118

@tboerger tboerger closed this Oct 12, 2016
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.

None yet

8 participants