SlyMarbo / spdy

[deprecated] A full-featured SPDY library for the Go language.
BSD 2-Clause "Simplified" License
116 stars 13 forks source link

Clients MUST support gzip #77

Closed jabley closed 9 years ago

jabley commented 9 years ago

From the spec – http://www.chromium.org/spdy/spdy-protocol/spdy-protocol-draft3-1

“User-agents MUST support gzip compression. Regardless of the Accept-Encoding sent by the user-agent, the server may always send content encoded with gzip or deflate encoding.”

This change adds transparent decompression of gripped content.

I found this when I modified client code to use SPDY to talk to an origin server. The origin is using nginx. I was getting errors when trying to unmarshall JSON:

invalid character '\\x1f' looking for beginning of value

Enabling debug logging, I saw the request:

Header:               map[Url:[/public/dashboards?slug=carers-allowance] Version:[HTTP/1.1] Host:[example.gov.uk] Scheme:[https] Accept:[application/json] User-Agent:[Performance-Platform-Client/1.0]  Method:[GET]]

and a response:

map[Status:[200] Content-Type:[application/json] Via:[1.1 varnish] Date:[Thu, 12 Feb 2015 19:52:58 GMT] Cache-Control:[max-age=300] Access-Control-Allow-Origin:[*] X-Frame-Options:[SAMEORIGIN] X-Varnish:[1873677853 1873677852] Age:[0] X-Cache:[HIT] Content-Encoding:[gzip] Version:[HTTP/1.1] Server:[nginx] Alternate-Protocol:[443:npn-spdy/2 443:npn-spdy/2] Strict-Transport-Security:[max-age=2628000]]
Data:                 [1f 8b 08 00 00 00 00 00 00 ... 05 af e6 9f 47 2a 3c 00 00]

This shows that the response is coming back compressed, even though the client didn’t ask for that.

jabley commented 9 years ago

Seems to be a resource leak somewhere. Seeing

Error: Max concurrent streams limit exceeded. and not sure if this is due to my change.

jabley commented 9 years ago

Resource leak isn't due to this change. It's an existing thing that I'll open a separate issue for.

SlyMarbo commented 9 years ago

I'm not too keen to add transparent decompression, since "net/http" doesn't (see "net/http.Transport.DisableCompression"'s documentation) and the client may wish to store the compressed version directly, without having to recompress after transparent decompression.

jabley commented 9 years ago

What about a different patch?

In this instance, responses were compressed even though the client didn't ask for compressed responses. It feels like a client shouldn't have to decompress that response.

SlyMarbo commented 9 years ago

Ah, fair point, I'll get to work

jabley commented 9 years ago

Tweaked patch that checks the request to see if the Accept-Encoding header was set.

SlyMarbo commented 9 years ago

Sorry, didn't see you'd also done roughly the same thing. Thanks a lot for your help; much appreciated =)

jabley commented 9 years ago

np, thanks for fixing it.