You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
As mentioned over at #1294 (comment), I wondered why BkBasicTest.handlesTwoRequestInOneConnection() succeeds, despite the Content-Length of the first request not counting the \r\n that Joined() inserts between "Hello First" and "POST" .
So it should be 13, not 11, which it is now:
it turns out the second request has no headers, except for the synthetic ones added by Takes. Instead, the headers end up in the body. To wit:
1st request
-------- REQUEST HEAD --------
POST / HTTP/1.1
Host:localhost
Content-Length: 11
X-Takes-LocalAddress: 192.168.32.222
X-Takes-LocalPort: 61791
X-Takes-RemoteAddress: 192.168.32.222
X-Takes-RemotePort: 61792
-------- REQUEST BODY --------
Hello First
2nd request
-------- REQUEST HEAD --------
X-Takes-LocalAddress: 192.168.32.222
X-Takes-LocalPort: 61791
X-Takes-RemoteAddress: 192.168.32.222
X-Takes-RemotePort: 61792
-------- REQUEST BODY --------
POST / HTTP/1.1
Host:localhost
Content-Length: 12
Hello Second
When changing the content length of the first request to the correct value of 13, headers of the second request will be parsed correctly.
I think in case of \r or \n preceding the http method, Takes should either
disconnect, because the request is invalid
discard those leading \r and \n characters and otherwise parse the request as normal
I just checked google.com's "gws" server and an nginx-alpine docker image: They both implement 2., discarding leading \r and \n characters, regardless of how many there are and in what order:
> echo -ne 'GET / HTTP/1.1\r\nHost: google.com\r\n\r\n' | nc google.com 80 | head -1
HTTP/1.1 301 Moved Permanently
> echo -ne '\rGET / HTTP/1.1\r\nHost: google.com\r\n\r\n' | nc google.com 80 | head -1
HTTP/1.1 301 Moved Permanently
> echo -ne '\nGET / HTTP/1.1\r\nHost: google.com\r\n\r\n' | nc google.com 80 | head -1
HTTP/1.1 301 Moved Permanently
> echo -ne '\r\nGET / HTTP/1.1\r\nHost: google.com\r\n\r\n' | nc google.com 80 | head -1
HTTP/1.1 301 Moved Permanently
> echo -ne '\r\n\r\r\n\n\nGET / HTTP/1.1\r\nHost: google.com\r\n\r\n' | nc google.com 80 | head -1
HTTP/1.1 301 Moved Permanently
But they don't discard a leading space:
> echo -ne ' GET / HTTP/1.1\r\nHost: google.com\r\n\r\n' | nc google.com 80 | head -1
HTTP/1.0 400 Bad Request
So, I suggest that Takes follow the models of gws and nginx and discard leading \r and \n.
The text was updated successfully, but these errors were encountered:
As mentioned over at #1294 (comment), I wondered why
BkBasicTest.handlesTwoRequestInOneConnection()
succeeds, despite the Content-Length of the first request not counting the \r\n that Joined() inserts between "Hello First" and "POST" .So it should be 13, not 11, which it is now:
When you change the test to print head and body like so:
it turns out the second request has no headers, except for the synthetic ones added by Takes. Instead, the headers end up in the body. To wit:
1st request
2nd request
When changing the content length of the first request to the correct value of 13, headers of the second request will be parsed correctly.
I think in case of \r or \n preceding the http method, Takes should either
I just checked google.com's "gws" server and an nginx-alpine docker image: They both implement 2., discarding leading \r and \n characters, regardless of how many there are and in what order:
But they don't discard a leading space:
So, I suggest that Takes follow the models of gws and nginx and discard leading \r and \n.
The text was updated successfully, but these errors were encountered: