http2: don't send bogus :path pseudo headers if Request.URL.Opaque is set

Fixes golang/go#16847

Change-Id: I1e6ae1e0746051fd4cf7afc9beae7853293d5b68
Reviewed-on: https://go-review.googlesource.com/27632
Reviewed-by: Chris Broadfoot <[email protected]>

Author: bradfitz

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] != '/')
+}

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 != "" {

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)
+		}
+
+	}
+
+}