From 02f0ed951f923f96426ee0990be9f0de6b6fd001 Mon Sep 17 00:00:00 2001 From: Areg Harutyunyan Date: Thu, 5 Sep 2019 16:56:28 -0500 Subject: [PATCH] TUN-2276: Path encoding broken --- streamhandler/request.go | 15 +-- streamhandler/request_test.go | 192 ++++++++++++++++++++++++++++++++++ 2 files changed, 197 insertions(+), 10 deletions(-) create mode 100644 streamhandler/request_test.go diff --git a/streamhandler/request.go b/streamhandler/request.go index 40791d06..44b5d86a 100644 --- a/streamhandler/request.go +++ b/streamhandler/request.go @@ -45,16 +45,11 @@ func H2RequestHeadersToH1Request(h2 []h2mux.Header, h1 *http.Request) error { // Otherwise the host header will be based on the origin URL h1.Host = header.Value case ":path": - u, err := url.Parse(header.Value) - if err != nil { - return fmt.Errorf("unparseable path") - } - resolved := h1.URL.ResolveReference(u) - // prevent escaping base URL - if !strings.HasPrefix(resolved.String(), h1.URL.String()) { - return fmt.Errorf("invalid path") - } - h1.URL = resolved + // We can't just set `URL.Path`, because there's no way to ask the library to *not* escape it, + // causing https://github.com/cloudflare/cloudflared/issues/124. + // The only way to bypass the URL escape seems to currently be `URL.Opaque`. + // See https://github.com/golang/go/issues/5777 + h1.URL.Opaque = fmt.Sprintf("//%v%v", h1.URL.Host, header.Value) case "content-length": contentLength, err := strconv.ParseInt(header.Value, 10, 64) if err != nil { diff --git a/streamhandler/request_test.go b/streamhandler/request_test.go new file mode 100644 index 00000000..d88b07dd --- /dev/null +++ b/streamhandler/request_test.go @@ -0,0 +1,192 @@ +package streamhandler + +import ( + "fmt" + "net/http" + "testing" + + "github.com/cloudflare/cloudflared/h2mux" + + "github.com/stretchr/testify/assert" +) + +func TestH2RequestHeadersToH1Request_RegularHeaders(t *testing.T) { + request, err := http.NewRequest(http.MethodGet, "http://example.com", nil) + assert.NoError(t, err) + + headersConversionErr := H2RequestHeadersToH1Request( + []h2mux.Header{ + h2mux.Header{ + Name: "Mock header 1", + Value: "Mock value 1", + }, + h2mux.Header{ + Name: "Mock header 2", + Value: "Mock value 2", + }, + }, + request, + ) + + assert.Equal(t, http.Header{ + "Mock header 1": []string{"Mock value 1"}, + "Mock header 2": []string{"Mock value 2"}, + }, request.Header) + + assert.NoError(t, headersConversionErr) +} + +func TestH2RequestHeadersToH1Request_NoHeaders(t *testing.T) { + request, err := http.NewRequest(http.MethodGet, "http://example.com", nil) + assert.NoError(t, err) + + headersConversionErr := H2RequestHeadersToH1Request( + []h2mux.Header{}, + request, + ) + + assert.Equal(t, http.Header{}, request.Header) + + assert.NoError(t, headersConversionErr) +} + +func TestH2RequestHeadersToH1Request_InvalidHostPath(t *testing.T) { + request, err := http.NewRequest(http.MethodGet, "http://example.com", nil) + assert.NoError(t, err) + + headersConversionErr := H2RequestHeadersToH1Request( + []h2mux.Header{ + h2mux.Header{ + Name: ":path", + Value: "//bad_path/", + }, + h2mux.Header{ + Name: "Mock header", + Value: "Mock value", + }, + }, + request, + ) + + assert.Equal(t, http.Header{ + "Mock header": []string{"Mock value"}, + }, request.Header) + + assert.Equal(t, "http://example.com//bad_path/", request.URL.String()) + + assert.NoError(t, headersConversionErr) +} + +func TestH2RequestHeadersToH1Request_HostPathWithQuery(t *testing.T) { + request, err := http.NewRequest(http.MethodGet, "http://example.com/", nil) + assert.NoError(t, err) + + headersConversionErr := H2RequestHeadersToH1Request( + []h2mux.Header{ + h2mux.Header{ + Name: ":path", + Value: "/?query=mock%20value", + }, + h2mux.Header{ + Name: "Mock header", + Value: "Mock value", + }, + }, + request, + ) + + assert.Equal(t, http.Header{ + "Mock header": []string{"Mock value"}, + }, request.Header) + + assert.Equal(t, "http://example.com/?query=mock%20value", request.URL.String()) + + assert.NoError(t, headersConversionErr) +} + +func TestH2RequestHeadersToH1Request_HostPathWithURLEncoding(t *testing.T) { + request, err := http.NewRequest(http.MethodGet, "http://example.com/", nil) + assert.NoError(t, err) + + headersConversionErr := H2RequestHeadersToH1Request( + []h2mux.Header{ + h2mux.Header{ + Name: ":path", + Value: "/mock%20path", + }, + h2mux.Header{ + Name: "Mock header", + Value: "Mock value", + }, + }, + request, + ) + + assert.Equal(t, http.Header{ + "Mock header": []string{"Mock value"}, + }, request.Header) + + assert.Equal(t, "http://example.com/mock%20path", request.URL.String()) + + assert.NoError(t, headersConversionErr) +} + +func TestH2RequestHeadersToH1Request_WeirdURLs(t *testing.T) { + expectedPaths := []string{ + "", + "/", + "//", + "/%20", + "/ ", + "/ a ", + "/a%20b", + "/foo/bar;param?query#frag", + "/a␠b", + "/a-umlaut-ä", + "/a-umlaut-%C3%A4", + "/a-umlaut-%c3%a4", + "/a#b#c", + "/a#b␠c", + "/a#b%20c", + "/a#b c", + "/\\", + "/a\\", + "/a\\b", + "/a,b.c.", + "/.", + "/a`", + "/a[0]", + "/?a[0]=5 &b[]=", + "/?a=%22b%20%22", + } + + for index, expectedPath := range expectedPaths { + requestURL := "https://example.com" + expectedURL := fmt.Sprintf("https://example.com%v", expectedPath) + + request, err := http.NewRequest(http.MethodGet, requestURL, nil) + assert.NoError(t, err) + + headersConversionErr := H2RequestHeadersToH1Request( + []h2mux.Header{ + h2mux.Header{ + Name: ":path", + Value: expectedPath, + }, + h2mux.Header{ + Name: "Mock header", + Value: "Mock value", + }, + }, + request, + ) + + assert.Equal(t, http.Header{ + "Mock header": []string{"Mock value"}, + }, request.Header) + + assert.Equal(t, expectedURL, request.URL.String(), fmt.Sprintf("Failed URL index: %v", index)) + + assert.NoError(t, headersConversionErr) + } +}