pages/pages-server
1
0
Fork 0
mirror of https://codeberg.org/Codeberg/pages-server.git synced 2024-11-18 10:29:43 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

Avoid extra slashes in redirects with :splat (#308)

Browse source 
Remove leading slashes from captured portions of paths when
redirecting using splats. This makes a directive like
"/articles/*  /posts/:splat  302" behave as described in
FEATURES.md, i.e. "/articles/foo" now redirects to
"/posts/foo" rather than to "/posts//foo". Fixes #269.

This also changes the behavior of a redirect like
"/articles/*  /posts:splat  302". "/articles/foo" will now
redirect to "/postsfoo" rather than to "/posts/foo".

This change also fixes an issue where paths like
"/articles123" would be incorrectly matched by the above
patterns.

Reviewed-on: https://codeberg.org/Codeberg/pages-server/pulls/308
Reviewed-by: crapStone <codeberg@crapstone.dev>
Co-authored-by: Daniel Erat <dan@erat.org>
Co-committed-by: Daniel Erat <dan@erat.org>
This commit is contained in:
Daniel Erat 2024-04-20 11:00:15 +00:00 committed by crapStone
parent a986a52755
commit 69fb22a9e7
2 changed files with 17 additions and 15 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
server/upstream/redirects.go
View file

@ -24,14 +24,17 @@ func (r *Redirect) rewriteURL(reqURL string) (dstURL string, ok bool) {
return r.To, true return r.To, true
} }
// handle wildcard redirects // handle wildcard redirects
if strings.HasSuffix(r.From, "/*") {
trimmedFromURL := strings.TrimSuffix(r.From, "/*") trimmedFromURL := strings.TrimSuffix(r.From, "/*")
if strings.HasSuffix(r.From, "/*") && strings.HasPrefix(reqURL, trimmedFromURL) { if reqURL == trimmedFromURL || strings.HasPrefix(reqURL, trimmedFromURL+"/") {
if strings.Contains(r.To, ":splat") { if strings.Contains(r.To, ":splat") {
splatURL := strings.ReplaceAll(r.To, ":splat", strings.TrimPrefix(reqURL, trimmedFromURL)) matched := strings.TrimPrefix(reqURL, trimmedFromURL)
return splatURL, true matched = strings.TrimPrefix(matched, "/")
return strings.ReplaceAll(r.To, ":splat", matched), true
} }
return r.To, true return r.To, true
} }
}
return "", false return "", false
} }

17
server/upstream/redirects_test.go
View file

@ -19,15 +19,14 @@ func TestRedirect_rewriteURL(t *testing.T) {
{Redirect{"/*", "/dst", 200}, "/", "/dst", true}, {Redirect{"/*", "/dst", 200}, "/", "/dst", true},
{Redirect{"/*", "/dst", 200}, "/src", "/dst", true}, {Redirect{"/*", "/dst", 200}, "/src", "/dst", true},
{Redirect{"/src/*", "/dst/:splat", 200}, "/src", "/dst/", true}, {Redirect{"/src/*", "/dst/:splat", 200}, "/src", "/dst/", true},
// TODO: There shouldn't be double-slashes in these URLs: {Redirect{"/src/*", "/dst/:splat", 200}, "/src/", "/dst/", true},
// https://codeberg.org/Codeberg/pages-server/issues/269 {Redirect{"/src/*", "/dst/:splat", 200}, "/src/foo", "/dst/foo", true},
{Redirect{"/src/*", "/dst/:splat", 200}, "/src/", "/dst//", true}, {Redirect{"/src/*", "/dst/:splat", 200}, "/src/foo/bar", "/dst/foo/bar", true},
{Redirect{"/src/*", "/dst/:splat", 200}, "/src/foo", "/dst//foo", true}, {Redirect{"/src/*", "/dst/:splatsuffix", 200}, "/src/foo", "/dst/foosuffix", true},
{Redirect{"/src/*", "/dst/:splat", 200}, "/src/foo/bar", "/dst//foo/bar", true}, {Redirect{"/src/*", "/dst:splat", 200}, "/src/foo", "/dstfoo", true},
// TODO: This is a workaround for the above bug. Should it be preserved? {Redirect{"/src/*", "/dst", 200}, "/srcfoo", "", false},
{Redirect{"/src/*", "/dst:splat", 200}, "/src/foo", "/dst/foo", true}, // This is the example from FEATURES.md:
// TODO: This behavior is incorrect; no redirect should be performed. {Redirect{"/articles/*", "/posts/:splat", 302}, "/articles/2022/10/12/post-1/", "/posts/2022/10/12/post-1/", true},
{Redirect{"/src/*", "/dst", 200}, "/srcfoo", "/dst", true},
} { } {
if dstURL, ok := tc.redirect.rewriteURL(tc.reqURL); dstURL != tc.wantDstURL || ok != tc.wantOk { if dstURL, ok := tc.redirect.rewriteURL(tc.reqURL); dstURL != tc.wantDstURL || ok != tc.wantOk {
t.Errorf("%#v.rewriteURL(%q) = %q, %v; want %q, %v", t.Errorf("%#v.rewriteURL(%q) = %q, %v; want %q, %v",