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: transport checkConnHeaders should case-insensitive #15422

Closed
phuslu opened this issue Apr 23, 2016 · 7 comments
Closed

x/net/http2: transport checkConnHeaders should case-insensitive #15422

phuslu opened this issue Apr 23, 2016 · 7 comments

Comments

@phuslu
Copy link

@phuslu phuslu commented Apr 23, 2016

Please answer these questions before submitting your issue. Thanks!

  1. What version of Go are you using (go version)?
    go tip
  2. What operating system and processor architecture are you using (go env)?
    set GOARCH=amd64
    set GOBIN=
    set GOEXE=.exe
    set GOHOSTARCH=amd64
    set GOHOSTOS=windows
    set GOOS=windows
    set GOPATH=D:\gopath
    set GORACE=
    set GOROOT=D:\go
    set GOTOOLDIR=D:\go\pkg\tool\windows_amd64
    set CC=gcc
    set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0 -fdebug-prefix-map=C:\Users\phuslu\AppData\Local\Temp\go-build318681378=/tmp/go-build -gno-record-gcc-switches
    set CXX=g++
    set CGO_ENABLED=1
  3. What did you do?
    Write a google reverse proxy with x/net/http2, it convert request from http1 to http2 when browser visit google sites.
  4. What did you expect to see?
    Browsers should works well with the reverse proxy.
  5. What did you see instead?
    IE11 could not pass the proxy. the error is http2: invalid Connection request header
    Indeed, IE Sent Connection: Keep-Alive to the proxy and failed in checkConnHeaders
@bradfitz

This comment has been minimized.

Copy link
Contributor

@bradfitz bradfitz commented Apr 23, 2016

This was fixed already in https://golang.org/cl/19223

@bradfitz bradfitz closed this Apr 23, 2016
@phuslu

This comment has been minimized.

Copy link
Author

@phuslu phuslu commented Apr 24, 2016

@bradfitz Thanks. But IE11 sent Connection: Keep-Alive, not Connection: keep-alive, Is it a bug of http2, or my proxy should handle it at first?

@bradfitz

This comment has been minimized.

Copy link
Contributor

@bradfitz bradfitz commented Apr 24, 2016

It's not a bug in http2. If IE11 sent that over http2, it should be rejected. If your proxy generated that over http2, it's a bug in the proxy. But whose bug in the proxy depends on what the code of your proxy looks like, but you didn't provide the code. If you use httputil.ReverseProxy, it should already be removing those. See its var hopHeaders = ... list.

@phuslu

This comment has been minimized.

Copy link
Author

@phuslu phuslu commented Apr 26, 2016

Thanks @bradfitz clarify, I add a "fix" in my proxy here https://github.com/phuslu/goproxy/commit/c45cec8a321602378876bdf6286ee0e9bf207c06

But I still think we should treat Connection: Keep-Alive as a valid header in http2 checkConnHeaders function [1]

[1]: RFC 2068 section 19.7.1

@bradfitz

This comment has been minimized.

Copy link
Contributor

@bradfitz bradfitz commented Apr 26, 2016

@phuslu, you referenced an HTTP/1.x RFC. The HTTP/2 RFC explicitly prohibits connection-level headers.

@bobrik

This comment has been minimized.

Copy link

@bobrik bobrik commented Sep 30, 2016

@bradfitz is there a reason why the error can't include the actual value of the invalid header? It'd be much easier to debug:

@bradfitz

This comment has been minimized.

Copy link
Contributor

@bradfitz bradfitz commented Sep 30, 2016

No major reason.

@golang golang locked and limited conversation to collaborators Sep 30, 2017
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
4 participants
You can’t perform that action at this time.