author | Brad Fitzpatrick
<bradfitz@golang.org> 2016-08-23 20:08:20 UTC |
committer | Brad Fitzpatrick
<bradfitz@golang.org> 2016-08-25 02:14:58 UTC |
parent | 7394c112eae4dba7e96bfcfe738e6373d61772b4 |
http2/http2.go | +10 | -0 |
http2/transport.go | +17 | -1 |
http2/transport_test.go | +129 | -0 |
diff --git a/http2/http2.go b/http2/http2.go index f06e87b..401923b 100644 --- a/http2/http2.go +++ b/http2/http2.go @@ -350,3 +350,13 @@ func (s *sorter) SortStrings(ss []string) { sort.Sort(s) s.v = save } + +// validPseudoPath reports whether v is a valid :path pseudo-header +// value. It must be a non-empty string starting with '/', and not +// start with two slashes. +// For now this is only used a quick check for deciding when to clean +// up Opaque URLs before sending requests from the Transport. +// See golang.org/issue/16847 +func validPseudoPath(v string) bool { + return len(v) > 0 && v[0] == '/' && (len(v) == 1 || v[1] != '/') +} diff --git a/http2/transport.go b/http2/transport.go index ccb885f..bcd27ae 100644 --- a/http2/transport.go +++ b/http2/transport.go @@ -998,6 +998,22 @@ func (cc *ClientConn) encodeHeaders(req *http.Request, addGzipHeader bool, trail host = req.URL.Host } + var path string + if req.Method != "CONNECT" { + path = req.URL.RequestURI() + if !validPseudoPath(path) { + orig := path + path = strings.TrimPrefix(path, req.URL.Scheme+"://"+host) + if !validPseudoPath(path) { + if req.URL.Opaque != "" { + return nil, fmt.Errorf("invalid request :path %q from URL.Opaque = %q", orig, req.URL.Opaque) + } else { + return nil, fmt.Errorf("invalid request :path %q", orig) + } + } + } + } + // Check for any invalid headers and return an error before we // potentially pollute our hpack state. (We want to be able to // continue to reuse the hpack encoder for future requests) @@ -1020,7 +1036,7 @@ func (cc *ClientConn) encodeHeaders(req *http.Request, addGzipHeader bool, trail cc.writeHeader(":authority", host) cc.writeHeader(":method", req.Method) if req.Method != "CONNECT" { - cc.writeHeader(":path", req.URL.RequestURI()) + cc.writeHeader(":path", path) cc.writeHeader(":scheme", "https") } if trailers != "" { diff --git a/http2/transport_test.go b/http2/transport_test.go index 1bc5c4b..a7cce94 100644 --- a/http2/transport_test.go +++ b/http2/transport_test.go @@ -2468,3 +2468,132 @@ func TestTransportBodyDoubleEndStream(t *testing.T) { defer res.Body.Close() } } + +// golangorg/issue/16847 +func TestTransportRequestPathPseudo(t *testing.T) { + type result struct { + path string + err string + } + tests := []struct { + req *http.Request + want result + }{ + 0: { + req: &http.Request{ + Method: "GET", + URL: &url.URL{ + Host: "foo.com", + Path: "/foo", + }, + }, + want: result{path: "/foo"}, + }, + // I guess we just don't let users request "//foo" as + // a path, since it's illegal to start with two + // slashes.... + 1: { + req: &http.Request{ + Method: "GET", + URL: &url.URL{ + Host: "foo.com", + Path: "//foo", + }, + }, + want: result{err: `invalid request :path "//foo"`}, + }, + + // Opaque with //$Matching_Hostname/path + 2: { + req: &http.Request{ + Method: "GET", + URL: &url.URL{ + Scheme: "https", + Opaque: "//foo.com/path", + Host: "foo.com", + Path: "/ignored", + }, + }, + want: result{path: "/path"}, + }, + + // Opaque with some other Request.Host instead: + 3: { + req: &http.Request{ + Method: "GET", + Host: "bar.com", + URL: &url.URL{ + Scheme: "https", + Opaque: "//bar.com/path", + Host: "foo.com", + Path: "/ignored", + }, + }, + want: result{path: "/path"}, + }, + + // Opaque without the leading "//": + 4: { + req: &http.Request{ + Method: "GET", + URL: &url.URL{ + Opaque: "/path", + Host: "foo.com", + Path: "/ignored", + }, + }, + want: result{path: "/path"}, + }, + + // Opaque we can't handle: + 5: { + req: &http.Request{ + Method: "GET", + URL: &url.URL{ + Scheme: "https", + Opaque: "//unknown_host/path", + Host: "foo.com", + Path: "/ignored", + }, + }, + want: result{err: `invalid request :path "https://unknown_host/path" from URL.Opaque = "//unknown_host/path"`}, + }, + + // A CONNECT request: + 6: { + req: &http.Request{ + Method: "CONNECT", + URL: &url.URL{ + Host: "foo.com", + }, + }, + want: result{}, + }, + } + for i, tt := range tests { + cc := &ClientConn{} + cc.henc = hpack.NewEncoder(&cc.hbuf) + cc.mu.Lock() + hdrs, err := cc.encodeHeaders(tt.req, false, "", -1) + cc.mu.Unlock() + var got result + hpackDec := hpack.NewDecoder(initialHeaderTableSize, func(f hpack.HeaderField) { + if f.Name == ":path" { + got.path = f.Value + } + }) + if err != nil { + got.err = err.Error() + } else if len(hdrs) > 0 { + if _, err := hpackDec.Write(hdrs); err != nil { + t.Errorf("%d. bogus hpack: %v", i, err) + continue + } + } + if got != tt.want { + t.Errorf("%d. got %+v; want %+v", i, got, tt.want) + } + + } + +}