spacewander commented on code in PR #8336: URL: https://github.com/apache/apisix/pull/8336#discussion_r1027758310
########## t/plugin/proxy-rewrite3.t: ########## @@ -328,12 +379,95 @@ passed -=== TEST 13: rewrite X-Forwarded-Host +=== TEST 15: add exist header in muti-header --- request GET /echo HTTP/1.1 --- more_headers -X-Forwarded-Host: apisix.ai +test: sssss +test: bbb --- response_headers -X-Forwarded-Host: test.com +test: sssss, bbb, 123 +--- no_error_log Review Comment: Need to deal with https://github.com/apache/apisix/pull/8336#discussion_r1026081925 in other test ########## apisix/core/request.lua: ########## @@ -138,6 +145,15 @@ function _M.set_header(ctx, header_name, header_value) end end +function _M.add_header(header_name, header_value) + local err + header_name, err = _validate_header_name(header_name) + if err then + error(err) + end + + req_add_header(header_name,header_value) Review Comment: Please ensure a space is after the ','. Let's deal with similar places in this PR. ########## apisix/core/request.lua: ########## @@ -22,6 +22,12 @@ local lfs = require("lfs") local log = require("apisix.core.log") local io = require("apisix.core.io") +local ngx_req Review Comment: We don't need a ngx_req as a module variable ########## docs/en/latest/plugins/proxy-rewrite.md: ########## @@ -39,9 +39,18 @@ The `proxy-rewrite` Plugin rewrites Upstream proxy information such as `scheme`, | method | string | False | | ["GET", "POST", "PUT", "HEAD", "DELETE", "OPTIONS","MKCOL", "COPY", "MOVE", "PROPFIND", "PROPFIND","LOCK", "UNLOCK", "PATCH", "TRACE"] | Rewrites the HTTP method. | | regex_uri | array[string] | False | | | New upstream forwarding address. Regular expressions can be used to match the URL from client. If it matches, the URL template is forwarded to the Upstream otherwise, the URL from the client is forwarded. When both `uri` and `regex_uri` are configured, `uri` is used first. For example, `[" ^/iresty/(.*)/(.*)/(.*)", "/$1-$2-$3"]`. Here, the first element is the regular expression to match and the second element is the URL template forwarded to the Upstream. | | host | string | False | | | New Upstream host address. | -| headers | object | False | | | New Upstream headers. Headers are overwritten if they are already present otherwise, they are added to the present headers. To remove a header, set the header value to an empty string. The values in the header can contain Nginx variables like `$remote_addr` and `$client_addr`. | +| headers | object | False | | | | +| headers.add | object | false | | | Append the new headers. The format is `{"name: value",...}`. The values in the header can contain Nginx variables like $remote_addr and $balancer_ip. | +| headers.set | object | false | | | Overwrite the headers. If header is not exist, will add it. The format is `{"name": "value", ...}`. The values in the header can contain Nginx variables like $remote_addr and $balancer_ip. | +| headers.remove | array | false | | | Remove the headers. The format is `["name", ...]`. | use_real_request_uri_unsafe | boolean | False | false | | Use real_request_uri (original $request_uri in nginx) to bypass URI normalization. **Enabling this is considered unsafe as it bypasses all URI normalization steps**. | +## Header Priority + +Header Header configurations are executed according to the following priorities: Review Comment: ```suggestion Header configurations are executed according to the following priorities: ``` ########## t/plugin/proxy-rewrite3.t: ########## @@ -313,6 +313,57 @@ ngx.var.request_uri: /print_uri_detailed }]] ) + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- response_body +passed + + + +=== TEST 13: rewrite X-Forwarded-Host Review Comment: This test isn't relative to this feature? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
