-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Read gzipped response and request marshal include headers #171
Conversation
mumugoah
commented
Jun 21, 2018
•
edited
Loading
edited
- add feather: read gzipped HTTP response
- fix: request marshal and unMarshal include headers (fix should request marshal include headers? #169 )
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.
Good PR, thanks! Please check the review notes.
request.go
Outdated
@@ -56,6 +56,7 @@ type serializableRequest struct { | |||
Body []byte | |||
ID uint32 | |||
Ctx map[string]interface{} | |||
Header http.Header |
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.
Please call it Headers
to be compatible with the current naming convention.
http_backend.go
Outdated
@@ -185,6 +187,12 @@ func (h *httpBackend) Do(request *http.Request, bodySize int) (*Response, error) | |||
if bodySize > 0 { | |||
bodyReader = io.LimitReader(bodyReader, int64(bodySize)) | |||
} | |||
if res.Header.Get("Content-Encoding") == "gzip" { |
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.
As I know golang's http lib handles compression by default. If it successfully decompresses the content, then res.Uncompressed
[1] is true
, so we could check here if decompression happened to avoid double decompression:
if !res.Uncompressed && res.Header.Get("Content-Encoding") == "gzip" {
@asciimoo ok, I'll correct 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.
Great, thanks!