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

net/http: HTTP Server messed up pipelined HTTP requests after POST request #10876

Closed
chenxiaoqino opened this issue May 16, 2015 · 10 comments
Closed
Milestone

Comments

@chenxiaoqino
Copy link

By default, Go's HTTP server (from package net/http) supports HTTP/1.1 pipelined requests (Connection: keep-alive) and will dispatch each request to a new handler Go-routine (before encountering any POST requests).

However, after the first POST request encountered in a pipeline, the Go HTTP server failed to parse the following requests, either GET or POST, and will reply with a "400 Bad Request".

A simple demo for this misbehavior: (assume we already launched the server on port 8080)

printf "\
POST / HTTP/1.1\r\n\
Host: localhost\r\n\
Connection: keep-alive\r\n\
Content-Type: application/x-www-form-urlencoded\r\n\
Content-Length: 7\r\n\
\r\n\
A=a&B=b\r\n\
\r\n\
GET / HTTP/1.1\r\n\
Host: localhost\r\n\
Connection: keep-alive\r\n\
\r\n\
GET / HTTP/1.1\r\n\
Host: localhost\r\n\
Connection: keep-alive\r\n\
\r\n\
" | nc localhost 8080

Version&architecture:I observed this phenomenon on "go version go1.4.2 windows/amd64"; the architecture shouldn't be important though. Similar behavior appears in Mac version also. This should be a package's bug.

Expected result: Other traditional HTTP servers (including Nginx/Apache, and default server in Node.js) all handled this correctly. The expected result should be three consecutive "HTTP/1.1 200 OK"s (with contents respectively).

Observed result: using the example minimal net/http server as seen in go ducument's "Writing Web Applications", the response to above pipelined requests is:

HTTP/1.1 200 OK
Date: Sat, 16 May 2015 05:09:19 GMT
Content-Length: 18
Content-Type: text/plain; charset=utf-8

Hi there, I love !HTTP/1.1 400 Bad Request

(close: No error)
@chenxiaoqino
Copy link
Author

Personally, I suspect this is due to request.go#741 , the ReadAll() may affect later requests. Perhaps there need add a missing header.get("Content-Length"). However, I haven't looked into the server implementation thoroughly.

@dspezia
Copy link
Contributor

dspezia commented May 16, 2015

I think it is not due to the implementation of parsePostForm but rather to the bogus \r\n you have added after the message body of the POST query. See https://www.ietf.org/rfc/rfc2616.txt section 4.1

It says:

In the interest of robustness, servers SHOULD ignore any empty
line(s) received where a Request-Line is expected. In other words, if
the server is reading the protocol stream at the beginning of a
message and receives a CRLF first, it should ignore the CRLF.

Certain buggy HTTP/1.0 client implementations generate extra CRLF's
after a POST request. To restate what is explicitly forbidden by the
BNF, an HTTP/1.1 client MUST NOT preface or follow a request with an
extra CRLF.

The Go http package SHOULD ignore the CRLF, but does not. The Go implementation could be made more robust by ignoring those CRLF, but what it currently does is still correct.

@chenxiaoqino
Copy link
Author

Yes, you're right, the correct request (which Go server will now accept) is

printf "\
POST / HTTP/1.1\r\n\
Host: localhost\r\n\
Connection: keep-alive\r\n\
Content-Type: application/x-www-form-urlencoded\r\n\
Content-Length: 7\r\n\
\r\n\
A=a&B=bGET / HTTP/1.1\r\n\
Host: localhost\r\n\
Connection: keep-alive\r\n\
\r\n\
GET / HTTP/1.1\r\n\
Host: localhost\r\n\
Connection: keep-alive\r\n\
\r\n\
" | nc localhost 8080

Still, please consider ignoring the carriage return (as it'll be much more convenient for sending request over command prompt).

@dspezia
Copy link
Contributor

dspezia commented May 16, 2015

Probably, @bradfitz to decide.

@dspezia
Copy link
Contributor

dspezia commented May 16, 2015

Mailing a CL in case he wants it.

@mikioh
Copy link
Contributor

mikioh commented May 16, 2015

Please refer http://tools.ietf.org/html/rfc7230#section-3 instead of obsoleted RFC 2616.

@gopherbot
Copy link

CL https://golang.org/cl/10170 mentions this issue.

@dspezia
Copy link
Contributor

dspezia commented May 16, 2015

Ah yes, sorry. In RFC 7230, this recommendation is in section 3.5.
http://tools.ietf.org/html/rfc7230#section-3.5
Updating the CL.

@bradfitz
Copy link
Contributor

My previous HTTP load balancer (https://github.com/perlbal/Perlbal) ignored that extra \r\n after a POST, but I thought that one buggy browser (Netscape Navigator something) died out 10+ years ago.

Is there an actual HTTP client which still is broken like this?

I would really rather be strict unless it would actually help an important client.

Or we could at least detect it and return an error message informing the user that their HTTP client is buggy and old.

@gopherbot
Copy link

CL https://golang.org/cl/11491 mentions this issue.

@mikioh mikioh modified the milestones: Go1.5, Go1.5Maybe Jun 25, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants