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/02/23 07:10:47 UTC
[GitHub] [apisix] rampagecong opened a new pull request #6426: feat: add `rewrite:RespHeaders` and modify the upstream response headers via `request` implementation
rampagecong opened a new pull request #6426:
URL: https://github.com/apache/apisix/pull/6426
### What this PR does / why we need it:
This PR add the ability to call upstream server when change the response header.
When we implement SSO, we can set and modify cookie in APISIX plugin layer and also call upstream at the same time.
This is refer to https://github.com/apache/apisix-go-plugin-runner/issues/67
ext-plugin-proto : https://github.com/api7/ext-plugin-proto/pull/26
apisix-go-plugin-runner : https://github.com/apache/apisix-go-plugin-runner/pull/68
### Pre-submission checklist:
* [x] Did you explain what problem does this PR solve? Or what new features have been added?
* [x] Have you added corresponding test cases?
* [x] Have you modified the corresponding document?
* [x] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix/tree/master#community) first**
--
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
[GitHub] [apisix] spacewander merged pull request #6426: feat: add `rewrite:RespHeaders` and modify the upstream response headers via `request` implementation
Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #6426:
URL: https://github.com/apache/apisix/pull/6426
--
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
[GitHub] [apisix] rampagecong commented on pull request #6426: feat: add `rewrite:RespHeaders` and modify the upstream response headers via `request` implementation
Posted by GitBox <gi...@apache.org>.
rampagecong commented on pull request #6426:
URL: https://github.com/apache/apisix/pull/6426#issuecomment-1054076518
The test cases can't be run in both my two centos machines(follow the step in docs). So I have to try follow the exsisting cases and push code to ci to verify it.
`[root@localdev apisix]# make test
make test
[ info ] test -> [ Start ]
git submodule update --init --recursive
prove -I../test-nginx/lib -I./ -r -s t/
t/node/remote-addr.t ............................ nginx: [error] init_by_lua error: /data/service/test/apisix/deps/share/lua/5.1/protoc.lua:1148: attempt to call field 'load' (a nil value)
stack traceback:
/data/service/test/apisix/deps/share/lua/5.1/protoc.lua:1148: in function </data/service/test/apisix/deps/share/lua/5.1/protoc.lua:1147>
/data/service/test/apisix/deps/share/lua/5.1/protoc.lua:1192: in main chunk
[C]: in function 'require'
...vice/test/apisix/apisix/plugins/grpc-transcode/proto.lua:20: in main chunk
[C]: in function 'require'
/data/service/test/apisix/apisix/admin/proto.lua:23: in main chunk
[C]: in function 'require'
/data/service/test/apisix/apisix/admin/init.lua:51: in main chunk
[C]: in function 'require'
/data/service/test/apisix/apisix/init.lua:34: in main chunk
[C]: in function 'require'
init_by_lua:9: in main chunk
`
--
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
[GitHub] [apisix] tzssangglass commented on a change in pull request #6426: feat: add `rewrite:RespHeaders` and modify the upstream response headers via `request` implementation
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on a change in pull request #6426:
URL: https://github.com/apache/apisix/pull/6426#discussion_r815806843
##########
File path: apisix/plugins/ext-plugin/init.lua
##########
@@ -611,6 +611,29 @@ local rpc_handlers = {
end
end
+ local len = rewrite:RespHeadersLength()
+ local excludeRespHeader = {
Review comment:
```suggestion
local exclude_resp_header = {
```
is better
--
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
[GitHub] [apisix] shuaijinchao commented on a change in pull request #6426: feat: add `rewrite:RespHeaders` and modify the upstream response headers via `request` implementation
Posted by GitBox <gi...@apache.org>.
shuaijinchao commented on a change in pull request #6426:
URL: https://github.com/apache/apisix/pull/6426#discussion_r817479583
##########
File path: apisix/plugins/ext-plugin/init.lua
##########
@@ -611,6 +611,29 @@ local rpc_handlers = {
end
end
+ local len = rewrite:RespHeadersLength()
+ local exclude_resp_header = {
Review comment:
You can move to the top of the current file.
--
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
[GitHub] [apisix] shuaijinchao commented on a change in pull request #6426: feat: add `rewrite:RespHeaders` and modify the upstream response headers via `request` implementation
Posted by GitBox <gi...@apache.org>.
shuaijinchao commented on a change in pull request #6426:
URL: https://github.com/apache/apisix/pull/6426#discussion_r815807966
##########
File path: t/lib/ext-plugin.lua
##########
@@ -67,7 +67,7 @@ function _M.go(case)
ty = constants.RPC_ERROR
err_resp.Start(builder)
err_resp.AddCode(builder, err_code.BAD_REQUEST)
- local req = prepare_conf_req.End(builder)
+ local req = prepare_conf_req.End(builder)
Review comment:
```suggestion
local req = prepare_conf_req.End(builder)
```
--
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
[GitHub] [apisix] rampagecong commented on a change in pull request #6426: feat: add `rewrite:RespHeaders` and modify the upstream response headers via `request` implementation
Posted by GitBox <gi...@apache.org>.
rampagecong commented on a change in pull request #6426:
URL: https://github.com/apache/apisix/pull/6426#discussion_r817414846
##########
File path: apisix/plugins/ext-plugin/init.lua
##########
@@ -611,6 +611,29 @@ local rpc_handlers = {
end
end
+ local len = rewrite:RespHeadersLength()
+ local exclude_resp_header = {
Review comment:
I understand to put it in the 'apisix/constants. lua' file, right?
--
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
[GitHub] [apisix] spacewander commented on a change in pull request #6426: feat: add `rewrite:RespHeaders` and modify the upstream response headers via `request` implementation
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #6426:
URL: https://github.com/apache/apisix/pull/6426#discussion_r817270826
##########
File path: apisix/plugins/ext-plugin/init.lua
##########
@@ -611,6 +611,29 @@ local rpc_handlers = {
end
end
+ local len = rewrite:RespHeadersLength()
+ local exclude_resp_header = {
Review comment:
Let's move the table in the module level
--
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
[GitHub] [apisix] lijing-21 commented on pull request #6426: feat: add `rewrite:RespHeaders` and modify the upstream response headers via `request` implementation
Posted by GitBox <gi...@apache.org>.
lijing-21 commented on pull request #6426:
URL: https://github.com/apache/apisix/pull/6426#issuecomment-1068992467
Hi @rampagecong , thank you for your contribution!
Here is the Contributor T-shirt form[1], if you're interested, kindly take a look :)
[1] https://github.com/apache/apisix/blob/master/CONTRIBUTING.md#contributor-t-shirt
--
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
[GitHub] [apisix] shuaijinchao edited a comment on pull request #6426: feat: add `rewrite:RespHeaders` and modify the upstream response headers via `request` implementation
Posted by GitBox <gi...@apache.org>.
shuaijinchao edited a comment on pull request #6426:
URL: https://github.com/apache/apisix/pull/6426#issuecomment-1055132608
@rampagecong OK, I need some time to check this question and get back to you later.
--
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
[GitHub] [apisix] tokers commented on a change in pull request #6426: feat: add `rewrite:RespHeaders` and modify the upstream response headers via `request` implementation
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #6426:
URL: https://github.com/apache/apisix/pull/6426#discussion_r818237305
##########
File path: apisix/plugins/ext-plugin/init.lua
##########
@@ -65,6 +65,18 @@ local type = type
local events_list
+local exclude_resp_header = {
+ ["connection"] = true,
+ ["content-length"] = true,
+ ["transfer-encoding"] = true,
+ ["location"] = true,
+ ["server"] = true,
+ ["www-authenticate"] = true,
+ ["content-encoding"] = true,
+ ["content-type"] = true,
+ ["content-location"] = true,
+ ["content-language"] = true,
Review comment:
Maybe some users want to use the external plugin to modify the Cookie, but it's also unsafe.
--
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
[GitHub] [apisix] tokers commented on a change in pull request #6426: feat: add `rewrite:RespHeaders` and modify the upstream response headers via `request` implementation
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #6426:
URL: https://github.com/apache/apisix/pull/6426#discussion_r818237030
##########
File path: apisix/plugins/ext-plugin/init.lua
##########
@@ -65,6 +65,18 @@ local type = type
local events_list
+local exclude_resp_header = {
+ ["connection"] = true,
+ ["content-length"] = true,
+ ["transfer-encoding"] = true,
+ ["location"] = true,
+ ["server"] = true,
+ ["www-authenticate"] = true,
+ ["content-encoding"] = true,
+ ["content-type"] = true,
+ ["content-location"] = true,
+ ["content-language"] = true,
Review comment:
Should we also exclude headers like: "Set-Cookie"?
--
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
[GitHub] [apisix] spacewander commented on pull request #6426: feat: add `rewrite:RespHeaders` and modify the upstream response headers via `request` implementation
Posted by GitBox <gi...@apache.org>.
spacewander commented on pull request #6426:
URL: https://github.com/apache/apisix/pull/6426#issuecomment-1055056532
> `[root@iZbp1efn5vn52xspp2bid5Z apisix]# make test [ info ] test -> [ Start ] git submodule update --init --recursive prove -I../test-nginx/lib -I./ -r -s t/ t/stream-node/sni.t ............................. nginx: [error] init_by_lua error: error loading module 'ssl.core' from file '/opt/service/test/apisix/deps/lib/lua/5.1/ssl.so': /opt/service/test/apisix/deps/lib/lua/5.1/ssl.so: undefined symbol: sk_free stack traceback: [C]: at 0x7f6b5dc70a20 [C]: in function 'require' /opt/service/test/apisix/deps/share/lua/5.1/ssl.lua:8: in main chunk [C]: in function 'require' /opt/service/test/apisix/apisix/patch.lua:22: in main chunk [C]: in function 'require' /opt/service/test/apisix/apisix/init.lua:28: in main chunk [C]: in function 'require' init_by_lua:9: in main chunk`
@shuaijinchao
Would you provide help to solve the OpenSSL problem?
--
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
[GitHub] [apisix] rampagecong edited a comment on pull request #6426: feat: add `rewrite:RespHeaders` and modify the upstream response headers via `request` implementation
Posted by GitBox <gi...@apache.org>.
rampagecong edited a comment on pull request #6426:
URL: https://github.com/apache/apisix/pull/6426#issuecomment-1054076518
The test cases can't be run in both my two centos machines(follow the step in docs). So I have to try follow the exsisting cases and push code to ci to verify it.
`[root@localdev apisix]# make test
make test
[ info ] test -> [ Start ]
git submodule update --init --recursive
prove -I../test-nginx/lib -I./ -r -s t/
t/node/remote-addr.t ............................ nginx: [error] init_by_lua error: /data/service/test/apisix/deps/share/lua/5.1/protoc.lua:1148: attempt to call field 'load' (a nil value)
stack traceback:
/data/service/test/apisix/deps/share/lua/5.1/protoc.lua:1148: in function </data/service/test/apisix/deps/share/lua/5.1/protoc.lua:1147>
/data/service/test/apisix/deps/share/lua/5.1/protoc.lua:1192: in main chunk
[C]: in function 'require'
...vice/test/apisix/apisix/plugins/grpc-transcode/proto.lua:20: in main chunk
[C]: in function 'require'
/data/service/test/apisix/apisix/admin/proto.lua:23: in main chunk
[C]: in function 'require'
/data/service/test/apisix/apisix/admin/init.lua:51: in main chunk
[C]: in function 'require'
/data/service/test/apisix/apisix/init.lua:34: in main chunk
[C]: in function 'require'
init_by_lua:9: in main chunk
`
@spacewander
--
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
[GitHub] [apisix] tokers commented on a change in pull request #6426: feat: add `rewrite:RespHeaders` and modify the upstream response headers via `request` implementation
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #6426:
URL: https://github.com/apache/apisix/pull/6426#discussion_r812685835
##########
File path: apisix/plugins/ext-plugin/init.lua
##########
@@ -611,6 +611,14 @@ local rpc_handlers = {
end
end
+ local len = rewrite:RespHeadersLength()
Review comment:
Please reindent the codes.
--
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
[GitHub] [apisix] rampagecong commented on pull request #6426: feat: add `rewrite:RespHeaders` and modify the upstream response headers via `request` implementation
Posted by GitBox <gi...@apache.org>.
rampagecong commented on pull request #6426:
URL: https://github.com/apache/apisix/pull/6426#issuecomment-1055120863
`[root@iZbp1efn5vn52xspp2bid5Z apisix]# prove -I../test-nginx/lib -I./ -r -s t/plugin/ext-plugin/http-req-call.t
t/plugin/ext-plugin/http-req-call.t .. nginx: [warn] stream [lua] health_check.lua:99: report_failure(): update endpoint: http://127.0.0.1:2379 to unhealthy
nginx: [warn] stream [lua] v3.lua:151: _request_uri(): http://127.0.0.1:2379: connection refused. Retrying
nginx: [error] stream [lua] init.lua:822: stream_init(): failed to load the configuration: has no healthy etcd endpoint available
nginx: [warn] [lua] health_check.lua:99: report_failure(): update endpoint: http://127.0.0.1:2379 to unhealthy
nginx: [warn] [lua] v3.lua:151: _request_uri(): http://127.0.0.1:2379: connection refused. Retrying
nginx: [error] [lua] init.lua:86: http_init(): failed to load the configuration: has no healthy etcd endpoint available
t/plugin/ext-plugin/http-req-call.t .. 1/?
# Failed test 'TEST 7: rewrite args - status code ok'
# at /opt/service/test/apisix/../test-nginx/lib/Test/Nginx/Socket.pm line 948.
# got: '404'
# expected: '200'
# Failed test 'TEST 7: rewrite args - response_body - response is expected (repeated req 0, req 0)'
# at /opt/service/test/apisix/../test-nginx/lib/Test/Nginx/Socket.pm line 1589.
# @@ -1,3 +1 @@
# -uri: /plugin_proxy_rewrite_args
# +{"error_msg":"404 Route Not Found"}
# -a: foo,bar
# -c: bar`
'TEST 7' also failed. Is there a problem with setting it?Please help me.
@shuaijinchao
--
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
[GitHub] [apisix] tokers commented on a change in pull request #6426: feat: add `rewrite:RespHeaders` and modify the upstream response headers via `request` implementation
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #6426:
URL: https://github.com/apache/apisix/pull/6426#discussion_r813468326
##########
File path: apisix/plugins/ext-plugin/init.lua
##########
@@ -611,6 +611,14 @@ local rpc_handlers = {
end
end
+ local len = rewrite:RespHeadersLength()
Review comment:
Should add some test cases.
Also, shall we exclude some vital headers such as `Connection`, `Content-Length`, `Transfer-Encoding` and etc.
--
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
[GitHub] [apisix] tzssangglass commented on pull request #6426: feat: add `rewrite:RespHeaders` and modify the upstream response headers via `request` implementation
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on pull request #6426:
URL: https://github.com/apache/apisix/pull/6426#issuecomment-1049426993
need to update
https://github.com/apache/apisix/blob/749094bb55421f5b715af18e376c373603bf5d62/rockspec/apisix-master-0.rockspec#L70
to 0.4.0
ref: https://github.com/api7/ext-plugin-proto/releases/tag/v0.4.0
--
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
[GitHub] [apisix] shuaijinchao commented on pull request #6426: feat: add `rewrite:RespHeaders` and modify the upstream response headers via `request` implementation
Posted by GitBox <gi...@apache.org>.
shuaijinchao commented on pull request #6426:
URL: https://github.com/apache/apisix/pull/6426#issuecomment-1055132608
@rampagecong r Ok, I need some time to check this question and get back to you later.
--
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
[GitHub] [apisix] spacewander commented on a change in pull request #6426: feat: add `rewrite:RespHeaders` and modify the upstream response headers via `request` implementation
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #6426:
URL: https://github.com/apache/apisix/pull/6426#discussion_r814442173
##########
File path: apisix/plugins/ext-plugin/init.lua
##########
@@ -611,6 +611,14 @@ local rpc_handlers = {
end
end
+ local len = rewrite:RespHeadersLength()
Review comment:
> Connection,Content-Length,Transfer-Encoding, Location,Server, WWW-Authenticate, Content-Encoding, Content-Type,Content-Location,Content-Type,Content-Language
LGTM
--
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
[GitHub] [apisix] rampagecong commented on a change in pull request #6426: feat: add `rewrite:RespHeaders` and modify the upstream response headers via `request` implementation
Posted by GitBox <gi...@apache.org>.
rampagecong commented on a change in pull request #6426:
URL: https://github.com/apache/apisix/pull/6426#discussion_r816629285
##########
File path: t/lib/ext-plugin.lua
##########
@@ -367,6 +367,33 @@ function _M.go(case)
local action = http_req_call_rewrite.End(builder)
build_action(action, http_req_call_action.Rewrite)
+ elseif case.rewrite_resp_header == true then
+ local hdrs = {
Review comment:
I have added two test cases.
--
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
[GitHub] [apisix] tokers commented on a change in pull request #6426: feat: add `rewrite:RespHeaders` and modify the upstream response headers via `request` implementation
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #6426:
URL: https://github.com/apache/apisix/pull/6426#discussion_r813707858
##########
File path: apisix/plugins/ext-plugin/init.lua
##########
@@ -611,6 +611,14 @@ local rpc_handlers = {
end
end
+ local len = rewrite:RespHeadersLength()
Review comment:
Let's see others' suggestions about this.
--
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
[GitHub] [apisix] rampagecong commented on a change in pull request #6426: feat: add `rewrite:RespHeaders` and modify the upstream response headers via `request` implementation
Posted by GitBox <gi...@apache.org>.
rampagecong commented on a change in pull request #6426:
URL: https://github.com/apache/apisix/pull/6426#discussion_r813511307
##########
File path: apisix/plugins/ext-plugin/init.lua
##########
@@ -611,6 +611,14 @@ local rpc_handlers = {
end
end
+ local len = rewrite:RespHeadersLength()
Review comment:
OK,I'll add some test cases.
And exclude some important headers like Connection,Content-Length,Transfer-Encoding, Location,Server, WWW-Authenticate, Content-Encoding, Content-Type,Content-Location,Content-Type,Content-Language, feel free to give any other suggesions.
--
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
[GitHub] [apisix] tokers commented on a change in pull request #6426: feat: add `rewrite:RespHeaders` and modify the upstream response headers via `request` implementation
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #6426:
URL: https://github.com/apache/apisix/pull/6426#discussion_r815735569
##########
File path: apisix/plugins/ext-plugin/init.lua
##########
@@ -612,10 +612,27 @@ local rpc_handlers = {
end
local len = rewrite:RespHeadersLength()
+ local excludeRespHeader = {
Review comment:
We can just use a hash-like to save these headers so we don't need to iterate all of them, i.e. we can reduce the time complicity from O(N^2) => O(N).
--
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
[GitHub] [apisix] rampagecong commented on pull request #6426: feat: add `rewrite:RespHeaders` and modify the upstream response headers via `request` implementation
Posted by GitBox <gi...@apache.org>.
rampagecong commented on pull request #6426:
URL: https://github.com/apache/apisix/pull/6426#issuecomment-1054415079
`[root@iZbp1efn5vn52xspp2bid5Z apisix]# make test
[ info ] test -> [ Start ]
git submodule update --init --recursive
prove -I../test-nginx/lib -I./ -r -s t/
t/stream-node/sni.t ............................. nginx: [error] init_by_lua error: error loading module 'ssl.core' from file '/opt/service/test/apisix/deps/lib/lua/5.1/ssl.so':
/opt/service/test/apisix/deps/lib/lua/5.1/ssl.so: undefined symbol: sk_free
stack traceback:
[C]: at 0x7f6b5dc70a20
[C]: in function 'require'
/opt/service/test/apisix/deps/share/lua/5.1/ssl.lua:8: in main chunk
[C]: in function 'require'
/opt/service/test/apisix/apisix/patch.lua:22: in main chunk
[C]: in function 'require'
/opt/service/test/apisix/apisix/init.lua:28: in main chunk
[C]: in function 'require'
init_by_lua:9: in main chunk`
--
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
[GitHub] [apisix] tokers commented on a change in pull request #6426: feat: add `rewrite:RespHeaders` and modify the upstream response headers via `request` implementation
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #6426:
URL: https://github.com/apache/apisix/pull/6426#discussion_r816380078
##########
File path: t/lib/ext-plugin.lua
##########
@@ -367,6 +367,33 @@ function _M.go(case)
local action = http_req_call_rewrite.End(builder)
build_action(action, http_req_call_action.Rewrite)
+ elseif case.rewrite_resp_header == true then
+ local hdrs = {
Review comment:
I think you can also add some unexpected headers and verify if they exist in the response headers.
--
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
[GitHub] [apisix] rampagecong commented on pull request #6426: feat: add `rewrite:RespHeaders` and modify the upstream response headers via `request` implementation
Posted by GitBox <gi...@apache.org>.
rampagecong commented on pull request #6426:
URL: https://github.com/apache/apisix/pull/6426#issuecomment-1055263080
`[root@iZbp1efn5vn52xspp2bid5Z apisix]# prove -I. -I../test-nginx/inc -I../test-nginx/lib -r t/plugin/ext-plugin/http-req-call.t
t/plugin/ext-plugin/http-req-call.t .. ok
All tests successful.
Files=1, Tests=18, 2 wallclock secs ( 0.02 usr 0.00 sys + 0.39 cusr 0.10 csys = 0.51 CPU)
Result: PASS`
Successfully!
--
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
[GitHub] [apisix] tokers commented on a change in pull request #6426: feat: add `rewrite:RespHeaders` and modify the upstream response headers via `request` implementation
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #6426:
URL: https://github.com/apache/apisix/pull/6426#discussion_r819313454
##########
File path: apisix/plugins/ext-plugin/init.lua
##########
@@ -65,6 +65,18 @@ local type = type
local events_list
+local exclude_resp_header = {
+ ["connection"] = true,
+ ["content-length"] = true,
+ ["transfer-encoding"] = true,
+ ["location"] = true,
+ ["server"] = true,
+ ["www-authenticate"] = true,
+ ["content-encoding"] = true,
+ ["content-type"] = true,
+ ["content-location"] = true,
+ ["content-language"] = true,
Review comment:
I think we can just let it go and see the reaction of community.
--
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
[GitHub] [apisix] rampagecong commented on a change in pull request #6426: feat: add `rewrite:RespHeaders` and modify the upstream response headers via `request` implementation
Posted by GitBox <gi...@apache.org>.
rampagecong commented on a change in pull request #6426:
URL: https://github.com/apache/apisix/pull/6426#discussion_r818250650
##########
File path: apisix/plugins/ext-plugin/init.lua
##########
@@ -65,6 +65,18 @@ local type = type
local events_list
+local exclude_resp_header = {
+ ["connection"] = true,
+ ["content-length"] = true,
+ ["transfer-encoding"] = true,
+ ["location"] = true,
+ ["server"] = true,
+ ["www-authenticate"] = true,
+ ["content-encoding"] = true,
+ ["content-type"] = true,
+ ["content-location"] = true,
+ ["content-language"] = true,
Review comment:
Yes,I want use the external plugin to modify the Cookie and there is a better way?
--
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
[GitHub] [apisix] spacewander commented on a change in pull request #6426: feat: add `rewrite:RespHeaders` and modify the upstream response headers via `request` implementation
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #6426:
URL: https://github.com/apache/apisix/pull/6426#discussion_r819244706
##########
File path: apisix/plugins/ext-plugin/init.lua
##########
@@ -65,6 +65,18 @@ local type = type
local events_list
+local exclude_resp_header = {
+ ["connection"] = true,
+ ["content-length"] = true,
+ ["transfer-encoding"] = true,
+ ["location"] = true,
+ ["server"] = true,
+ ["www-authenticate"] = true,
+ ["content-encoding"] = true,
+ ["content-type"] = true,
+ ["content-location"] = true,
+ ["content-language"] = true,
Review comment:
Maybe we can allow using "Set-Cookie"? Setting Cookie won't break the upstream's response.
--
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
[GitHub] [apisix] rampagecong commented on a change in pull request #6426: feat: add `rewrite:RespHeaders` and modify the upstream response headers via `request` implementation
Posted by GitBox <gi...@apache.org>.
rampagecong commented on a change in pull request #6426:
URL: https://github.com/apache/apisix/pull/6426#discussion_r817482803
##########
File path: apisix/plugins/ext-plugin/init.lua
##########
@@ -611,6 +611,29 @@ local rpc_handlers = {
end
end
+ local len = rewrite:RespHeadersLength()
+ local exclude_resp_header = {
Review comment:
OK
--
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