-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
HTTP client gives incorrect Content-Length for automatically decompressed responses #2743
Comments
Apparently Tornado decompresses gzip responses by default. Worse, it fails to adjust the Content-Length header when it does. tornadoweb/tornado#2743 Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
Apparently Tornado decompresses gzip responses by default. Worse, it fails to adjust the Content-Length header when it does. tornadoweb/tornado#2743 Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
That's true in principle, but that ship has sailed. We probably should rename the received |
That’s true. Perhaps But class ProxyHandler(tornado.web.RequestHandler):
def initialize(self):
self.headers = None
self.headers_added = False
def header_callback(self, line):
if self.headers is None:
start_line = tornado.httputil.parse_response_start_line(line)
self.set_status(start_line.code, start_line.reason)
self.headers = tornado.httputil.HTTPHeaders()
else:
self.headers.parse_line(line)
def add_headers(self):
if not self.headers_added:
for name, value in self.headers.get_all():
self.clear_header(name)
for name, value in self.headers.get_all():
if name != "Transfer-Encoding":
self.add_header(name, value)
self.headers_added = True
def streaming_callback(self, chunk):
self.add_headers()
self.write(chunk)
self.flush()
async def get(self):
await tornado.httpclient.AsyncHTTPClient().fetch(
"http://localhost:8888/hello.tar.gz",
header_callback=self.header_callback,
streaming_callback=self.streaming_callback,
# decompress_response=False,
)
self.add_headers() This correctly passes on |
Apparently Tornado decompresses gzip responses by default. Worse, it fails to adjust the Content-Length header when it does. tornadoweb/tornado#2743 Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
The idea is that applications should be free to implement transfer-encodings themselves, without Tornado's knowledge or interference. If you pass a It seems a similar argument applies to |
Apparently Tornado decompresses gzip responses by default. Worse, it fails to adjust the Content-Length header when it does. tornadoweb/tornado#2743 Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
Apparently Tornado decompresses gzip responses by default. Worse, it fails to adjust the Content-Length header when it does. tornadoweb/tornado#2743 Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
The application below has two handlers, the first of which serves a gzipped tarball at
/hello.tar.gz
, and the second of which tries to proxy/hello2.tar.gz
to the first. But the second actually raisestornado.httputil.HTTPOutputError: Tried to write more data than Content-Length
.The reason is that the Tornado HTTP client has decompressed the
Content-Encoding: gzip
without adjusting theContent-Length
field, resulting in an invalid response. A workaround is settingdecompress_response=False
.When
decompress_response
is on, Tornado should adjust or remove theContent-Length
field of decompressed responses, similarly to how it already removes theContent-Encoding
field.I would also argue that
decompress_response
is unexpected behavior that should really be off by default, even if it’s fixed, as it changes the semantic meaning of the response:Content-Encoding
is notTransfer-Encoding
.The text was updated successfully, but these errors were encountered: