fastcgi: remove dir redirection when useless in php_fastcgi (#6698)

* perf: remove dir redirection when useless in php_fastcgi * fix test * review * fix * fix * simplify * simplify again * restore test * add test
caddyserver · Nov 21, 2024 · eddbccd · eddbccd
1 parent 197c564
commit eddbccd
Show file tree

Hide file tree

Showing 2 changed files with 134 additions and 24 deletions.
diff --git a/...est/integration/caddyfile_adapt/php_fastcgi_try_files_override_no_dir_index.caddyfiletest b/...est/integration/caddyfile_adapt/php_fastcgi_try_files_override_no_dir_index.caddyfiletest
@@ -0,0 +1,94 @@
+:8884
+
+php_fastcgi localhost:9000 {
+	# some php_fastcgi-specific subdirectives
+	split .php .php5
+	env VAR1 value1
+	env VAR2 value2
+	root /var/www
+	try_files {path} index.php
+	dial_timeout 3s
+	read_timeout 10s
+	write_timeout 20s
+
+	# passed through to reverse_proxy (directive order doesn't matter!)
+	lb_policy random
+}
+----------
+{
+	"apps": {
+		"http": {
+			"servers": {
+				"srv0": {
+					"listen": [
+						":8884"
+					],
+					"routes": [
+						{
+							"match": [
+								{
+									"file": {
+										"try_files": [
+											"{http.request.uri.path}",
+											"index.php"
+										],
+										"split_path": [
+											".php",
+											".php5"
+										]
+									}
+								}
+							],
+							"handle": [
+								{
+									"handler": "rewrite",
+									"uri": "{http.matchers.file.relative}"
+								}
+							]
+						},
+						{
+							"match": [
+								{
+									"path": [
+										"*.php",
+										"*.php5"
+									]
+								}
+							],
+							"handle": [
+								{
+									"handler": "reverse_proxy",
+									"load_balancing": {
+										"selection_policy": {
+											"policy": "random"
+										}
+									},
+									"transport": {
+										"dial_timeout": 3000000000,
+										"env": {
+											"VAR1": "value1",
+											"VAR2": "value2"
+										},
+										"protocol": "fastcgi",
+										"read_timeout": 10000000000,
+										"root": "/var/www",
+										"split_path": [
+											".php",
+											".php5"
+										],
+										"write_timeout": 20000000000
+									},
+									"upstreams": [
+										{
+											"dial": "localhost:9000"
+										}
+									]
+								}
+							]
+						}
+					]
+				}
+			}
+		}
+	}
+}
diff --git a/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go b/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go
@@ -179,7 +179,7 @@ func parsePHPFastCGI(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error
 	indexFile := "index.php"
 
 	// set up for explicitly overriding try_files
-	tryFiles := []string{}
+	var tryFiles []string
 
 	// if the user specified a matcher token, use that
 	// matcher in a route that wraps both of our routes;
@@ -310,31 +310,47 @@ func parsePHPFastCGI(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error
 
 	// if the index is turned off, we skip the redirect and try_files
 	if indexFile != "off" {
-		// route to redirect to canonical path if index PHP file
-		redirMatcherSet := caddy.ModuleMap{
-			"file": h.JSON(fileserver.MatchFile{
-				TryFiles: []string{"{http.request.uri.path}/" + indexFile},
-			}),
-			"not": h.JSON(caddyhttp.MatchNot{
-				MatcherSetsRaw: []caddy.ModuleMap{
-					{
-						"path": h.JSON(caddyhttp.MatchPath{"*/"}),
-					},
-				},
-			}),
-		}
-		redirHandler := caddyhttp.StaticResponse{
-			StatusCode: caddyhttp.WeakString(strconv.Itoa(http.StatusPermanentRedirect)),
-			Headers:    http.Header{"Location": []string{"{http.request.orig_uri.path}/"}},
-		}
-		redirRoute := caddyhttp.Route{
-			MatcherSetsRaw: []caddy.ModuleMap{redirMatcherSet},
-			HandlersRaw:    []json.RawMessage{caddyconfig.JSONModuleObject(redirHandler, "handler", "static_response", nil)},
-		}
+		dirRedir := false
+		dirIndex := "{http.request.uri.path}/" + indexFile
 
 		// if tryFiles wasn't overridden, use a reasonable default
 		if len(tryFiles) == 0 {
-			tryFiles = []string{"{http.request.uri.path}", "{http.request.uri.path}/" + indexFile, indexFile}
+			tryFiles = []string{"{http.request.uri.path}", dirIndex, indexFile}
+			dirRedir = true
+		} else {
+			for _, tf := range tryFiles {
+				if tf == dirIndex {
+					dirRedir = true
+
+					break
+				}
+			}
+		}
+
+		if dirRedir {
+			// route to redirect to canonical path if index PHP file
+			redirMatcherSet := caddy.ModuleMap{
+				"file": h.JSON(fileserver.MatchFile{
+					TryFiles: []string{dirIndex},
+				}),
+				"not": h.JSON(caddyhttp.MatchNot{
+					MatcherSetsRaw: []caddy.ModuleMap{
+						{
+							"path": h.JSON(caddyhttp.MatchPath{"*/"}),
+						},
+					},
+				}),
+			}
+			redirHandler := caddyhttp.StaticResponse{
+				StatusCode: caddyhttp.WeakString(strconv.Itoa(http.StatusPermanentRedirect)),
+				Headers:    http.Header{"Location": []string{"{http.request.orig_uri.path}/"}},
+			}
+			redirRoute := caddyhttp.Route{
+				MatcherSetsRaw: []caddy.ModuleMap{redirMatcherSet},
+				HandlersRaw:    []json.RawMessage{caddyconfig.JSONModuleObject(redirHandler, "handler", "static_response", nil)},
+			}
+
+			routes = append(routes, redirRoute)
 		}
 
 		// route to rewrite to PHP index file
@@ -352,7 +368,7 @@ func parsePHPFastCGI(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error
 			HandlersRaw:    []json.RawMessage{caddyconfig.JSONModuleObject(rewriteHandler, "handler", "rewrite", nil)},
 		}
 
-		routes = append(routes, redirRoute, rewriteRoute)
+		routes = append(routes, rewriteRoute)
 	}
 
 	// route to actually reverse proxy requests to PHP files;