Avoid extra slashes in redirects with :splat

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.

server/handler/try.go

@@ -1,6 +1,7 @@
 package handler
 
 import (
+	"fmt"
 	"net/http"
 	"strings"
 
@@ -41,7 +42,7 @@ func tryUpstream(ctx *context.Context, giteaClient *gitea.Client,
 
 	// Try to request the file from the Gitea API
 	if !options.Upstream(ctx, giteaClient, redirectsCache) {
-		html.ReturnErrorPage(ctx, "forge client failed", ctx.StatusCode)
+		html.ReturnErrorPage(ctx, fmt.Sprintf("Forge returned %d %s", ctx.StatusCode, http.StatusText(ctx.StatusCode)), ctx.StatusCode)
 	}
 }
 

server/upstream/redirects.go

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

server/upstream/redirects_test.go

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