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
net/http: Content-Range not honored by http.Client #8923
Labels
Milestone
Comments
The repro program given ignores the error value, which is the most interesting part in this bug. More information for posterity and laziness: Curl says: < HTTP/1.1 206 Partial Content < Date: Mon, 13 Oct 2014 10:14:40 GMT * Server Apache is not blacklisted < Server: Apache < Access-Control-Allow-Origin: https://render.githubusercontent.com < Content-Security-Policy: default-src 'none' < X-XSS-Protection: 1; mode=block < X-Frame-Options: deny < X-Content-Type-Options: nosniff < Strict-Transport-Security: max-age=31536000 < ETag: "272e65d6363f086441d12d519ac3ec60fbf1cda2" < Content-Type: text/plain; charset=utf-8 < Cache-Control: max-age=300 < Accept-Ranges: bytes < Via: 1.1 varnish < X-Served-By: cache-lcy1122-LCY < X-Cache: MISS < X-Cache-Hits: 0 < Vary: Authorization,Accept-Encoding < Expires: Mon, 13 Oct 2014 10:19:40 GMT < Source-Age: 0 < Content-Range: bytes 0-5/1862 < Content-Length: 6 < Go says: $ go run client.go Res: &http.Response{Status:"206 Partial Content", StatusCode:206, Proto:"HTTP/1.1", ProtoMajor:1, ProtoMinor:1, Header:http.Header{"Source-Age":[]string{"0"}, "Access-Control-Allow-Origin":[]string{"https://render.githubusercontent.com"}, "Content-Security-Policy":[]string{"default-src 'none'"}, "X-Content-Type-Options":[]string{"nosniff"}, "Cache-Control":[]string{"max-age=300"}, "Accept-Ranges":[]string{"bytes"}, "Via":[]string{"1.1 varnish"}, "X-Cache-Hits":[]string{"0"}, "X-Frame-Options":[]string{"deny"}, "Etag":[]string{"\"272e65d6363f086441d12d519ac3ec60fbf1cda2\""}, "X-Served-By":[]string{"cache-lcy1129-LCY"}, "Vary":[]string{"Authorization,Accept-Encoding"}, "Expires":[]string{"Mon, 13 Oct 2014 10:22:26 GMT"}, "Date":[]string{"Mon, 13 Oct 2014 10:17:26 GMT"}, "Server":[]string{"Apache"}, "Content-Type":[]string{"text/plain; charset=utf-8"}, "X-Xss-Protection":[]string{"1; mode=block"}, "Strict-Transport-Security":[]string{"max-age=31536000"}, "X-Cache":[]string{"MISS"}, "Content-Range":[]string{"bytes 0-5/856"}}, Body:(*http.bodyEOFSignal)(0xc208144d00), ContentLength:-1, TransferEncoding:[]string(nil), Close:false, Trailer:http.Header(nil), Request:(*http.Request)(0xc208032820), TLS:(*tls.ConnectionState)(0xc20804fc00)} Expires: [Mon, 13 Oct 2014 10:22:26 GMT] X-Frame-Options: [deny] Etag: ["272e65d6363f086441d12d519ac3ec60fbf1cda2"] X-Served-By: [cache-lcy1129-LCY] Vary: [Authorization,Accept-Encoding] Date: [Mon, 13 Oct 2014 10:17:26 GMT] Server: [Apache] Content-Type: [text/plain; charset=utf-8] X-Xss-Protection: [1; mode=block] Strict-Transport-Security: [max-age=31536000] X-Cache: [MISS] Content-Range: [bytes 0-5/856] Accept-Ranges: [bytes] Via: [1.1 varnish] X-Cache-Hits: [0] Source-Age: [0] Access-Control-Allow-Origin: [https://render.githubusercontent.com] Content-Security-Policy: [default-src 'none'] X-Content-Type-Options: [nosniff] Cache-Control: [max-age=300] length: -1 Body: "" (0); unexpected EOF So I think transfer.go is converting the actual "Content-Length: 6" from the server into Content-Length: -1 (unknown) because the StatusCode (206) is unexpected. And then the transfer body reader is returning unexpected EOF because there's a Content-Length of -1 (which normally means chunked), but it's not in chunked encoding. So the test and fix should be be that a response with 206 + a Content-Length keeps the Content-Length, and then reads the body, in response_test.go. The fix is probably in transfer.fo. Feel free to try to fix it soon here if you can in the next day or so, otherwise I will. Labels changed: added release-go1.4, removed release-none. Owner changed to @bradfitz. Status changed to Accepted. |
It seems to come out of ReadResponse just fine and a test hopefully shows that as well by adding another small respTest entry: http://play.golang.org/p/PWsy9tmeG8 It rhymes ok with my stepping through as well where it seems to reset the ContentLength when considering gzip. Just dumping out a few thoughts. Not ready to give up yet but any hints are welcome. |
I am not sure this is an error anymore... Correct me if I am wrong but if you run curl like this: curl -vv --header 'Range: bytes=0-5' --header 'Accept-Encoding: gzip' 'https://raw.githubusercontent.com/square/okhttp/master/README.md' I.e. enforcing gzip there is no content in curl either. I assume it is because the server has to zip the entire content before it sends it and that makes it impossible to decode the little received chunk. Knock, knock. If we get a gzip header we wrap a gzipReader innermost in the reader stack (is that the proper term, stack?). This reader tries to automatically unzip the payload I guess and then it errors out. Can that be it? If so it either seems like a harder problem or quite a lot simpler. Just skip the wrapped gzipReader and leave it to the caller to assemble and unzip the resulting file. Does that make sense? |
Actually, curl -vv --header 'Range: bytes=0-5' --header 'Accept-Encoding: gzip' 'https://raw.githubusercontent.com/square/okhttp/master/README.md' is giving the raw gzipped data. If you will run without giving Range header, curl -vv --header 'Accept-Encoding: gzip' 'https://raw.githubusercontent.com/square/okhttp/master/README.md' then, also it will give the raw gzipped data and not print the real content. |
CL https://golang.org/cl/155420044 mentions this issue. |
This issue was closed by revision 42c3130. Status changed to Fixed. |
wheatman
pushed a commit
to wheatman/go-akaros
that referenced
this issue
Jun 25, 2018
The http package by default adds "Accept-Encoding: gzip" to outgoing requests, unless it's a bad idea, or the user requested otherwise. Only when the http package adds its own implicit Accept-Encoding header does the http package also transparently un-gzip the response. If the user requested part of a document (e.g. bytes 40 to 50), it appears that Github/Varnish send: range(gzip(content), 40, 50) And not: gzip(range(content, 40, 50)) The RFC 2616 set of replacements (with the purpose of clarifying ambiguities since 1999) has an RFC about Range requests (http://tools.ietf.org/html/rfc7233) but does not mention the interaction with encodings. Regardless of whether range(gzip(content)) or gzip(range(content)) is correct, this change prevents the Go package from asking for gzip in requests if we're also asking for Range, avoiding the issue. If the user cared, they can do it themselves. But Go transparently un-gzipping a fragment of gzip is never useful. Fixes golang#8923 LGTM=adg R=adg CC=golang-codereviews https://golang.org/cl/155420044
wheatman
pushed a commit
to wheatman/go-akaros
that referenced
this issue
Jun 26, 2018
The http package by default adds "Accept-Encoding: gzip" to outgoing requests, unless it's a bad idea, or the user requested otherwise. Only when the http package adds its own implicit Accept-Encoding header does the http package also transparently un-gzip the response. If the user requested part of a document (e.g. bytes 40 to 50), it appears that Github/Varnish send: range(gzip(content), 40, 50) And not: gzip(range(content, 40, 50)) The RFC 2616 set of replacements (with the purpose of clarifying ambiguities since 1999) has an RFC about Range requests (http://tools.ietf.org/html/rfc7233) but does not mention the interaction with encodings. Regardless of whether range(gzip(content)) or gzip(range(content)) is correct, this change prevents the Go package from asking for gzip in requests if we're also asking for Range, avoiding the issue. If the user cared, they can do it themselves. But Go transparently un-gzipping a fragment of gzip is never useful. Fixes golang#8923 LGTM=adg R=adg CC=golang-codereviews https://golang.org/cl/155420044
wheatman
pushed a commit
to wheatman/go-akaros
that referenced
this issue
Jul 9, 2018
The http package by default adds "Accept-Encoding: gzip" to outgoing requests, unless it's a bad idea, or the user requested otherwise. Only when the http package adds its own implicit Accept-Encoding header does the http package also transparently un-gzip the response. If the user requested part of a document (e.g. bytes 40 to 50), it appears that Github/Varnish send: range(gzip(content), 40, 50) And not: gzip(range(content, 40, 50)) The RFC 2616 set of replacements (with the purpose of clarifying ambiguities since 1999) has an RFC about Range requests (http://tools.ietf.org/html/rfc7233) but does not mention the interaction with encodings. Regardless of whether range(gzip(content)) or gzip(range(content)) is correct, this change prevents the Go package from asking for gzip in requests if we're also asking for Range, avoiding the issue. If the user cared, they can do it themselves. But Go transparently un-gzipping a fragment of gzip is never useful. Fixes golang#8923 LGTM=adg R=adg CC=golang-codereviews https://golang.org/cl/155420044
wheatman
pushed a commit
to wheatman/go-akaros
that referenced
this issue
Jul 30, 2018
The http package by default adds "Accept-Encoding: gzip" to outgoing requests, unless it's a bad idea, or the user requested otherwise. Only when the http package adds its own implicit Accept-Encoding header does the http package also transparently un-gzip the response. If the user requested part of a document (e.g. bytes 40 to 50), it appears that Github/Varnish send: range(gzip(content), 40, 50) And not: gzip(range(content, 40, 50)) The RFC 2616 set of replacements (with the purpose of clarifying ambiguities since 1999) has an RFC about Range requests (http://tools.ietf.org/html/rfc7233) but does not mention the interaction with encodings. Regardless of whether range(gzip(content)) or gzip(range(content)) is correct, this change prevents the Go package from asking for gzip in requests if we're also asking for Range, avoiding the issue. If the user cared, they can do it themselves. But Go transparently un-gzipping a fragment of gzip is never useful. Fixes golang#8923 LGTM=adg R=adg CC=golang-codereviews https://golang.org/cl/155420044
This issue was closed.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
by dahankzter:
The text was updated successfully, but these errors were encountered: