From 5e9d81b507e0beb46b3812e21566bfef79c87af4 Mon Sep 17 00:00:00 2001
From: Matthew Holt <mholt@users.noreply.github.com>
Date: Thu, 12 Dec 2019 15:27:09 -0700
Subject: try_files, rewrite: allow query string in try_files (fix #2891)

Also some minor cleanup/improvements discovered along the way
---
 modules/caddyhttp/fileserver/caddyfile.go          | 52 +++++++++++++++++-----
 .../caddyhttp/reverseproxy/fastcgi/caddyfile.go    | 10 ++---
 modules/caddyhttp/rewrite/rewrite.go               |  5 ---
 3 files changed, 47 insertions(+), 20 deletions(-)

(limited to 'modules/caddyhttp')

diff --git a/modules/caddyhttp/fileserver/caddyfile.go b/modules/caddyhttp/fileserver/caddyfile.go
index fb931a1..8013fa2 100644
--- a/modules/caddyhttp/fileserver/caddyfile.go
+++ b/modules/caddyhttp/fileserver/caddyfile.go
@@ -15,6 +15,8 @@
 package fileserver
 
 import (
+	"strings"
+
 	"github.com/caddyserver/caddy/v2"
 	"github.com/caddyserver/caddy/v2/caddyconfig/httpcaddyfile"
 	"github.com/caddyserver/caddy/v2/modules/caddyhttp"
@@ -98,7 +100,7 @@ func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error)
 //
 //    try_files <files...>
 //
-// and is shorthand for:
+// and is basically shorthand for:
 //
 //    matcher:try_files {
 //        file {
@@ -107,25 +109,55 @@ func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error)
 //    }
 //    rewrite match:try_files {http.matchers.file.relative}{http.request.uri.query_string}
 //
+// If any of the files in the list have a query string, the query string will
+// be ignored when checking for file existence, but will be augmented into
+// the request's URI when rewriting the request.
 func parseTryFiles(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) {
 	if !h.Next() {
 		return nil, h.ArgErr()
 	}
 
-	try := h.RemainingArgs()
-	if len(try) == 0 {
+	tryFiles := h.RemainingArgs()
+	if len(tryFiles) == 0 {
 		return nil, h.ArgErr()
 	}
 
-	handler := rewrite.Rewrite{
-		URI: "{http.matchers.file.relative}{http.request.uri.query_string}",
+	// makeRoute returns a route that tries the files listed in try
+	// and then rewrites to the matched file, and appends writeURIAppend
+	// to the end of the query string.
+	makeRoute := func(try []string, writeURIAppend string) []httpcaddyfile.ConfigValue {
+		handler := rewrite.Rewrite{
+			URI: "{http.matchers.file.relative}{http.request.uri.query_string}" + writeURIAppend,
+		}
+		matcherSet := caddy.ModuleMap{
+			"file": h.JSON(MatchFile{
+				TryFiles: try,
+			}),
+		}
+		return h.NewRoute(matcherSet, handler)
 	}
 
-	matcherSet := caddy.ModuleMap{
-		"file": h.JSON(MatchFile{
-			TryFiles: try,
-		}, nil),
+	var result []httpcaddyfile.ConfigValue
+
+	// if there are query strings in the list, we have to split into
+	// a separate route for each item with a query string, because
+	// the rewrite is different for that item
+	var try []string
+	for _, item := range tryFiles {
+		if idx := strings.Index(item, "?"); idx >= 0 {
+			if len(try) > 0 {
+				result = append(result, makeRoute(try, "")...)
+				try = []string{}
+			}
+			result = append(result, makeRoute([]string{item[:idx]}, "&"+item[idx+1:])...)
+			continue
+		}
+		// accumulate consecutive non-query-string parameters
+		try = append(try, item)
+	}
+	if len(try) > 0 {
+		result = append(result, makeRoute(try, "")...)
 	}
 
-	return h.NewRoute(matcherSet, handler), nil
+	return result, nil
 }
diff --git a/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go b/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go
index 8e723b2..699f680 100644
--- a/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go
+++ b/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go
@@ -125,12 +125,12 @@ func parsePHPFastCGI(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error
 	redirMatcherSet := caddy.ModuleMap{
 		"file": h.JSON(fileserver.MatchFile{
 			TryFiles: []string{"{http.request.uri.path}/index.php"},
-		}, nil),
+		}),
 		"not": h.JSON(caddyhttp.MatchNegate{
 			MatchersRaw: caddy.ModuleMap{
-				"path": h.JSON(caddyhttp.MatchPath{"*/"}, nil),
+				"path": h.JSON(caddyhttp.MatchPath{"*/"}),
 			},
-		}, nil),
+		}),
 	}
 	redirHandler := caddyhttp.StaticResponse{
 		StatusCode: caddyhttp.WeakString("308"),
@@ -145,7 +145,7 @@ func parsePHPFastCGI(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error
 	rewriteMatcherSet := caddy.ModuleMap{
 		"file": h.JSON(fileserver.MatchFile{
 			TryFiles: []string{"{http.request.uri.path}", "{http.request.uri.path}/index.php", "index.php"},
-		}, nil),
+		}),
 	}
 	rewriteHandler := rewrite.Rewrite{
 		URI:      "{http.matchers.file.relative}{http.request.uri.query_string}",
@@ -159,7 +159,7 @@ func parsePHPFastCGI(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error
 	// route to actually reverse proxy requests to PHP files;
 	// match only requests that are for PHP files
 	rpMatcherSet := caddy.ModuleMap{
-		"path": h.JSON([]string{"*.php"}, nil),
+		"path": h.JSON([]string{"*.php"}),
 	}
 
 	// if the user specified a matcher token, use that
diff --git a/modules/caddyhttp/rewrite/rewrite.go b/modules/caddyhttp/rewrite/rewrite.go
index a142080..adb90f4 100644
--- a/modules/caddyhttp/rewrite/rewrite.go
+++ b/modules/caddyhttp/rewrite/rewrite.go
@@ -190,11 +190,6 @@ func (rep replacer) do(r *http.Request, repl caddy.Replacer) bool {
 	r.URL.Path = strings.Replace(oldPath, find, replace, lim)
 	r.URL.RawQuery = strings.Replace(oldQuery, find, replace, lim)
 
-	// changed := r.URL.Path != oldPath && r.URL.RawQuery != oldQuery
-	// if changed {
-	// 	r.RequestURI = r.URL.RequestURI()
-	// }
-
 	return r.URL.Path != oldPath && r.URL.RawQuery != oldQuery
 }
 
-- 
cgit v1.2.3