tokers commented on code in PR #8336:
URL: https://github.com/apache/apisix/pull/8336#discussion_r1024668471


##########
t/plugin/proxy-rewrite3.t:
##########
@@ -337,3 +337,94 @@ X-Forwarded-Host: apisix.ai
 X-Forwarded-Host: test.com
 --- no_error_log
 [error]
+
+
+
+=== TEST 14: set route add test
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/routes/1',
+                 ngx.HTTP_PUT,
+                 [[{
+                        "methods": ["GET"],
+                        "plugins": {
+                            "proxy-rewrite": {
+                                "headers": {
+                                    "add":{"test": "123"},
+                                    "set":{"test2": "2233"},
+                                    "remove":["hello"]
+                                }
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1980": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/echo"
+                }]]
+                )
+
+            if code >= 300 then
+                ngx.status = code
+            end
+            ngx.say(body)
+        }
+    }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 15: add exist header in muti-header
+--- request
+GET /echo HTTP/1.1
+--- more_headers
+test: sssss
+test: bbb
+--- response_headers
+test: sssss, bbb, 123
+--- no_error_log
+[error]
+
+
+
+=== TEST 16: add header to exist header
+--- request
+GET /echo HTTP/1.1
+--- more_headers
+test: sssss
+--- response_headers
+test: sssss, 123
+--- no_error_log
+[error]
+
+
+
+=== TEST 17: remove header
+--- request
+GET /echo HTTP/1.1
+--- more_headers
+hello: word
+--- response_headers
+hello:
+--- no_error_log
+[error]
+
+
+
+=
+== TEST 18: remove header success
+--- request
+GET /echo HTTP/1.1
+--- response_headers
+test2: 2233
+--- no_error_log
+[error]

Review Comment:
   Missing cases:
   
   1. The set is not used
   2. Add a case which add, set the same header, for testing the priority.



##########
docs/en/latest/plugins/proxy-rewrite.md:
##########
@@ -39,7 +39,10 @@ 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     |        |                 | Rewriting 
the headers. The format is  `{"name": "value", ...}`. The values in the header 
can contain Nginx variables like $remote_addr and $balancer_ip.                 
                                                                               |

Review Comment:
   I think overwrite will be better than rewriting.



##########
docs/en/latest/plugins/proxy-rewrite.md:
##########
@@ -39,7 +39,10 @@ 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     |        |                 | Rewriting 
the headers. 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", ...]`.

Review Comment:
   Should also tell users the priority of set, remove, and add.



-- 
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]

Reply via email to