diff --git a/internal/configs/version1/nginx-plus.tmpl b/internal/configs/version1/nginx-plus.tmpl index 917fe4415d..77f979c0e2 100644 --- a/internal/configs/version1/nginx-plus.tmpl +++ b/internal/configs/version1/nginx-plus.tmpl @@ -89,6 +89,10 @@ http { variables_hash_bucket_size {{.VariablesHashBucketSize}}; variables_hash_max_size {{.VariablesHashMaxSize}}; + map $request_uri $request_uri_no_args { + "~^(?P[^?]*)(\?.*)?$" $path; + } + map $http_upgrade $connection_upgrade { default upgrade; '' close; diff --git a/internal/configs/version1/nginx.tmpl b/internal/configs/version1/nginx.tmpl index 42de34b9d3..6360c58de6 100644 --- a/internal/configs/version1/nginx.tmpl +++ b/internal/configs/version1/nginx.tmpl @@ -74,6 +74,10 @@ http { variables_hash_bucket_size {{.VariablesHashBucketSize}}; variables_hash_max_size {{.VariablesHashMaxSize}}; + map $request_uri $request_uri_no_args { + "~^(?P[^?]*)(\?.*)?$" $path; + } + map $http_upgrade $connection_upgrade { default upgrade; '' close; diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go index 19847dd7b0..755d9edc0b 100644 --- a/internal/configs/virtualserver.go +++ b/internal/configs/virtualserver.go @@ -1395,8 +1395,11 @@ func generateRewrites(path string, proxy *conf_v1.ActionProxy, internal bool, or var rewrites []string if internal { - // For internal locations (splits locations) only, recover the original request_uri. - rewrites = append(rewrites, "^ $request_uri") + // For internal locations only, recover the original request_uri without (!) the arguments. + // This is necessary, because if we just use $request_uri (which includes the arguments), + // the rewrite that follows will result in an URI with duplicated arguments: + // for example, /test%3Fhello=world?hello=world instead of /test?hello=world + rewrites = append(rewrites, "^ $request_uri_no_args") } if isRegex { diff --git a/internal/configs/virtualserver_test.go b/internal/configs/virtualserver_test.go index 7e72451b9c..cf237f9165 100644 --- a/internal/configs/virtualserver_test.go +++ b/internal/configs/virtualserver_test.go @@ -5040,7 +5040,7 @@ func TestGenerateSplits(t *testing.T) { Path: "/internal_location_splits_1_split_0", ProxyPass: "http://vs_default_cafe_coffee-v1", Rewrites: []string{ - "^ $request_uri", + "^ $request_uri_no_args", fmt.Sprintf(`"^%v(.*)$" "/rewrite$1" break`, originalPath), }, ProxyNextUpstream: "error timeout", @@ -7282,16 +7282,19 @@ func TestGenerateRewrites(t *testing.T) { originalPath string grpcEnabled bool expected []string + msg string }{ { proxy: nil, expected: nil, + msg: "action isn't proxy", }, { proxy: &conf_v1.ActionProxy{ RewritePath: "", }, expected: nil, + msg: "no rewrite is configured", }, { path: "/path", @@ -7299,6 +7302,7 @@ func TestGenerateRewrites(t *testing.T) { RewritePath: "/rewrite", }, expected: nil, + msg: "non-regex rewrite for non-internal location is not needed", }, { path: "/_internal_path", @@ -7307,7 +7311,8 @@ func TestGenerateRewrites(t *testing.T) { RewritePath: "/rewrite", }, originalPath: "/path", - expected: []string{`^ $request_uri`, `"^/path(.*)$" "/rewrite$1" break`}, + expected: []string{`^ $request_uri_no_args`, `"^/path(.*)$" "/rewrite$1" break`}, + msg: "non-regex rewrite for internal location", }, { path: "~/regex", @@ -7316,7 +7321,8 @@ func TestGenerateRewrites(t *testing.T) { RewritePath: "/rewrite", }, originalPath: "/path", - expected: []string{`^ $request_uri`, `"^/path(.*)$" "/rewrite$1" break`}, + expected: []string{`^ $request_uri_no_args`, `"^/path(.*)$" "/rewrite$1" break`}, + msg: "regex rewrite for internal location", }, { path: "~/regex", @@ -7325,6 +7331,7 @@ func TestGenerateRewrites(t *testing.T) { RewritePath: "/rewrite", }, expected: []string{`"^/regex" "/rewrite" break`}, + msg: "regex rewrite for non-internal location", }, { path: "/_internal_path", @@ -7334,7 +7341,8 @@ func TestGenerateRewrites(t *testing.T) { }, originalPath: "/path", grpcEnabled: true, - expected: []string{`^ $request_uri`, `"^/path(.*)$" "/rewrite$1" break`}, + expected: []string{`^ $request_uri_no_args`, `"^/path(.*)$" "/rewrite$1" break`}, + msg: "non-regex rewrite for internal location with grpc enabled", }, { path: "/_internal_path", @@ -7342,14 +7350,14 @@ func TestGenerateRewrites(t *testing.T) { originalPath: "/path", grpcEnabled: true, expected: []string{`^ $request_uri break`}, + msg: "empty rewrite for internal location with grpc enabled", }, } for _, test := range tests { result := generateRewrites(test.path, test.proxy, test.internal, test.originalPath, test.grpcEnabled) - if !reflect.DeepEqual(result, test.expected) { - t.Errorf("generateRewrites(%v, %v, %v, %v) returned \n %v but expected \n %v", - test.path, test.proxy, test.internal, test.originalPath, result, test.expected) + if diff := cmp.Diff(test.expected, result); diff != "" { + t.Errorf("generateRewrites() '%v' mismatch (-want +got):\n%s", test.msg, diff) } } }