Gzip compression shouldn't force chunked encoding #162

Closed
afcowie opened this Issue Dec 29, 2012 · 3 comments

Comments

Projects
None yet
2 participants
@afcowie
Contributor

afcowie commented Dec 29, 2012

Setting a Content-Length header should be preserved even if the client has Accept-Encoding: gzip; most servers do set that header for fixed size compressed content, it seems. Which is what I'm trying to replicate with

n <- getSize "path/to/file.txt"
modifyResponse $ setContentLength n
sendFile "path/to/file.txt"

and many variations, but it seems that when gzip takes over it switches to chunked regardless? Be nice if the content length header (being set properly?) could be preserved...

Observed with snap-core 0.9.2.2 / snap-server 0.9.2.4, so I don't know if this is something that's changed in new-snap-server.

AfC

@gregorycollins

This comment has been minimized.

Show comment
Hide comment
@gregorycollins

gregorycollins Dec 29, 2012

Member

Working as intended. You can't know the size of a gzipped stream without gzipping the whole thing. To do as you ask would mean that streaming would not be possible. So, we either force chunked transfer encoding on http/1.1, or set connection: close and mark end-of-stream by closing the connection.

Note that the content-length header gives the size of the transfer-encoded response body. i.e. post gzipping. In the code snippet you posted, if we preserved the content-length the user set after gzipping then we would break the protocol invariants.

Member

gregorycollins commented Dec 29, 2012

Working as intended. You can't know the size of a gzipped stream without gzipping the whole thing. To do as you ask would mean that streaming would not be possible. So, we either force chunked transfer encoding on http/1.1, or set connection: close and mark end-of-stream by closing the connection.

Note that the content-length header gives the size of the transfer-encoded response body. i.e. post gzipping. In the code snippet you posted, if we preserved the content-length the user set after gzipping then we would break the protocol invariants.

@afcowie

This comment has been minimized.

Show comment
Hide comment
@afcowie

afcowie Dec 30, 2012

Contributor

Alright, fair enough. I guess the behaviour I was comparing to was Apache serving static .html files; not sure why, but when mod_deflate does gzip compression outbound, it does it in one pass and sets the Content-Length header. Same if you're pulling from various HTTP caches, of course. Anyway, I'll adjust my test suite to confirm to Snap's behaviour; I just wanted a case with Content-Length and gzip compression to exercise that code path.

Sorry to trouble you.

AfC

Contributor

afcowie commented Dec 30, 2012

Alright, fair enough. I guess the behaviour I was comparing to was Apache serving static .html files; not sure why, but when mod_deflate does gzip compression outbound, it does it in one pass and sets the Content-Length header. Same if you're pulling from various HTTP caches, of course. Anyway, I'll adjust my test suite to confirm to Snap's behaviour; I just wanted a case with Content-Length and gzip compression to exercise that code path.

Sorry to trouble you.

AfC

@afcowie

This comment has been minimized.

Show comment
Hide comment
@afcowie

afcowie Dec 30, 2012

Contributor

In the code snippet you posted, if we preserved the content-length the user set after gzipping then we would break the protocol invariants.

Yes, of course, duh. I did mean that I was looking for the compressed size in Content-Length, but that's not the code I wrote. Maybe I should amend the docs for setContentLength to explicitly point out "unless compression is enabled"...

AfC

Contributor

afcowie commented Dec 30, 2012

In the code snippet you posted, if we preserved the content-length the user set after gzipping then we would break the protocol invariants.

Yes, of course, duh. I did mean that I was looking for the compressed size in Content-Length, but that's not the code I wrote. Maybe I should amend the docs for setContentLength to explicitly point out "unless compression is enabled"...

AfC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment