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

Setting Content Length on GZip Requests #9

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gja
Copy link

@gja gja commented Oct 28, 2016

This adds a Content-Length to the response, if an original Content-Length was present, and less than 1MB. This prevents jetty from adding Transfer-Encoding: chunked to the response, without requiring Connection: Close

gja added 2 commits October 28, 2016 17:27
This adds a Content-Length to the response, if an original Content-Length was present, and less than 1MB. This prevents jetty from adding Transfer-Encoding: chunked to the response, without requiring Connection: Close
@gja
Copy link
Author

gja commented Nov 7, 2016

Hello Maintainers,

Do you have any feedback on this pull request?

@danielcompton
Copy link
Member

danielcompton commented Dec 6, 2018

Hi there, this looks like it could be a good patch to include, are you able to add any tests? I realise it's been a long time since the original PR, so can understand if you've moved on. Also, this behaviour seems like it should be configurable somehow, as people may not want to create the byte-array. What are the implications of the current behaviour?

This prevents jetty from adding Transfer-Encoding: chunked to the response, without requiring Connection: Close

Can you expand on what the issues with this are?

@gja
Copy link
Author

gja commented Dec 6, 2018

Let me check. I know that we rely on this behaviour, but I can't remember what are the reasons are. IIRC, it was that some proxies were refusing to cache the response, as it believed that it was receiving a stream, or something like that.

@gja
Copy link
Author

gja commented Dec 6, 2018

I took a sensible default of only buffering responses up to 1mb in memory. This should cover most HTML responses and API responses, while only consuming < 1mb of memory. I doubt anyone will serve files more than 1 mb from ring.

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