You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2022/11/18 06:51:37 UTC

[GitHub] [apisix] spacewander commented on a diff in pull request #8336: feat: proxy-rewrite support config add set and remove header

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


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

Review Comment:
   We need to use https://github.com/openresty/lua-resty-core/blob/master/lib/ngx/req.md#add_header. Merging multiple request headers with `,` doesn't obey the standard.



##########
apisix/plugins/proxy-rewrite.lua:
##########
@@ -70,8 +74,58 @@ local schema = {
         },
         headers = {
             description = "new headers for request",
-            type = "object",
-            minProperties = 1,
+            anyOf = {
+                {
+                    type = "object",
+                    minProperties = 1,
+                    patternProperties = {
+                        ["^[^:]+$"] = {
+                            oneOf = {
+                                { type = "string" },
+                                { type = "number" }
+                            }
+                        }
+                    },
+                },
+                {
+                    properties = {
+                        add = {
+                            type = "object",
+                            minProperties = 1,
+                            patternProperties = {
+                                ["^[^:]+$"] = {
+                                    oneOf = {
+                                        { type = "string" },
+                                        { type = "number" }
+                                    }
+                                }
+                            },
+                        },
+                        set = {
+                            type = "object",
+                            minProperties = 1,
+                            patternProperties = {
+                                ["^[^:]+$"] = {
+                                    oneOf = {
+                                        { type = "string" },
+                                        { type = "number" },
+                                    }
+                                }
+                            },
+                        },
+                        remove = {
+                            type = "array",
+                            minItems = 1,
+                            items = {
+                                type = "string",
+                                -- "Set-Cookie"

Review Comment:
   Set-Cookie is a response header. Better to use another example.



-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org