From 0bacf513bc9c6ea0766d6813bbdcaa3decd88392 Mon Sep 17 00:00:00 2001
From: Hisham Muhammad <hisham@gobolinux.org>
Date: Wed, 20 Nov 2024 17:45:07 -0300
Subject: [PATCH] feat(proxy-wasm) set querystring when setting ":path"

---
 src/common/proxy_wasm/ngx_proxy_wasm_maps.c   |  14 +-
 .../111-proxy_set_http_request_header.t       | 133 ++++++++++++++++--
 2 files changed, 129 insertions(+), 18 deletions(-)

diff --git a/src/common/proxy_wasm/ngx_proxy_wasm_maps.c b/src/common/proxy_wasm/ngx_proxy_wasm_maps.c
index 20c175bea..45502dd83 100644
--- a/src/common/proxy_wasm/ngx_proxy_wasm_maps.c
+++ b/src/common/proxy_wasm/ngx_proxy_wasm_maps.c
@@ -511,15 +511,17 @@ ngx_proxy_wasm_maps_set_path(ngx_wavm_instance_t *instance, ngx_str_t *value,
     ngx_proxy_wasm_ctx_t     *pwctx = pwexec->parent;
     ngx_http_wasm_req_ctx_t  *rctx = ngx_http_proxy_wasm_get_rctx(instance);
     ngx_http_request_t       *r = rctx->r;
+    u_char                   *query;
+    size_t                    len = value->len;
 
-    if (ngx_strchr(value->data, '?')) {
-        ngx_wavm_instance_trap_printf(instance,
-                                      "NYI - cannot set request path "
-                                      "with querystring");
-        return NGX_ERROR;
+    query = (u_char *) ngx_strchr(value->data, '?');
+    if (query) {
+        len = query - value->data;
+        r->args.len = value->len - len - 1;
+        r->args.data = query + 1;
     }
 
-    r->uri.len = value->len;
+    r->uri.len = len;
     r->uri.data = value->data;
 
     if (pwctx->path.len) {
diff --git a/t/03-proxy_wasm/hfuncs/111-proxy_set_http_request_header.t b/t/03-proxy_wasm/hfuncs/111-proxy_set_http_request_header.t
index a9cc579f6..24c2c4957 100644
--- a/t/03-proxy_wasm/hfuncs/111-proxy_set_http_request_header.t
+++ b/t/03-proxy_wasm/hfuncs/111-proxy_set_http_request_header.t
@@ -295,26 +295,135 @@ path: /test
 
 
 
-=== TEST 12: proxy_wasm - set_http_request_headers() cannot set ':path' with querystring (NYI)
+=== TEST 12: proxy_wasm - set_http_request_headers() can set ':path' with querystring
 --- wasm_modules: hostcalls
+--- http_config eval
+qq{
+    upstream test_upstream {
+        server unix:$ENV{TEST_NGINX_UNIX_SOCKET};
+    }
+
+    server {
+        listen unix:$ENV{TEST_NGINX_UNIX_SOCKET};
+
+        location / {
+            return 200 '\$request_uri \$uri \$is_args \$args\n';
+        }
+    }
+}
 --- config
     location /t {
         proxy_wasm hostcalls 'test=/t/set_request_header name=:path value=/test?foo=bar';
-        return 200;
+        proxy_wasm hostcalls 'test=/t/log/request_path';
+        proxy_pass http://test_upstream$uri$is_args$args;
     }
---- error_code: 500
---- response_body_like: 500 Internal Server Error
---- grep_error_log eval: qr/(NYI|\[.*?failed resuming).*/
---- grep_error_log_out eval
-qr/.*?NYI - cannot set request path with querystring.*
-\[info\] .*? filter chain failed resuming: previous error \(instance trapped\)/
+--- response_body
+/test?foo=bar /test ? foo=bar
+--- error_log
+path: /test?foo=bar
 --- no_error_log
-[alert]
-[stderr]
+[error]
+[crit]
+
+
+
+=== TEST 13: proxy_wasm - set_http_request_headers() setting ':path' with empty querystring drops the querystring
+--- wasm_modules: hostcalls
+--- http_config eval
+qq{
+    upstream test_upstream {
+        server unix:$ENV{TEST_NGINX_UNIX_SOCKET};
+    }
+
+    server {
+        listen unix:$ENV{TEST_NGINX_UNIX_SOCKET};
+
+        location / {
+            return 200 '(\$request_uri) (\$uri) (\$is_args) (\$args)\n';
+        }
+    }
+}
+--- config
+    location /t {
+        proxy_wasm hostcalls 'test=/t/set_request_header name=:path value=/test?';
+        proxy_wasm hostcalls 'test=/t/log/request_path';
+        proxy_pass http://test_upstream$uri$is_args$args;
+    }
+--- response_body
+(/test) (/test) () ()
+--- error_log
+path: /test
+--- no_error_log
+[error]
+[crit]
+
+
+
+=== TEST 14: proxy_wasm - set_http_request_headers() can set ':path' with empty path
+--- wasm_modules: hostcalls
+--- http_config eval
+qq{
+    upstream test_upstream {
+        server unix:$ENV{TEST_NGINX_UNIX_SOCKET};
+    }
+
+    server {
+        listen unix:$ENV{TEST_NGINX_UNIX_SOCKET};
+
+        location / {
+            return 200 '(\$request_uri) (\$uri) (\$is_args) (\$args)\n';
+        }
+    }
+}
+--- config
+    location /t {
+        proxy_wasm hostcalls 'test=/t/set_request_header name=:path value=';
+        proxy_wasm hostcalls 'test=/t/log/request_path';
+        proxy_pass http://test_upstream$uri$is_args$args;
+    }
+--- response_body
+(/t) (/t) () ()
+--- error_log
+path:
+--- no_error_log
+[error]
+[crit]
+
+
+
+=== TEST 15: proxy_wasm - set_http_request_headers() setting ':path' with single '?' has same behavior as empty path
+--- wasm_modules: hostcalls
+--- http_config eval
+qq{
+    upstream test_upstream {
+        server unix:$ENV{TEST_NGINX_UNIX_SOCKET};
+    }
+
+    server {
+        listen unix:$ENV{TEST_NGINX_UNIX_SOCKET};
+
+        location / {
+            return 200 '(\$request_uri) (\$uri) (\$is_args) (\$args)\n';
+        }
+    }
+}
+--- config
+    location /t {
+        proxy_wasm hostcalls 'test=/t/set_request_header name=:path value=?';
+        proxy_wasm hostcalls 'test=/t/log/request_path';
+        proxy_pass http://test_upstream$uri$is_args$args;
+    }
+--- response_body
+(/t) (/t) () ()
+--- error_log
+path:
+--- no_error_log
+[error]
+[crit]
 
 
 
-=== TEST 13: proxy_wasm - set_http_request_header() sets ':method'
+=== TEST 16: proxy_wasm - set_http_request_header() sets ':method'
 --- wasm_modules: hostcalls
 --- http_config eval
 qq{
@@ -344,7 +453,7 @@ POST
 
 
 
-=== TEST 14: proxy_wasm - set_http_request_header() cannot set ':scheme'
+=== TEST 17: proxy_wasm - set_http_request_header() cannot set ':scheme'
 --- wasm_modules: hostcalls
 --- http_config eval
 qq{