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


##########
apisix/plugins/proxy-rewrite.lua:
##########
@@ -70,8 +74,58 @@ local schema = {
         },
         headers = {
             description = "new headers for request",
-            type = "object",
-            minProperties = 1,
+            anyOf = {

Review Comment:
   `anyOf` means you can configure both the old way and the new way at the same 
time, which is unnecessary. While `oneOf` means you can configure either the 
old way or the new way.



##########
t/plugin/proxy-rewrite3.t:
##########
@@ -337,3 +337,145 @@ X-Forwarded-Host: apisix.ai
 X-Forwarded-Host: test.com
 --- no_error_log
 [error]
+
+
+
+=== TEST 14: set route header 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: set header success
+--- request
+GET /echo HTTP/1.1
+--- response_headers
+test2: 2233
+--- no_error_log
+[error]
+
+
+
+=== TEST 19: header priority 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": "test_in_add"},
+                                    "set":{"test": "test_in_set"}
+                                }
+                            }
+                        },
+                        "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 20: set and test priority test
+--- request
+GET /echo HTTP/1.1
+--- response_headers
+test: test_in_set

Review Comment:
   As per the doc you wrote, the `set` operation takes the precedence than 
`add`, in such a case, the `test` header should have two values.



##########
apisix/plugins/proxy-rewrite.lua:
##########
@@ -163,6 +163,8 @@ local function check_set_headers(headers)
         if #field == 0 then
             return false, 'invalid field length in header'
         end
+
+        core.log.info("header field: ", field)

Review Comment:
   OK, ignore it.



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