diff --git a/modules/caddyhttp/rewrite/rewrite.go b/modules/caddyhttp/rewrite/rewrite.go index 690e7526..da922693 100644 --- a/modules/caddyhttp/rewrite/rewrite.go +++ b/modules/caddyhttp/rewrite/rewrite.go @@ -135,13 +135,20 @@ func (rewr Rewrite) Rewrite(r *http.Request, repl *caddy.Replacer) bool { // find the bounds of each part of the URI that exist pathStart, qsStart, fragStart := -1, -1, -1 pathEnd, qsEnd := -1, -1 + loop: for i, ch := range uri { switch { case ch == '?' && qsStart < 0: pathEnd, qsStart = i, i+1 - case ch == '#' && fragStart < 0: - qsEnd, fragStart = i, i+1 - case pathStart < 0 && qsStart < 0 && fragStart < 0: + case ch == '#' && fragStart < 0: // everything after fragment is fragment (very clear in RFC 3986 section 4.2) + if qsStart < 0 { + pathEnd = i + } else { + qsEnd = i + } + fragStart = i + 1 + break loop + case pathStart < 0 && qsStart < 0: pathStart = i } } diff --git a/modules/caddyhttp/rewrite/rewrite_test.go b/modules/caddyhttp/rewrite/rewrite_test.go index 3e2e13da..84dce95e 100644 --- a/modules/caddyhttp/rewrite/rewrite_test.go +++ b/modules/caddyhttp/rewrite/rewrite_test.go @@ -204,6 +204,16 @@ func TestRewrite(t *testing.T) { input: newRequest(t, "GET", "/%C2%B7%E2%88%B5.png?a=b"), expect: newRequest(t, "GET", "/i/%C2%B7%E2%88%B5.png?a=b"), }, + { + rule: Rewrite{URI: "/bar#?"}, + input: newRequest(t, "GET", "/foo#fragFirst?c=d"), // not a valid query string (is part of fragment) + expect: newRequest(t, "GET", "/bar#?"), // I think this is right? but who knows; std lib drops fragment when parsing + }, + { + rule: Rewrite{URI: "/bar"}, + input: newRequest(t, "GET", "/foo#fragFirst?c=d"), + expect: newRequest(t, "GET", "/bar#fragFirst?c=d"), + }, { rule: Rewrite{StripPathPrefix: "/prefix"}, @@ -271,10 +281,11 @@ func TestRewrite(t *testing.T) { } { // copy the original input just enough so that we can // compare it after the rewrite to see if it changed + urlCopy := *tc.input.URL originalInput := &http.Request{ Method: tc.input.Method, RequestURI: tc.input.RequestURI, - URL: &*tc.input.URL, + URL: &urlCopy, } // populate the replacer just enough for our tests