feat: proxy-rewrite support config add set and remove header#8336
Conversation
| | 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. | |
There was a problem hiding this comment.
I think use headers.rewrite is more clear.
What happens if headers.set is set, but the actual request does not have a header name that needs to be rewritten?
There was a problem hiding this comment.
This headers.set is the same as the response-rewrite plugin. When the header does not exist will add it.
| } | ||
| end | ||
|
|
||
| local function addHeader(ctx, header_name, header_value) |
There was a problem hiding this comment.
| local function addHeader(ctx, header_name, header_value) | |
| local function add_header(ctx, header_name, header_value) |
| end | ||
|
|
||
| local function addHeader(ctx, header_name, header_value) | ||
| local oldHeader = core.request.header(ctx, header_name) |
There was a problem hiding this comment.
| local oldHeader = core.request.header(ctx, header_name) | |
| local old_header = core.request.header(ctx, header_name) |
| test: sssss | ||
| test: bbb | ||
| --- response_headers | ||
| test: sssss, bbb, 123 |
There was a problem hiding this comment.
why it is
test: sssss
test: bbb
test: 123
There was a problem hiding this comment.
This is for test header combination. First combine two existing headers test: sssss and test: bbb. And then add a new header test: 123 from proxy_rewrite. Finally, it will combine all headers in one.
There was a problem hiding this comment.
Is it necessary to merge header with the same name? I'm not sure. cc @spacewander
There was a problem hiding this 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.
| minItems = 1, | ||
| items = { | ||
| type = "string", | ||
| -- "Set-Cookie" |
There was a problem hiding this comment.
Set-Cookie is a response header. Better to use another example.
| test: sssss | ||
| test: bbb | ||
| --- response_headers | ||
| test: sssss, bbb, 123 |
There was a problem hiding this 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.
| } | ||
| } | ||
| --- request | ||
| GET /t |
There was a problem hiding this comment.
Please remove duplicate sections which are set in
apisix/t/plugin/proxy-rewrite3.t
Line 28 in ae400b9
| GET /t | ||
| --- response_body | ||
| passed | ||
| --- no_error_log |
| --- response_headers | ||
| X-Forwarded-Host: test.com | ||
| test: sssss, bbb, 123 | ||
| --- no_error_log |
|
|
||
|
|
||
|
|
||
| === TEST 13: rewrite X-Forwarded-Host |
There was a problem hiding this comment.
This test isn't relative to this feature?
There was a problem hiding this comment.
Nope, but this change is to remove duplicate sections from your advice. So I changed some test code.
| error(err) | ||
| end | ||
|
|
||
| req_add_header(header_name,header_value) |
There was a problem hiding this comment.
Please ensure a space is after the ','. Let's deal with similar places in this PR.
Co-authored-by: 罗泽轩 <[email protected]>
| local req_set_uri_args = ngx.req.set_uri_args | ||
|
|
||
|
|
||
|
|
4 participants
Description
Fixes #8239
Checklist