You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "kingluo (via GitHub)" <gi...@apache.org> on 2023/04/03 09:57:27 UTC
[GitHub] [apisix] kingluo opened a new issue, #9228: bug: hold_body_chunk can not be used in more than one plugin
kingluo opened a new issue, #9228:
URL: https://github.com/apache/apisix/issues/9228
### Current Behavior
hold_body_chunk uses a global buffer associated with the request context, then if two plugins use it to hold the body, the plugin with a smaller priority will get nil, because the `ctx._body_buffer` is reset after the body collection is finished.
https://github.com/apache/apisix/blob/809ba09b26ddd62e0efa612f85e90d1aa938ce02/apisix/core/response.lua#L195-L204
**Use case:**
The body-transformer plugin should use the body provided by the proxy-cache plugin. They both use `hold_body_chunk()`.
### Expected Behavior
hold_body_chunk should use a specified buffer if provided, other than `ctx._body_buffer`.
### Error Logs
2023/04/03 17:13:25 [error] 2707597#2707597: *227 [lua] body-transformer.lua:145: transform(): response template rendering: [string "context=... or {}..."]:7: attempt t
o index global 'Envelope' (a nil value) while sending to client, client: 127.0.0.1, server: _, request: "POST /capital HTTP/1.1", upstream: "http://127.0.0.1:3000/websa
mples.countryinfo/CountryInfoService.wso", host: "127.0.0.1:9080"
### Steps to Reproduce
Refer to #9226
### Environment
- APISIX version (run `apisix version`): master
- Operating system (run `uname -a`):
- OpenResty / Nginx version (run `openresty -V` or `nginx -V`):
- etcd version, if relevant (run `curl http://127.0.0.1:9090/v1/server_info`):
- APISIX Dashboard version, if relevant:
- Plugin runner version, for issues related to plugin runners:
- LuaRocks version, for installation issues (run `luarocks --version`):
--
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.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] monkeyDluffy6017 commented on issue #9228: bug: hold_body_chunk can not be used in more than one plugin
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on issue #9228:
URL: https://github.com/apache/apisix/issues/9228#issuecomment-1495288641
@kingluo If change like yours, will the lower priority plugins get the processing results of the higher priority plugins?
--
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] kingluo commented on issue #9228: bug: hold_body_chunk can not be used in more than one plugin
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on issue #9228:
URL: https://github.com/apache/apisix/issues/9228#issuecomment-1495291454
Yes, it's necessary and desirable.
--
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] kingluo commented on issue #9228: bug: hold_body_chunk can not be used in more than one plugin
Posted by "kingluo (via GitHub)" <gi...@apache.org>.
kingluo commented on issue #9228:
URL: https://github.com/apache/apisix/issues/9228#issuecomment-1495272975
@leslie-tsang
IMO, the best solution is to export the plugin_name in the `ctx` when running the plugin, so that `hold_body_chunk()` could collect the body in the buffer bound to the plugin to avoid pollution.
**Tested patch:**
```diff
diff --git a/apisix/core/response.lua b/apisix/core/response.lua
index b934d94b..c7eebdc3 100644
--- a/apisix/core/response.lua
+++ b/apisix/core/response.lua
@@ -178,13 +178,15 @@ function _M.hold_body_chunk(ctx, hold_the_copy)
local body_buffer
local chunk, eof = arg[1], arg[2]
if type(chunk) == "string" and chunk ~= "" then
- body_buffer = ctx._body_buffer
+ if not ctx._body_buffer then
+ ctx._body_buffer = {}
+ end
if not body_buffer then
body_buffer = {
chunk,
n = 1
}
- ctx._body_buffer = body_buffer
+ ctx._body_buffer[ctx._plugin_name] = body_buffer
else
local n = body_buffer.n + 1
body_buffer.n = n
@@ -193,13 +195,13 @@ function _M.hold_body_chunk(ctx, hold_the_copy)
end
if eof then
- body_buffer = ctx._body_buffer
+ body_buffer = ctx._body_buffer[ctx._plugin_name]
if not body_buffer then
return chunk
end
body_buffer = concat_tab(body_buffer, "", 1, body_buffer.n)
- ctx._body_buffer = nil
+ ctx._body_buffer[ctx._plugin_name] = nil
return body_buffer
end
diff --git a/apisix/plugin.lua b/apisix/plugin.lua
index 3b5cecac..a7d55db7 100644
--- a/apisix/plugin.lua
+++ b/apisix/plugin.lua
@@ -1091,7 +1091,9 @@ function _M.run_plugin(phase, plugins, api_ctx)
end
plugin_run = true
+ api_ctx._plugin_name = plugins[i]["name"]
local code, body = phase_func(conf, api_ctx)
+ api_ctx._plugin_name = nil
if code or body then
if is_http then
if code >= 400 then
@@ -1126,7 +1128,9 @@ function _M.run_plugin(phase, plugins, api_ctx)
local conf = plugins[i + 1]
if phase_func and meta_filter(api_ctx, plugins[i]["name"], conf) then
plugin_run = true
+ api_ctx._plugin_name = plugins[i]["name"]
phase_func(conf, api_ctx)
+ api_ctx._plugin_name = nil
end
end
```
--
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] monkeyDluffy6017 closed issue #9228: bug: hold_body_chunk can not be used in more than one plugin
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 closed issue #9228: bug: hold_body_chunk can not be used in more than one plugin
URL: https://github.com/apache/apisix/issues/9228
--
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