From 3ee6d306594606b3f51bc520ab0365b9ce550078 Mon Sep 17 00:00:00 2001 From: Toby Allen Date: Sat, 17 Mar 2018 23:17:42 +0000 Subject: [PATCH] httpserver: Fix #2038 (query string being lost from URI) (#2039) --- caddyhttp/httpserver/server.go | 18 +++++++--- caddyhttp/httpserver/server_test.go | 54 ++++++++++++++++++++--------- 2 files changed, 50 insertions(+), 22 deletions(-) diff --git a/caddyhttp/httpserver/server.go b/caddyhttp/httpserver/server.go index ab65d955f..a98a8fdf3 100644 --- a/caddyhttp/httpserver/server.go +++ b/caddyhttp/httpserver/server.go @@ -422,13 +422,21 @@ func (s *Server) serveHTTP(w http.ResponseWriter, r *http.Request) (int, error) func trimPathPrefix(u *url.URL, prefix string) *url.URL { // We need to use URL.EscapedPath() when trimming the pathPrefix as // URL.Path is ambiguous about / or %2f - see docs. See #1927 - trimmed := strings.TrimPrefix(u.EscapedPath(), prefix) - if !strings.HasPrefix(trimmed, "/") { - trimmed = "/" + trimmed + trimmedPath := strings.TrimPrefix(u.EscapedPath(), prefix) + if !strings.HasPrefix(trimmedPath, "/") { + trimmedPath = "/" + trimmedPath } - trimmedURL, err := url.Parse(trimmed) + // After trimming path reconstruct uri string with Query before parsing + trimmedURI := trimmedPath + if u.RawQuery != "" || u.ForceQuery == true { + trimmedURI = trimmedPath + "?" + u.RawQuery + } + if u.Fragment != "" { + trimmedURI = trimmedURI + "#" + u.Fragment + } + trimmedURL, err := url.Parse(trimmedURI) if err != nil { - log.Printf("[ERROR] Unable to parse trimmed URL %s: %v", trimmed, err) + log.Printf("[ERROR] Unable to parse trimmed URL %s: %v", trimmedURI, err) return u } return trimmedURL diff --git a/caddyhttp/httpserver/server_test.go b/caddyhttp/httpserver/server_test.go index 82926851d..afd29a983 100644 --- a/caddyhttp/httpserver/server_test.go +++ b/caddyhttp/httpserver/server_test.go @@ -129,88 +129,108 @@ func TestMakeHTTPServerWithTimeouts(t *testing.T) { func TestTrimPathPrefix(t *testing.T) { for i, pt := range []struct { - path string + url string prefix string expected string shouldFail bool }{ { - path: "/my/path", + url: "/my/path", prefix: "/my", expected: "/path", shouldFail: false, }, { - path: "/my/%2f/path", + url: "/my/%2f/path", prefix: "/my", expected: "/%2f/path", shouldFail: false, }, { - path: "/my/path", + url: "/my/path", prefix: "/my/", expected: "/path", shouldFail: false, }, { - path: "/my///path", + url: "/my///path", prefix: "/my", expected: "/path", shouldFail: true, }, { - path: "/my///path", + url: "/my///path", prefix: "/my", expected: "///path", shouldFail: false, }, { - path: "/my/path///slash", + url: "/my/path///slash", prefix: "/my", expected: "/path///slash", shouldFail: false, }, { - path: "/my/%2f/path/%2f", + url: "/my/%2f/path/%2f", prefix: "/my", expected: "/%2f/path/%2f", shouldFail: false, }, { - path: "/my/%20/path", + url: "/my/%20/path", prefix: "/my", expected: "/%20/path", shouldFail: false, }, { - path: "/path", + url: "/path", prefix: "", expected: "/path", shouldFail: false, }, { - path: "/path/my/", + url: "/path/my/", prefix: "/my", expected: "/path/my/", shouldFail: false, }, { - path: "", + url: "", prefix: "/my", expected: "/", shouldFail: false, }, { - path: "/apath", + url: "/apath", prefix: "", expected: "/apath", shouldFail: false, + }, { + url: "/my/path/page.php?akey=value", + prefix: "/my", + expected: "/path/page.php?akey=value", + shouldFail: false, + }, { + url: "/my/path/page?key=value#fragment", + prefix: "/my", + expected: "/path/page?key=value#fragment", + shouldFail: false, + }, { + url: "/my/path/page#fragment", + prefix: "/my", + expected: "/path/page#fragment", + shouldFail: false, + }, { + url: "/my/apath?", + prefix: "/my", + expected: "/apath?", + shouldFail: false, }, } { - u, _ := url.Parse(pt.path) - if got, want := trimPathPrefix(u, pt.prefix), pt.expected; got.EscapedPath() != want { + u, _ := url.Parse(pt.url) + if got, want := trimPathPrefix(u, pt.prefix), pt.expected; got.String() != want { if !pt.shouldFail { - t.Errorf("Test %d: Expected='%s', but was '%s' ", i, want, got.EscapedPath()) + t.Errorf("Test %d: Expected='%s', but was '%s' ", i, want, got.String()) } } else if pt.shouldFail { - t.Errorf("SHOULDFAIL Test %d: Expected='%s', and was '%s' but should fail", i, want, got.EscapedPath()) + t.Errorf("SHOULDFAIL Test %d: Expected='%s', and was '%s' but should fail", i, want, got.String()) } } }