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

Comments

Projects
None yet
4 participants
@mtojek
Copy link

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

This comment has been minimized.

Copy link
Member

commented Feb 21, 2016

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

This comment has been minimized.

Copy link
Contributor

commented Feb 21, 2016

FWIW, a nicer solution to this is possible with the use of cmux. See tamird/cockroach@a513df1#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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Member

commented Feb 23, 2016

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

@mtojek

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link

commented Mar 31, 2016

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

@gopherbot gopherbot closed this in a6557a0 Mar 31, 2016

@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.
You can’t perform that action at this time.