-
Notifications
You must be signed in to change notification settings - Fork 32
Add support for Content-Encoding on the HTTP request #47
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
Conversation
voidmain
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing major, but it looks like there is a dead variable.
| case Headers.ContentEncodingLower: | ||
| String[] encodings = value.split(","); | ||
| List<String> contentEncodings = new ArrayList<>(1); | ||
| int encodingIndex = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this variable is needed. Can we remove it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm.. not sure what that was for.. but not necessary any longer. 😎
Thanks!
Handle the
Content-Encodingon the HTTP request to match our response compression support. This includesgzipanddeflate.We could consider adding support for additional compression algorithms such as Brotli (
br) or zStandard (zstd) - but to do that we'd likely have to pull in 3rd party libs, or implement them ourselves. This would go against our no-deps policy. So if we did, perhaps we'd want to just allow them to be added via separate jars so we don't pollute the core project.It is interesting to note that not a lot of HTTP servers support request compression. Sort of makes sense since if you assume the client is generally not sending large requests, only receiving large request. I don't see any harm in supporting it though - even if it isn't super common to use it.