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

x/net/http2: invalid Connection request header #23699

Closed
suconghou opened this issue Feb 5, 2018 · 6 comments
Closed

x/net/http2: invalid Connection request header #23699

suconghou opened this issue Feb 5, 2018 · 6 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@suconghou
Copy link

I meet the error and found the issue here

#15422

and the source code

https://github.com/golang/net/blob/6972930/http2/transport.go#L626-L634

as it said the keep-alive is valid

but why it is case-sensitive ,

many request failed due to Keep-Alive header

does HTTP/2 RFC say it must be case-sensitive ?

@bradfitz
Copy link
Contributor

bradfitz commented Feb 5, 2018

What is your issue?

How did you get this error?

HTTP/2 doesn't have connection-level headers, like the other bug says.

@suconghou
Copy link
Author

@bradfitz I made a proxy for some urls

example code https://gist.github.com/suconghou/50dd30df10bd6faa2abe1805afe3216f

notice it works well then you use chrome , because chrome send keep-alive header

but some other software and services may send Keep-Alive header , eg: cloudflare , IE ( https://stackoverflow.com/questions/9650236/how-to-send-lower-case-keep-alive-header-through-httpwebrequest/30071784 the answers mentioned IE )

use curl -H "Connection:Keep-Alive" http://127.0.0.1:8080/ simulate this behavior

@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 28, 2018
@ianlancetaylor ianlancetaylor added this to the Go1.11 milestone Mar 28, 2018
@suconghou
Copy link
Author

@bradfitz

please replace this

(v != "" && v != "close" && v != "keep-alive") 

with

(v != "" && v != "close" && v != "keep-alive" && v != "Keep-Alive") 

in checkConnHeaders function

@crvv
Copy link
Contributor

crvv commented May 30, 2018

Please read https://httpwg.org/specs/rfc7230.html#rfc.section.6.1

A proxy or gateway MUST parse a received Connection header field before a message is forwarded and, for each connection-option in this field, remove any header field(s) from the message with the same name as the connection-option, and then remove the Connection header field itself (or replace it with the intermediary's own connection options for the forwarded message).

I don't know why Go accepts Connection: keep-alive, which I think is the same as Connection: Keep-Alive. But this is a bug in your proxy, not in http package.

I think it is better to use httputil.ReverseProxy for this.

@szuecs
Copy link

szuecs commented Jul 3, 2018

Doesn't it makes sense to handle this transparently?
I think Go should just drop the Connection header if H2 does not support Connection headers.
I see the same problem if I use plain http.Transport.RoundTrip(req) in case Connection: Keep-Alive is set and the backend called specified in the http.Request supports H2.

For me it sounds really weird to argue you should use httputil.ReverseProxy, instead of fixing this.

@bradfitz bradfitz added the NeedsFix The path to resolution is known, but the work has not been done. label Jul 9, 2018
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 9, 2018
@bradfitz bradfitz self-assigned this Jul 9, 2018
@gopherbot
Copy link

Change https://golang.org/cl/122588 mentions this issue: http2: compare Connection header value case-insensitively

@golang golang locked and limited conversation to collaborators Jul 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

6 participants