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 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
Copy link
Contributor

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

@phuslu
Copy link
Author

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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

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

@bobrik
Copy link

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
Copy link
Contributor

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
Development

No branches or pull requests

4 participants