fix rewrite path for prefix match

nginxinc · Dec 23, 2024 · 2b5297c · 2b5297c
1 parent 938b7ff
commit 2b5297c
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 13 deletions.
diff --git a/internal/mode/static/nginx/config/servers.go b/internal/mode/static/nginx/config/servers.go
@@ -620,22 +620,24 @@ func createRewritesValForRewriteFilter(filter *dataplane.HTTPURLRewriteFilter, p
 				filterPrefix = "/"
 			}
 
-			// capture everything after the configured prefix
-			regex := fmt.Sprintf("^%s(.*)$", path)
-			// replace the configured prefix with the filter prefix and append what was captured
-			replacement := fmt.Sprintf("%s$1", filterPrefix)
+			// capture everything after the configured prefix up to the first ? (if present)
+			regex := fmt.Sprintf("^%s([^?]*)?", path)
+			// replace the configured prefix with the filter prefix, append the captured segment,
+			// and add the query string store in nginx variable $args
+			// (see: https://nginx.org/en/docs/http/ngx_http_core_module.html#var_args).
+			replacement := fmt.Sprintf("%s$1?$args?", filterPrefix)
 
 			// if configured prefix does not end in /, but replacement prefix does end in /,
 			// then make sure that we *require* but *don't capture* a trailing slash in the request,
 			// otherwise we'll get duplicate slashes in the full replacement
 			if strings.HasSuffix(filterPrefix, "/") && !strings.HasSuffix(path, "/") {
-				regex = fmt.Sprintf("^%s(?:/(.*))?$", path)
+				regex = fmt.Sprintf("^%s(?:/([^?]*))?", path)
 			}
 
 			// if configured prefix ends in / we won't capture it for a request (since it's not in the regex),
 			// so append it to the replacement prefix if the replacement prefix doesn't already end in /
 			if strings.HasSuffix(path, "/") && !strings.HasSuffix(filterPrefix, "/") {
-				replacement = fmt.Sprintf("%s/$1", filterPrefix)
+				replacement = fmt.Sprintf("%s/$1?$args?", filterPrefix)
 			}
 
 			rewrites.MainRewrite = fmt.Sprintf("%s %s break", regex, replacement)

diff --git a/internal/mode/static/nginx/config/servers_test.go b/internal/mode/static/nginx/config/servers_test.go
@@ -1278,7 +1278,7 @@ func TestCreateServers(t *testing.T) {
 			},
 			{
 				Path:            "/_ngf-internal-rule8-route0",
-				Rewrites:        []string{"^ $request_uri", "^/rewrite-with-headers(.*)$ /prefix-replacement$1 break"},
+				Rewrites:        []string{"^ $request_uri", "^/rewrite-with-headers([^?]*)? /prefix-replacement$1?$args? break"},
 				ProxyPass:       "http://test_foo_80",
 				ProxySetHeaders: rewriteProxySetHeaders,
 				Type:            http.InternalLocationType,
@@ -2441,7 +2441,7 @@ func TestCreateRewritesValForRewriteFilter(t *testing.T) {
 			},
 			expected: &rewriteConfig{
 				InternalRewrite: "^ $request_uri",
-				MainRewrite:     "^/original(.*)$ /prefix-path$1 break",
+				MainRewrite:     "^/original([^?]*)? /prefix-path$1?$args? break",
 			},
 			msg: "prefix path no trailing slashes",
 		},
@@ -2455,7 +2455,7 @@ func TestCreateRewritesValForRewriteFilter(t *testing.T) {
 			},
 			expected: &rewriteConfig{
 				InternalRewrite: "^ $request_uri",
-				MainRewrite:     "^/original(?:/(.*))?$ /$1 break",
+				MainRewrite:     "^/original(?:/([^?]*))? /$1?$args? break",
 			},
 			msg: "prefix path empty string",
 		},
@@ -2469,7 +2469,7 @@ func TestCreateRewritesValForRewriteFilter(t *testing.T) {
 			},
 			expected: &rewriteConfig{
 				InternalRewrite: "^ $request_uri",
-				MainRewrite:     "^/original(?:/(.*))?$ /$1 break",
+				MainRewrite:     "^/original(?:/([^?]*))? /$1?$args? break",
 			},
 			msg: "prefix path /",
 		},
@@ -2483,7 +2483,7 @@ func TestCreateRewritesValForRewriteFilter(t *testing.T) {
 			},
 			expected: &rewriteConfig{
 				InternalRewrite: "^ $request_uri",
-				MainRewrite:     "^/original(?:/(.*))?$ /trailing/$1 break",
+				MainRewrite:     "^/original(?:/([^?]*))? /trailing/$1?$args? break",
 			},
 			msg: "prefix path replacement with trailing /",
 		},
@@ -2497,7 +2497,7 @@ func TestCreateRewritesValForRewriteFilter(t *testing.T) {
 			},
 			expected: &rewriteConfig{
 				InternalRewrite: "^ $request_uri",
-				MainRewrite:     "^/original/(.*)$ /prefix-path/$1 break",
+				MainRewrite:     "^/original/([^?]*)? /prefix-path/$1?$args? break",
 			},
 			msg: "prefix path original with trailing /",
 		},
@@ -2511,7 +2511,7 @@ func TestCreateRewritesValForRewriteFilter(t *testing.T) {
 			},
 			expected: &rewriteConfig{
 				InternalRewrite: "^ $request_uri",
-				MainRewrite:     "^/original/(.*)$ /trailing/$1 break",
+				MainRewrite:     "^/original/([^?]*)? /trailing/$1?$args? break",
 			},
 			msg: "prefix path both with trailing slashes",
 		},

diff --git a/site/content/how-to/traffic-management/redirects-and-rewrites.md b/site/content/how-to/traffic-management/redirects-and-rewrites.md
@@ -188,6 +188,17 @@ Server name: coffee-6b8b6d6486-7fc78
 URI: /beans
 ```
 
+```shell
+curl --resolve cafe.example.com:$GW_PORT:$GW_IP http://cafe.example.com:$GW_PORT/coffee/mocha\?test\=v1\&test\=v2
+```
+
+```text
+Server address: 10.244.0.235:8080
+Server name: coffee-6db967495b-twn6x
+...
+URI: /beans?test=v1&test=v2
+```
+
 ```shell
 curl --resolve cafe.example.com:$GW_PORT:$GW_IP http://cafe.example.com:$GW_PORT/latte/prices
 ```
@@ -199,6 +210,18 @@ Server name: coffee-6b8b6d6486-7fc78
 URI: /prices
 ```
 
+```shell
+curl --resolve cafe.example.com:$GW_PORT:$GW_IP http://cafe.example.com:$GW_PORT/latte/prices\?test\=v1\&test\=v2
+```
+
+```text
+curl --resolve cafe.example.com:$GW_PORT:$GW_IP http://cafe.example.com:$GW_PORT/latte/prices\?test\=v1\&test\=v2
+Server address: 10.244.0.235:8080
+Server name: coffee-6db967495b-twn6x
+...
+URI: /prices?test=v1&test=v2
+```
+
 ## Further reading
 
 To learn more about redirects and rewrites using the Gateway API, see the following resource: