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/03/29 10:28:56 UTC
[GitHub] [apisix] kwanhur opened a new pull request #6750: feat(response-rewrite): support filters
kwanhur opened a new pull request #6750:
URL: https://github.com/apache/apisix/pull/6750
### Description
<!-- Please include a summary of the change and which issue is fixed. -->
<!-- Please also include relevant motivation and context. -->
Support filters to do substitute.
Change: add property `filters`.
* match pattern can be regex or fixed string
* match mode can be once or global, default once
* ngx.re.sub(gsub) default options jo
Fixes #5451
### Checklist
- [x] I have explained the need for this PR and the problem it solves
- [x] I have explained the changes or the new features added to this PR
- [x] I have added tests corresponding to this change
- [x] I have updated the documentation to reflect this change
- [x] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first)
<!--
Note
1. Mark the PR as draft until it's ready to be reviewed.
2. Always add/update tests for any changes unless you have a good reason.
3. Always update the documentation to reflect the changes made in the PR.
4. Make a new commit to resolve conversations instead of `push -f`.
5. To resolve merge conflicts, merge master instead of rebasing.
6. Use "request review" to notify the reviewer after making changes.
7. Only a reviewer can mark a conversation as resolved.
-->
--
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] kwanhur commented on a change in pull request #6750: feat(response-rewrite): support filters
Posted by GitBox <gi...@apache.org>.
kwanhur commented on a change in pull request #6750:
URL: https://github.com/apache/apisix/pull/6750#discussion_r841179988
##########
File path: docs/en/latest/plugins/response-rewrite.md
##########
@@ -32,13 +32,20 @@ response rewrite plugin, rewrite the content returned by the upstream as well as
## Attributes
-| Name | Type | Requirement | Default | Valid | Description |
-| ----------- | ------- | ----------- | ------- | ---------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| status_code | integer | optional | | [200, 598] | New `status code` to client, keep the original response code by default. |
-| body | string | optional | | | New `body` to client, and the content-length will be reset too. |
-| body_base64 | boolean | optional | false | | Identify if `body` in configuration need base64 decoded before rewrite to client. |
-| headers | object | optional | | | Set the new `headers` for client, can set up multiple. If it exists already from upstream, will rewrite the header, otherwise will add the header. You can set the corresponding value to an empty string to remove a header. The value can contain Nginx variables in `$var` format, like `$remote_addr $balancer_ip` |
-| vars | array[] | optional | | | A DSL to evaluate with the given ngx.var. See `vars` [lua-resty-expr](https://github.com/api7/lua-resty-expr#operator-list). if the `vars` is empty, then all rewrite operations will be executed unconditionally |
+| Name | Type | Requirement | Default | Valid | Description |
+|-----------------|---------|-------------|---------|----------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
+| status_code | integer | optional | | [200, 598] | New `status code` to client, keep the original response code by default. |
+| body | string | optional | | | New `body` to client, and the content-length will be reset too. |
+| body_base64 | boolean | optional | false | | Identify if `body` in configuration need base64 decoded before rewrite to client. |
+| headers | object | optional | | | Set the new `headers` for client, can set up multiple. If it exists already from upstream, will rewrite the header, otherwise will add the header. You can set the corresponding value to an empty string to remove a header. The value can contain Nginx variables in `$var` format, like `$remote_addr $balancer_ip`. |
+| vars | array[] | optional | | | A DSL to evaluate with the given ngx.var. See `vars` [lua-resty-expr](https://github.com/api7/lua-resty-expr#operator-list). if the `vars` is empty, then all rewrite operations will be executed unconditionally. |
+| filters | array[] | optional | | | A group of filters that modify response body by replacing one specified string by another. |
+| filters.regex | string | optional | | | match pattern on response body. |
+| filters.scope | string | optional | "one" | "one","global" | substitution range. |
Review comment:
Yep, fixed 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: 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 #6750: feat(response-rewrite): support filters
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #6750:
URL: https://github.com/apache/apisix/pull/6750#discussion_r840992956
##########
File path: apisix/plugins/response-rewrite.lua
##########
@@ -126,7 +177,25 @@ function _M.body_filter(conf, ctx)
return
end
- if conf.body then
+ if conf.filters then
+
+ local body = core.response.hold_body_chunk(ctx)
+ if not body then
+ return
+ end
+
+ for _, filter in ipairs(conf.filters) do
+ if filter.scope == "once" then
+ body = re_sub(body, filter.regex, filter.replace, filter.options)
+ else
+ body = re_gsub(body, filter.regex, filter.replace, filter.options)
+ end
+ end
+
+ ngx.arg[1] = body
+ return
+
+ elseif conf.body then
Review comment:
We can use `if` here directly.
##########
File path: docs/en/latest/plugins/response-rewrite.md
##########
@@ -32,13 +32,20 @@ response rewrite plugin, rewrite the content returned by the upstream as well as
## Attributes
-| Name | Type | Requirement | Default | Valid | Description |
-| ----------- | ------- | ----------- | ------- | ---------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| status_code | integer | optional | | [200, 598] | New `status code` to client, keep the original response code by default. |
-| body | string | optional | | | New `body` to client, and the content-length will be reset too. |
-| body_base64 | boolean | optional | false | | Identify if `body` in configuration need base64 decoded before rewrite to client. |
-| headers | object | optional | | | Set the new `headers` for client, can set up multiple. If it exists already from upstream, will rewrite the header, otherwise will add the header. You can set the corresponding value to an empty string to remove a header. The value can contain Nginx variables in `$var` format, like `$remote_addr $balancer_ip` |
-| vars | array[] | optional | | | A DSL to evaluate with the given ngx.var. See `vars` [lua-resty-expr](https://github.com/api7/lua-resty-expr#operator-list). if the `vars` is empty, then all rewrite operations will be executed unconditionally |
+| Name | Type | Requirement | Default | Valid | Description |
+|-----------------|---------|-------------|---------|----------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
+| status_code | integer | optional | | [200, 598] | New `status code` to client, keep the original response code by default. |
+| body | string | optional | | | New `body` to client, and the content-length will be reset too. |
+| body_base64 | boolean | optional | false | | Identify if `body` in configuration need base64 decoded before rewrite to client. |
+| headers | object | optional | | | Set the new `headers` for client, can set up multiple. If it exists already from upstream, will rewrite the header, otherwise will add the header. You can set the corresponding value to an empty string to remove a header. The value can contain Nginx variables in `$var` format, like `$remote_addr $balancer_ip`. |
+| vars | array[] | optional | | | A DSL to evaluate with the given ngx.var. See `vars` [lua-resty-expr](https://github.com/api7/lua-resty-expr#operator-list). if the `vars` is empty, then all rewrite operations will be executed unconditionally. |
+| filters | array[] | optional | | | A group of filters that modify response body by replacing one specified string by another. |
+| filters.regex | string | optional | | | match pattern on response body. |
+| filters.scope | string | optional | "one" | "one","global" | substitution range. |
Review comment:
Should be `once`?
##########
File path: t/plugin/response-rewrite2.t
##########
@@ -0,0 +1,516 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+BEGIN {
+ if ($ENV{TEST_NGINX_CHECK_LEAK}) {
+ $SkipReason = "unavailable for the hup tests";
+
+ } else {
+ $ENV{TEST_NGINX_USE_HUP} = 1;
+ undef $ENV{TEST_NGINX_USE_STAP};
+ }
+}
+
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_shuffle();
+no_root_location();
+run_tests;
+
+__DATA__
+
+=== TEST 1: add plugin with valid filters
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.response-rewrite")
+ local ok, err = plugin.check_schema({
+ filters = {
+ {
+ regex = "Hello",
+ scope = "global",
+ replace = "World",
+ options = "jo"
+ }
+ }
+ })
+ if not ok then
+ ngx.say(err)
+ end
+
+ ngx.say("done")
+ }
+ }
+--- request
+GET /t
+--- response_body
+done
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: add plugin with invalid filter scope
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.response-rewrite")
+ local ok, err = plugin.check_schema({
+ filters = {
+ {
+ regex = "Hello",
+ scope = "two",
+ replace = "World",
+ options = "jo"
+ }
+ }
+ })
+ if not ok then
+ ngx.say(err)
+ else
+ ngx.say("done")
+ end
+ }
+ }
+--- request
+GET /t
+--- response_body
+property "filters" validation failed: failed to validate item 1: property "scope" validation failed: matches none of the enum values
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: add plugin with invalid filter empty value
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.response-rewrite")
+ local ok, err = plugin.check_schema({
+ filters = {
+ {
+ regex = "",
+ replace = "world"
+ }
+ }
+ })
+ if not ok then
+ ngx.say(err)
+ else
+ ngx.say("done")
+ end
+ }
+ }
+--- request
+GET /t
+--- response_body
+invalid value as filter field regex
+--- no_error_log
+[error]
+
+
+
+=== TEST 4: add plugin with invalid filter regex options
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.response-rewrite")
+ local ok, err = plugin.check_schema({
+ filters = {
+ {
+ regex = "hello",
+ options = "h"
+ }
+ }
+ })
+ if not ok then
+ ngx.say(err)
+ else
+ ngx.say("done")
+ end
+ }
+ }
+--- request
+GET /t
+--- error_code eval
Review comment:
Why it's output is unlike the previous test?
--
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] kwanhur commented on a change in pull request #6750: feat(response-rewrite): support filters
Posted by GitBox <gi...@apache.org>.
kwanhur commented on a change in pull request #6750:
URL: https://github.com/apache/apisix/pull/6750#discussion_r841185095
##########
File path: t/plugin/response-rewrite2.t
##########
@@ -0,0 +1,516 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+BEGIN {
+ if ($ENV{TEST_NGINX_CHECK_LEAK}) {
+ $SkipReason = "unavailable for the hup tests";
+
+ } else {
+ $ENV{TEST_NGINX_USE_HUP} = 1;
+ undef $ENV{TEST_NGINX_USE_STAP};
+ }
+}
+
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_shuffle();
+no_root_location();
+run_tests;
+
+__DATA__
+
+=== TEST 1: add plugin with valid filters
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.response-rewrite")
+ local ok, err = plugin.check_schema({
+ filters = {
+ {
+ regex = "Hello",
+ scope = "global",
+ replace = "World",
+ options = "jo"
+ }
+ }
+ })
+ if not ok then
+ ngx.say(err)
+ end
+
+ ngx.say("done")
+ }
+ }
+--- request
+GET /t
+--- response_body
+done
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: add plugin with invalid filter scope
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.response-rewrite")
+ local ok, err = plugin.check_schema({
+ filters = {
+ {
+ regex = "Hello",
+ scope = "two",
+ replace = "World",
+ options = "jo"
+ }
+ }
+ })
+ if not ok then
+ ngx.say(err)
+ else
+ ngx.say("done")
+ end
+ }
+ }
+--- request
+GET /t
+--- response_body
+property "filters" validation failed: failed to validate item 1: property "scope" validation failed: matches none of the enum values
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: add plugin with invalid filter empty value
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.response-rewrite")
+ local ok, err = plugin.check_schema({
+ filters = {
+ {
+ regex = "",
+ replace = "world"
+ }
+ }
+ })
+ if not ok then
+ ngx.say(err)
+ else
+ ngx.say("done")
+ end
+ }
+ }
+--- request
+GET /t
+--- response_body
+invalid value as filter field regex
+--- no_error_log
+[error]
+
+
+
+=== TEST 4: add plugin with invalid filter regex options
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.response-rewrite")
+ local ok, err = plugin.check_schema({
+ filters = {
+ {
+ regex = "hello",
+ options = "h"
+ }
+ }
+ })
+ if not ok then
+ ngx.say(err)
+ else
+ ngx.say("done")
+ end
+ }
+ }
+--- request
+GET /t
+--- error_code eval
Review comment:
How about use `xpcall` to catch the err?
--
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] kwanhur commented on a change in pull request #6750: feat(response-rewrite): support filters
Posted by GitBox <gi...@apache.org>.
kwanhur commented on a change in pull request #6750:
URL: https://github.com/apache/apisix/pull/6750#discussion_r840624364
##########
File path: t/plugin/response-rewrite.t
##########
@@ -699,3 +699,512 @@ X-A: 127.0.0.1
X-B: from 127.0.0.1 to 127.0.0.1:1980
--- no_error_log
[error]
+
+
+
+=== TEST 25: add plugin with valid filters
Review comment:
All the filter related test had moved to `response-rewrite2.t`.
--
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] kwanhur commented on a change in pull request #6750: feat(response-rewrite): support filters
Posted by GitBox <gi...@apache.org>.
kwanhur commented on a change in pull request #6750:
URL: https://github.com/apache/apisix/pull/6750#discussion_r841183230
##########
File path: t/plugin/response-rewrite2.t
##########
@@ -0,0 +1,516 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+BEGIN {
+ if ($ENV{TEST_NGINX_CHECK_LEAK}) {
+ $SkipReason = "unavailable for the hup tests";
+
+ } else {
+ $ENV{TEST_NGINX_USE_HUP} = 1;
+ undef $ENV{TEST_NGINX_USE_STAP};
+ }
+}
+
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_shuffle();
+no_root_location();
+run_tests;
+
+__DATA__
+
+=== TEST 1: add plugin with valid filters
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.response-rewrite")
+ local ok, err = plugin.check_schema({
+ filters = {
+ {
+ regex = "Hello",
+ scope = "global",
+ replace = "World",
+ options = "jo"
+ }
+ }
+ })
+ if not ok then
+ ngx.say(err)
+ end
+
+ ngx.say("done")
+ }
+ }
+--- request
+GET /t
+--- response_body
+done
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: add plugin with invalid filter scope
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.response-rewrite")
+ local ok, err = plugin.check_schema({
+ filters = {
+ {
+ regex = "Hello",
+ scope = "two",
+ replace = "World",
+ options = "jo"
+ }
+ }
+ })
+ if not ok then
+ ngx.say(err)
+ else
+ ngx.say("done")
+ end
+ }
+ }
+--- request
+GET /t
+--- response_body
+property "filters" validation failed: failed to validate item 1: property "scope" validation failed: matches none of the enum values
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: add plugin with invalid filter empty value
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.response-rewrite")
+ local ok, err = plugin.check_schema({
+ filters = {
+ {
+ regex = "",
+ replace = "world"
+ }
+ }
+ })
+ if not ok then
+ ngx.say(err)
+ else
+ ngx.say("done")
+ end
+ }
+ }
+--- request
+GET /t
+--- response_body
+invalid value as filter field regex
+--- no_error_log
+[error]
+
+
+
+=== TEST 4: add plugin with invalid filter regex options
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.response-rewrite")
+ local ok, err = plugin.check_schema({
+ filters = {
+ {
+ regex = "hello",
+ options = "h"
+ }
+ }
+ })
+ if not ok then
+ ngx.say(err)
+ else
+ ngx.say("done")
+ end
+ }
+ }
+--- request
+GET /t
+--- error_code eval
Review comment:
Yes, it's unlike. Because when invalid option, it'll abort with exception like [here](https://gist.github.com/kwanhur/dc17e10efe1a029b65225a8a9f2a58e8) and return `500` status with body default `5xx.html`.
So add prefix on `err` has no effect.
--
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] kwanhur commented on a change in pull request #6750: feat(response-rewrite): support filters
Posted by GitBox <gi...@apache.org>.
kwanhur commented on a change in pull request #6750:
URL: https://github.com/apache/apisix/pull/6750#discussion_r840622850
##########
File path: apisix/plugins/response-rewrite.lua
##########
@@ -115,6 +152,23 @@ function _M.check_schema(conf)
end
end
+ if conf.filters then
+ for _, filter in ipairs(conf.filters) do
+ for field, value in pairs(filter) do
Review comment:
`require` seems not suitable. One of `conf.body` `conf.filters` should specify, so has used `oneOf` in the schema.
--
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] kwanhur commented on a change in pull request #6750: feat(response-rewrite): support filters
Posted by GitBox <gi...@apache.org>.
kwanhur commented on a change in pull request #6750:
URL: https://github.com/apache/apisix/pull/6750#discussion_r841182879
##########
File path: apisix/plugins/response-rewrite.lua
##########
@@ -48,6 +52,40 @@ local schema = {
vars = {
type = "array",
},
+ filters = {
+ description = "a group of filters that modify response body\
Review comment:
Done.
--
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] kwanhur commented on a change in pull request #6750: feat(response-rewrite): support filters
Posted by GitBox <gi...@apache.org>.
kwanhur commented on a change in pull request #6750:
URL: https://github.com/apache/apisix/pull/6750#discussion_r840623291
##########
File path: apisix/plugins/response-rewrite.lua
##########
@@ -115,6 +152,23 @@ function _M.check_schema(conf)
end
end
+ if conf.filters then
+ for _, filter in ipairs(conf.filters) do
+ for field, value in pairs(filter) do
+ if type(field) ~= 'string' then
+ return false, 'invalid type as filter field'
+ end
+ if field ~= "replace" and value == "" then
+ return false, 'invalid value as filter field ' .. field
+ end
+ end
+ local ok, err = re_compile(filter.regex, filter.options)
+ if not ok then
+ return false, err
Review comment:
Done.
--
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] kwanhur commented on a change in pull request #6750: feat(response-rewrite): support filters
Posted by GitBox <gi...@apache.org>.
kwanhur commented on a change in pull request #6750:
URL: https://github.com/apache/apisix/pull/6750#discussion_r840623587
##########
File path: docs/en/latest/plugins/response-rewrite.md
##########
@@ -32,13 +32,25 @@ response rewrite plugin, rewrite the content returned by the upstream as well as
## Attributes
-| Name | Type | Requirement | Default | Valid | Description |
-| ----------- | ------- | ----------- | ------- | ---------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| status_code | integer | optional | | [200, 598] | New `status code` to client, keep the original response code by default. |
-| body | string | optional | | | New `body` to client, and the content-length will be reset too. |
-| body_base64 | boolean | optional | false | | Identify if `body` in configuration need base64 decoded before rewrite to client. |
-| headers | object | optional | | | Set the new `headers` for client, can set up multiple. If it exists already from upstream, will rewrite the header, otherwise will add the header. You can set the corresponding value to an empty string to remove a header. The value can contain Nginx variables in `$var` format, like `$remote_addr $balancer_ip` |
-| vars | array[] | optional | | | A DSL to evaluate with the given ngx.var. See `vars` [lua-resty-expr](https://github.com/api7/lua-resty-expr#operator-list). if the `vars` is empty, then all rewrite operations will be executed unconditionally |
+| Name | Type | Requirement | Default | Valid | Description |
+|-------------|---------|-------------|---------|------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
+| status_code | integer | optional | | [200, 598] | New `status code` to client, keep the original response code by default. |
+| body | string | optional | | | New `body` to client, and the content-length will be reset too. |
+| body_base64 | boolean | optional | false | | Identify if `body` in configuration need base64 decoded before rewrite to client. |
+| headers | object | optional | | | Set the new `headers` for client, can set up multiple. If it exists already from upstream, will rewrite the header, otherwise will add the header. You can set the corresponding value to an empty string to remove a header. The value can contain Nginx variables in `$var` format, like `$remote_addr $balancer_ip`. |
+| vars | array[] | optional | | | A DSL to evaluate with the given ngx.var. See `vars` [lua-resty-expr](https://github.com/api7/lua-resty-expr#operator-list). if the `vars` is empty, then all rewrite operations will be executed unconditionally. |
+| filters | array[] | optional | | | A group of filters that modify response body by replacing one specified string by another. |
+
+Only one of `body`, `filters` can be specified.
+
+### Attributes of filters
+
+| Name | Type | Requirement | Default | Valid | Description |
+|---------|--------|-------------|---------|----------------|------------------------------------------------------------------------------------------------|
+| regex | string | required | | | match pattern on response body. |
+| scope | string | required | "one" | "one","global" | substitution range. |
Review comment:
Had updated them.
--
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 #6750: feat(response-rewrite): support filters
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #6750:
URL: https://github.com/apache/apisix/pull/6750#discussion_r840992845
##########
File path: apisix/plugins/response-rewrite.lua
##########
@@ -115,6 +152,23 @@ function _M.check_schema(conf)
end
end
+ if conf.filters then
+ for _, filter in ipairs(conf.filters) do
+ for field, value in pairs(filter) do
Review comment:
I mean a `require` for the filter.
##########
File path: apisix/plugins/response-rewrite.lua
##########
@@ -48,6 +52,40 @@ local schema = {
vars = {
type = "array",
},
+ filters = {
+ description = "a group of filters that modify response body\
Review comment:
We can use str concat
##########
File path: apisix/plugins/response-rewrite.lua
##########
@@ -115,6 +152,23 @@ function _M.check_schema(conf)
end
end
+ if conf.filters then
+ for _, filter in ipairs(conf.filters) do
+ for field, value in pairs(filter) do
Review comment:
@kwanhur
I mean a `require` for the filter.
--
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 #6750: feat(response-rewrite): support filters
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #6750:
URL: https://github.com/apache/apisix/pull/6750#discussion_r840194071
##########
File path: apisix/plugins/response-rewrite.lua
##########
@@ -115,6 +152,23 @@ function _M.check_schema(conf)
end
end
+ if conf.filters then
+ for _, filter in ipairs(conf.filters) do
+ for field, value in pairs(filter) do
Review comment:
We can use `require` in the schema to do the check.
##########
File path: docs/en/latest/plugins/response-rewrite.md
##########
@@ -32,13 +32,25 @@ response rewrite plugin, rewrite the content returned by the upstream as well as
## Attributes
-| Name | Type | Requirement | Default | Valid | Description |
-| ----------- | ------- | ----------- | ------- | ---------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| status_code | integer | optional | | [200, 598] | New `status code` to client, keep the original response code by default. |
-| body | string | optional | | | New `body` to client, and the content-length will be reset too. |
-| body_base64 | boolean | optional | false | | Identify if `body` in configuration need base64 decoded before rewrite to client. |
-| headers | object | optional | | | Set the new `headers` for client, can set up multiple. If it exists already from upstream, will rewrite the header, otherwise will add the header. You can set the corresponding value to an empty string to remove a header. The value can contain Nginx variables in `$var` format, like `$remote_addr $balancer_ip` |
-| vars | array[] | optional | | | A DSL to evaluate with the given ngx.var. See `vars` [lua-resty-expr](https://github.com/api7/lua-resty-expr#operator-list). if the `vars` is empty, then all rewrite operations will be executed unconditionally |
+| Name | Type | Requirement | Default | Valid | Description |
+|-------------|---------|-------------|---------|------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
+| status_code | integer | optional | | [200, 598] | New `status code` to client, keep the original response code by default. |
+| body | string | optional | | | New `body` to client, and the content-length will be reset too. |
+| body_base64 | boolean | optional | false | | Identify if `body` in configuration need base64 decoded before rewrite to client. |
+| headers | object | optional | | | Set the new `headers` for client, can set up multiple. If it exists already from upstream, will rewrite the header, otherwise will add the header. You can set the corresponding value to an empty string to remove a header. The value can contain Nginx variables in `$var` format, like `$remote_addr $balancer_ip`. |
+| vars | array[] | optional | | | A DSL to evaluate with the given ngx.var. See `vars` [lua-resty-expr](https://github.com/api7/lua-resty-expr#operator-list). if the `vars` is empty, then all rewrite operations will be executed unconditionally. |
+| filters | array[] | optional | | | A group of filters that modify response body by replacing one specified string by another. |
+
+Only one of `body`, `filters` can be specified.
+
+### Attributes of filters
+
+| Name | Type | Requirement | Default | Valid | Description |
+|---------|--------|-------------|---------|----------------|------------------------------------------------------------------------------------------------|
+| regex | string | required | | | match pattern on response body. |
+| scope | string | required | "one" | "one","global" | substitution range. |
Review comment:
We should provide the doc of the nested field like `tls.client_cert` in https://github.com/apache/apisix/blob/master/docs/en/latest/admin-api.md.
And it is optional as people don't need to configure it.
##########
File path: apisix/plugins/response-rewrite.lua
##########
@@ -126,7 +180,24 @@ function _M.body_filter(conf, ctx)
return
end
- if conf.body then
+ if conf.filters then
+
+ local body = core.response.hold_body_chunk(ctx)
+ if not body then
+ return
+ end
+
+ for _, filter in ipairs(conf.filters) do
+ if filter.scope == "once" then
+ body = re_sub(body, filter.regex, filter.replace, filter.options)
+ else
+ body = re_gsub(body, filter.regex, filter.replace, filter.options)
+ end
+ end
+
+ ngx.arg[1] = body
+
Review comment:
Would be better to `return` from here
##########
File path: apisix/plugins/response-rewrite.lua
##########
@@ -148,7 +219,8 @@ function _M.header_filter(conf, ctx)
ngx.status = conf.status_code
end
- if conf.body then
+ -- fixme: if filters have no any match, response body won't be modified.
Review comment:
```suggestion
-- if filters have no any match, response body won't be modified.
```
##########
File path: t/plugin/response-rewrite.t
##########
@@ -699,3 +699,512 @@ X-A: 127.0.0.1
X-B: from 127.0.0.1 to 127.0.0.1:1980
--- no_error_log
[error]
+
+
+
+=== TEST 25: add plugin with valid filters
Review comment:
Let's move the new test to t/plugin/response-rewrite2.t. Otherwise this test file is too long.
##########
File path: apisix/plugins/response-rewrite.lua
##########
@@ -115,6 +152,23 @@ function _M.check_schema(conf)
end
end
+ if conf.filters then
+ for _, filter in ipairs(conf.filters) do
+ for field, value in pairs(filter) do
+ if type(field) ~= 'string' then
Review comment:
We don't need to check the type of field. It doesn't do any harm as we only consume known fields.
##########
File path: apisix/plugins/response-rewrite.lua
##########
@@ -115,6 +152,23 @@ function _M.check_schema(conf)
end
end
+ if conf.filters then
+ for _, filter in ipairs(conf.filters) do
+ for field, value in pairs(filter) do
+ if type(field) ~= 'string' then
+ return false, 'invalid type as filter field'
+ end
+ if field ~= "replace" and value == "" then
+ return false, 'invalid value as filter field ' .. field
+ end
+ end
+ local ok, err = re_compile(filter.regex, filter.options)
+ if not ok then
+ return false, err
Review comment:
Better to add a prefix to the err
--
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] kwanhur commented on a change in pull request #6750: feat(response-rewrite): support filters
Posted by GitBox <gi...@apache.org>.
kwanhur commented on a change in pull request #6750:
URL: https://github.com/apache/apisix/pull/6750#discussion_r841180143
##########
File path: apisix/plugins/response-rewrite.lua
##########
@@ -126,7 +177,25 @@ function _M.body_filter(conf, ctx)
return
end
- if conf.body then
+ if conf.filters then
+
+ local body = core.response.hold_body_chunk(ctx)
+ if not body then
+ return
+ end
+
+ for _, filter in ipairs(conf.filters) do
+ if filter.scope == "once" then
+ body = re_sub(body, filter.regex, filter.replace, filter.options)
+ else
+ body = re_gsub(body, filter.regex, filter.replace, filter.options)
+ end
+ end
+
+ ngx.arg[1] = body
+ return
+
+ elseif conf.body then
Review comment:
Ok, updated here.
--
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] kwanhur commented on a change in pull request #6750: feat(response-rewrite): support filters
Posted by GitBox <gi...@apache.org>.
kwanhur commented on a change in pull request #6750:
URL: https://github.com/apache/apisix/pull/6750#discussion_r841175005
##########
File path: apisix/plugins/response-rewrite.lua
##########
@@ -115,6 +152,23 @@ function _M.check_schema(conf)
end
end
+ if conf.filters then
+ for _, filter in ipairs(conf.filters) do
+ for field, value in pairs(filter) do
Review comment:
> @kwanhur I mean a `require` for the filter.
I had placed `minItems = 1` limitation. I found many array type properties also use 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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] kwanhur commented on a change in pull request #6750: feat(response-rewrite): support filters
Posted by GitBox <gi...@apache.org>.
kwanhur commented on a change in pull request #6750:
URL: https://github.com/apache/apisix/pull/6750#discussion_r840624827
##########
File path: apisix/plugins/response-rewrite.lua
##########
@@ -115,6 +152,23 @@ function _M.check_schema(conf)
end
end
+ if conf.filters then
+ for _, filter in ipairs(conf.filters) do
+ for field, value in pairs(filter) do
+ if type(field) ~= 'string' then
Review comment:
Ok, had remove it.
##########
File path: apisix/plugins/response-rewrite.lua
##########
@@ -126,7 +180,24 @@ function _M.body_filter(conf, ctx)
return
end
- if conf.body then
+ if conf.filters then
+
+ local body = core.response.hold_body_chunk(ctx)
+ if not body then
+ return
+ end
+
+ for _, filter in ipairs(conf.filters) do
+ if filter.scope == "once" then
+ body = re_sub(body, filter.regex, filter.replace, filter.options)
+ else
+ body = re_gsub(body, filter.regex, filter.replace, filter.options)
+ end
+ end
+
+ ngx.arg[1] = body
+
Review comment:
Done.
--
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 #6750: feat(response-rewrite): support filters
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #6750:
URL: https://github.com/apache/apisix/pull/6750#discussion_r840993457
##########
File path: t/plugin/response-rewrite2.t
##########
@@ -0,0 +1,516 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+BEGIN {
+ if ($ENV{TEST_NGINX_CHECK_LEAK}) {
+ $SkipReason = "unavailable for the hup tests";
+
+ } else {
+ $ENV{TEST_NGINX_USE_HUP} = 1;
+ undef $ENV{TEST_NGINX_USE_STAP};
+ }
+}
+
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_shuffle();
+no_root_location();
+run_tests;
+
+__DATA__
+
+=== TEST 1: add plugin with valid filters
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.response-rewrite")
+ local ok, err = plugin.check_schema({
+ filters = {
+ {
+ regex = "Hello",
+ scope = "global",
+ replace = "World",
+ options = "jo"
+ }
+ }
+ })
+ if not ok then
+ ngx.say(err)
+ end
+
+ ngx.say("done")
+ }
+ }
+--- request
+GET /t
+--- response_body
+done
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: add plugin with invalid filter scope
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.response-rewrite")
+ local ok, err = plugin.check_schema({
+ filters = {
+ {
+ regex = "Hello",
+ scope = "two",
+ replace = "World",
+ options = "jo"
+ }
+ }
+ })
+ if not ok then
+ ngx.say(err)
+ else
+ ngx.say("done")
+ end
+ }
+ }
+--- request
+GET /t
+--- response_body
+property "filters" validation failed: failed to validate item 1: property "scope" validation failed: matches none of the enum values
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: add plugin with invalid filter empty value
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.response-rewrite")
+ local ok, err = plugin.check_schema({
+ filters = {
+ {
+ regex = "",
+ replace = "world"
+ }
+ }
+ })
+ if not ok then
+ ngx.say(err)
+ else
+ ngx.say("done")
+ end
+ }
+ }
+--- request
+GET /t
+--- response_body
+invalid value as filter field regex
+--- no_error_log
+[error]
+
+
+
+=== TEST 4: add plugin with invalid filter regex options
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.response-rewrite")
+ local ok, err = plugin.check_schema({
+ filters = {
+ {
+ regex = "hello",
+ options = "h"
+ }
+ }
+ })
+ if not ok then
+ ngx.say(err)
+ else
+ ngx.say("done")
+ end
+ }
+ }
+--- request
+GET /t
+--- error_code eval
Review comment:
Its output is unlike the previous test?
--
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