Skip to content
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

Swindon behavior with 304 (and possible other response codes) #47

Open
tailhook opened this issue Aug 2, 2017 · 3 comments
Open

Swindon behavior with 304 (and possible other response codes) #47

tailhook opened this issue Aug 2, 2017 · 3 comments

Comments

@tailhook
Copy link
Collaborator

tailhook commented Aug 2, 2017

Currently, swindon doesn't expect (and doesn't send) any response even if Transfer-Encoding and Content-Length are specified on such codes. For server part it's easy: just don't send Transfer-Encoding, but client (proxy) behavior is not that easy.

Here is a random subset of tested functionality:

  1. Aiohttp (2.2.0) when serving static responds with Transfer-Encoding: chunked and 0-length chunk (incorrect according to the spec). It looks like node.js has the same problem
  2. Nginx (1.12.1) accepts zero-length chunk if chunked is set. (but see below)
  3. Nginx accepts response without zero-length chunk and chunked too (but see below)
  4. It looks like some version of apache has the same problem as Swindon (but swindon drops bad response with 502 rather than treating the response as http/0.9)
  5. Curl expects the chunk if transfer encoding is specified, and hangs if there is no one
@tailhook tailhook added this to the v0.6.3 milestone Aug 2, 2017
@tailhook
Copy link
Collaborator Author

tailhook commented Aug 2, 2017

Well, under some circumstances nginx forwards zero-length chunk but strips Transfer-Encoding:

13648 recvfrom(10, "HTTP/1.1 304 Not Modified\r\nTransfer-Encoding: chunked\r\nContent-Type: application/octet-stream\r\nDate: Wed, 02 Aug 2017 
14:43:38 GMT\r\nServer: Python/3.6 aiohttp/2.2.0\r\n\r\n0\r\n\r\n", 4096, 0, NULL, NULL) = 173
13648 writev(9, [{"HTTP/1.1 304 Not Modified\r\nServer: nginx/1.12.1\r\nDate: Wed, 02 Aug 2017 14:44:06 GMT\r\nContent-Type: application/octet-s
tream\r\nConnection: keep-alive\r\n\r\n", 152}, {"0\r\n\r\n", 5}], 2) = 157

Which leads to curl issue:

> curl -v http://localhost:8080/304
*   Trying 127.0.0.1...
* Connected to localhost (127.0.0.1) port 8080 (#0)
> GET /304 HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.47.1
> Accept: */*
> 
< HTTP/1.1 304 Not Modified
< Server: nginx/1.12.1
< Date: Wed, 02 Aug 2017 14:44:07 GMT
< Content-Type: application/octet-stream
< Connection: keep-alive
< 
* Excess found in a non pipelined read: excess = 5 url = /304 (zero-length body)
* Connection #0 to host localhost left intact

@tailhook
Copy link
Collaborator Author

tailhook commented Aug 2, 2017

All in all, I think we have two options:

  1. Accept zero-length chunk if exists
  2. Reject as if some garbage was sent after the response (like we do now, but probably should reject earlier?)

The problem is they both work badly with keep-alives:

  1. We don't know if response is finished until response for the next request arrives (i.e. the chunk might be delayed)
  2. Pipelined requests trigger 502, and even without pipelining enabled it can trigger 502 anyway, if last chunk is delayed for a bit

@tailhook
Copy link
Collaborator Author

tailhook commented Aug 2, 2017

Currently looks more like this is a bug in aiohttp, and nothing to do in swindon.

@tailhook tailhook removed this from the v0.6.3 milestone Aug 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant