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

Support transparent response body compression #119

Closed
tanuck opened this issue Jun 21, 2016 · 18 comments
Closed

Support transparent response body compression #119

tanuck opened this issue Jun 21, 2016 · 18 comments
Milestone

Comments

@tanuck
Copy link

tanuck commented Jun 21, 2016

Hi,

Again, I don't know whether there's scope for this in fabio - but have you thought about applying compression to proxied responses?

Possibly supplying a list of mime types in the properties file that will be gzip'ed etc?

Cheers

@smancke
Copy link
Contributor

smancke commented Jun 21, 2016

I'have recently written a mime type aware gzip handler,
after reviewing some existing solutions out there:
https://github.com/smancke/handler/blob/master/gzip/gzip_handler.go

If you like, just copy over the code and tests to fabio.

@magiconair
Copy link
Contributor

magiconair commented Jun 21, 2016

Shouldn't this work by just setting the Accept-Encoding: gzip, deflate header? Can you explain a bit more what you're after?

@tanuck
Copy link
Author

tanuck commented Jun 28, 2016

@magiconair no, i was focusing on apply compression to responses proxied by fabio - what @smancke has suggested with his gzip handler

Cheers

@magiconair
Copy link
Contributor

@tanuck so you want fabio to inject the Accept-Encoding header into the outgoing request?

@tanuck
Copy link
Author

tanuck commented Jun 28, 2016

no, just leave that to the browser/client - but if it's present, have the same sort of feature as nginx, to compress the response from the upstream service before sending it to the client and then setting the Content-Encoding header in the response.

@magiconair
Copy link
Contributor

@tanuck Sorry, I don't get it. Isn't that exactly the behavior of the net/http library? The browser sets the Accept-Encoding header and fabio compresses the response sent back to the client? I'm sure I'm missing something here. Could you provide an example request/response from/to fabio and from/to upstream service?

@magiconair
Copy link
Contributor

@tanuck I think I get it. You want some responses to be compressed independent of whether the browser has requested compression, i.e. text/css should always be compressed even if the browser did not send the Accept-Encoding header.

@tanuck
Copy link
Author

tanuck commented Jul 18, 2016

@magiconair no not quite. So i'm trying to get a fabio setup that can replicate my current "nginx as a reverse proxy" configuration.

So right now I have an nginx service, proxying requests to various dockerized microservices. The microservices don't perform any gzip compression. Instead, we use the nginx gzip module. If the browser sends Accept-Encoding: gzip then nginx will compress the response from the upstream microservices and set Content-Encoding: gzip before returning the response to the browser. I hope that's more clear.

So my question is whether this can be replicated with fabio, whether responses from upstream services can be compressed returning the response to the client/browser.

The http.Handler that @smancke linked is pretty much what I mean, so some way to integrate that into fabio based on a config setting?

Cheers

@magiconair
Copy link
Contributor

@tanuck ah, I think the penny finally dropped. I was under the impression that the Go std lib already does this but it looks like it doesn't. The DisableCompression applies only to the client AFAICT. In that case I'll add it. Shouldn't be a big issue.

@tanuck
Copy link
Author

tanuck commented Jul 18, 2016

Awesome :) I'm happy to help out with the implementation if needed.

What do you think is the best way to configure this? In fabio.properties or as part of the route tags in consul etc?

@magiconair
Copy link
Contributor

magiconair commented Jul 18, 2016

I'll let you know when I have a patch ready. For now this needs to be a global option since I don't have a way to express this on the routes yet.

@magiconair
Copy link
Contributor

@smancke any objections if I incorporate your code with proper attribution? License is MIT.

@magiconair
Copy link
Contributor

@smancke just read your first comment again where you're ok with this.

@smancke
Copy link
Contributor

smancke commented Jul 18, 2016

@magiconair Yes, please just take it.

@magiconair magiconair changed the title Compress Support transparent response body compression Aug 31, 2016
magiconair added a commit that referenced this issue Aug 31, 2016
This patch adds an option 'proxy.gzip.contentype' which
enables transparent response body compression if the
client requests it, the content type of the response
matches the proxy.gzip.contenttype regexp and the
response is not already compressed.

The gzip handler is mostly based on the code from
@smanke from https://github.com/smanke/handler/gzip.
@magiconair
Copy link
Contributor

I've finally found the time to implement this. I've mostly taken your code @smancke and replaced only the testify/assert framework with my own 10-liner since I didn't want to pull in testify/assert + dependencies just for this. The other change is that the list of compressible content types need to be configurable and since we need to do prefix and postfix checks (e.g. everything ending on +json) I've opted for a regular expression in proxy.gzip.contenttype which describes the content types. I still need to implement an integration test that checks that it actually integrates properly. Feel free to test.

@numard
Copy link

numard commented Oct 17, 2016

+1 please :)

@magiconair
Copy link
Contributor

@numard If this can wait until the end of next week then I have the perfect thing to work on while I'm at your office. Should be in on Friday, 28 Oct and the week after.

magiconair added a commit that referenced this issue Oct 27, 2016
This patch adds an option 'proxy.gzip.contentype' which
enables transparent response body compression if the
client requests it, the content type of the response
matches the proxy.gzip.contenttype regexp and the
response is not already compressed.

The gzip handler is mostly based on the code from
@smanke from https://github.com/smanke/handler/gzip.
magiconair added a commit that referenced this issue Oct 28, 2016
This patch adds an option 'proxy.gzip.contentype' which
enables transparent response body compression if the
client requests it, the content type of the response
matches the proxy.gzip.contenttype regexp and the
response is not already compressed.

The gzip handler is mostly based on the code from
@smanke from https://github.com/smanke/handler/gzip.
magiconair added a commit that referenced this issue Oct 28, 2016
This patch adds an option 'proxy.gzip.contentype' which
enables transparent response body compression if the
client requests it, the content type of the response
matches the proxy.gzip.contenttype regexp and the
response is not already compressed.

The gzip handler is mostly based on the code from
@smanke from https://github.com/smanke/handler/gzip.
@magiconair
Copy link
Contributor

Merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants