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
Assignees
Milestone

Comments

@suconghou
Copy link

@suconghou suconghou commented Feb 5, 2018

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

This comment has been minimized.

Copy link
Contributor

@bradfitz 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

This comment has been minimized.

Copy link
Author

@suconghou suconghou commented Feb 6, 2018

@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 this to the Go1.11 milestone Mar 28, 2018
@suconghou

This comment has been minimized.

Copy link
Author

@suconghou suconghou commented May 2, 2018

@bradfitz

please replace this

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

with

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

in checkConnHeaders function

@crvv

This comment has been minimized.

Copy link
Contributor

@crvv 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

This comment has been minimized.

Copy link

@szuecs 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.

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Jul 9, 2018

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.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.