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: don't require Host header for "PRI * HTTP/2.0" requests #14451

Closed
mtojek opened this issue Feb 21, 2016 · 6 comments
Closed

net/http: don't require Host header for "PRI * HTTP/2.0" requests #14451

mtojek opened this issue Feb 21, 2016 · 6 comments
Milestone

Comments

@mtojek
Copy link

mtojek commented Feb 21, 2016

Hi,

I have been running two instances of simple HTTP server hidden behind HAProxy configured to terminate SSL and proxy further decrypted traffic.
Unfortunately, the implementation of http.Server connection handling processes HTTP/2.0 requests only when their TLS protos are manifestly defined (through TLSNextProto).
Now, proxying traffic through HAProxy (mode TCP) finishes with 400 Bad Request while PRI * HTTP/2.0 cannot be considered as valid Request-Line.

Best regards,
Marcin

@bradfitz
Copy link
Contributor

There is already bug #14141 open ("x/net/http2: support h2c for http2"), but it sounds like you're speaking neither http2-over-TLS nor h2c.

@tamird did some work recently to support a very similar case. See grpc/grpc-go#555

BTW, PRI * HTTP/2.0 is a valid Request-Line. It's a 400 Bad Request because the lack of Host: ... line.

We should probably special-case the http2 client preface as not requiring a Host header, though.

I'll repurpose this bug for that.

@bradfitz bradfitz changed the title http/h2_bundle: Support HTTP/2.0 over unencrypted HTTP net/http: don't require Host header for "PRI * HTTP/2.0" requests Feb 21, 2016
@bradfitz bradfitz self-assigned this Feb 21, 2016
@bradfitz bradfitz added this to the Go1.7 milestone Feb 21, 2016
@tamird
Copy link
Contributor

tamird commented Feb 21, 2016

FWIW, a nicer solution to this is possible with the use of cmux. See https://github.com/tamird/cockroach/commit/a513df1b1d88e16da8bc01d8b4411eedcb107768#diff-4bf1ae5b9eb22814a15582886403053f for my implementation in CockroachDB where it's used to implement h2c.

The solution referred to in grpc/grpc-go#555 (manually spoofing the host header) is more trouble than it's worth, in my opinion.

@mtojek
Copy link
Author

mtojek commented Feb 21, 2016

I agree with @tamird . Different way of handling HTTP requests in two versions of protocol should be solved by any strategy design pattern, even by multiplexing. I can only advise to consider websocket handling to be included in the multiplexing solution. req.expectsContinue() looks a bit weird...

@bradfitz
Copy link
Contributor

I don't understand, @mtojek, do you no longer need this changed?

@mtojek
Copy link
Author

mtojek commented Feb 23, 2016

@bradfitz
IMO this change is a must. I've just suggested to implement a solution (multiplexing) similar to suggested by @tamird . That's all.

@gopherbot
Copy link

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

@golang golang locked and limited conversation to collaborators Mar 31, 2017
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

4 participants