Commit 3ee6d306 authored by Toby Allen's avatar Toby Allen Committed by Matt Holt

httpserver: Fix #2038 (query string being lost from URI) (#2039)

parent 6e2de19d
...@@ -422,13 +422,21 @@ func (s *Server) serveHTTP(w http.ResponseWriter, r *http.Request) (int, error) ...@@ -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 { func trimPathPrefix(u *url.URL, prefix string) *url.URL {
// We need to use URL.EscapedPath() when trimming the pathPrefix as // We need to use URL.EscapedPath() when trimming the pathPrefix as
// URL.Path is ambiguous about / or %2f - see docs. See #1927 // URL.Path is ambiguous about / or %2f - see docs. See #1927
trimmed := strings.TrimPrefix(u.EscapedPath(), prefix) trimmedPath := strings.TrimPrefix(u.EscapedPath(), prefix)
if !strings.HasPrefix(trimmed, "/") { if !strings.HasPrefix(trimmedPath, "/") {
trimmed = "/" + trimmed 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 { 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 u
} }
return trimmedURL return trimmedURL
......
...@@ -129,88 +129,108 @@ func TestMakeHTTPServerWithTimeouts(t *testing.T) { ...@@ -129,88 +129,108 @@ func TestMakeHTTPServerWithTimeouts(t *testing.T) {
func TestTrimPathPrefix(t *testing.T) { func TestTrimPathPrefix(t *testing.T) {
for i, pt := range []struct { for i, pt := range []struct {
path string url string
prefix string prefix string
expected string expected string
shouldFail bool shouldFail bool
}{ }{
{ {
path: "/my/path", url: "/my/path",
prefix: "/my", prefix: "/my",
expected: "/path", expected: "/path",
shouldFail: false, shouldFail: false,
}, },
{ {
path: "/my/%2f/path", url: "/my/%2f/path",
prefix: "/my", prefix: "/my",
expected: "/%2f/path", expected: "/%2f/path",
shouldFail: false, shouldFail: false,
}, },
{ {
path: "/my/path", url: "/my/path",
prefix: "/my/", prefix: "/my/",
expected: "/path", expected: "/path",
shouldFail: false, shouldFail: false,
}, },
{ {
path: "/my///path", url: "/my///path",
prefix: "/my", prefix: "/my",
expected: "/path", expected: "/path",
shouldFail: true, shouldFail: true,
}, },
{ {
path: "/my///path", url: "/my///path",
prefix: "/my", prefix: "/my",
expected: "///path", expected: "///path",
shouldFail: false, shouldFail: false,
}, },
{ {
path: "/my/path///slash", url: "/my/path///slash",
prefix: "/my", prefix: "/my",
expected: "/path///slash", expected: "/path///slash",
shouldFail: false, shouldFail: false,
}, },
{ {
path: "/my/%2f/path/%2f", url: "/my/%2f/path/%2f",
prefix: "/my", prefix: "/my",
expected: "/%2f/path/%2f", expected: "/%2f/path/%2f",
shouldFail: false, shouldFail: false,
}, { }, {
path: "/my/%20/path", url: "/my/%20/path",
prefix: "/my", prefix: "/my",
expected: "/%20/path", expected: "/%20/path",
shouldFail: false, shouldFail: false,
}, { }, {
path: "/path", url: "/path",
prefix: "", prefix: "",
expected: "/path", expected: "/path",
shouldFail: false, shouldFail: false,
}, { }, {
path: "/path/my/", url: "/path/my/",
prefix: "/my", prefix: "/my",
expected: "/path/my/", expected: "/path/my/",
shouldFail: false, shouldFail: false,
}, { }, {
path: "", url: "",
prefix: "/my", prefix: "/my",
expected: "/", expected: "/",
shouldFail: false, shouldFail: false,
}, { }, {
path: "/apath", url: "/apath",
prefix: "", prefix: "",
expected: "/apath", expected: "/apath",
shouldFail: false, 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) u, _ := url.Parse(pt.url)
if got, want := trimPathPrefix(u, pt.prefix), pt.expected; got.EscapedPath() != want { if got, want := trimPathPrefix(u, pt.prefix), pt.expected; got.String() != want {
if !pt.shouldFail { 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 { } 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())
} }
} }
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment