janiussyafiq commented on PR #13232:
URL: https://github.com/apache/apisix/pull/13232#issuecomment-4297774564
> We recently encountered an similar issue in our online environment
involving ngx.ctx during the SSL phase and request processing phases. Here is
an example to demonstrate the problem:
>
> nginx.conf:
>
> ```nginx
> ssl_certificate_by_lua_block {
> ngx.ctx.tracing={}
> ngx.log(ngx.WARN, "id: ", tostring(ngx.ctx.tracing))
> }
> location /slow {
> content_by_lua_block {
> ngx.log(ngx.WARN, "id(slow): ", tostring(ngx.ctx.tracing))
> table.insert(ngx.ctx.tracing, "slow_1")
> ngx.sleep(1)
> table.insert(ngx.ctx.tracing, "slow_2")
> ngx.say("slow")
> }
> }
> location /fast {
> content_by_lua_block {
> ngx.log(ngx.WARN, "id(fast): ", tostring(ngx.ctx.tracing))
> table.insert(ngx.ctx.tracing, "fast_1")
> ngx.say("fast")
> }
> }
> log_by_lua_block {
> ngx.log(ngx.WARN, "data: ", table.concat(ngx.ctx.tracing, ", "),
", id:", tostring(ngx.ctx.tracing))
> ngx.ctx.tracing=nil
> ngx.log(ngx.WARN, "nil?(after set to nil): ", ngx.ctx.tracing==nil)
> ngx.log(ngx.WARN, "data(after set to nil): ",
table.concat(ngx.ctx.tracing, ", "))
> }
> ```
>
> Run the command bellow to process two requests concurrently within the
same connection:
>
> ```
> curl -Z -k --http2 https://127.0.0.1:8443/slow https://127.0.0.1:8443/fast
> ```
>
> Error Log Output:(Prefix timestamps and metadata removed)
>
> ```
> ssl_certificate_by_lua(nginx.conf:119):3: id: table: 0x7ff77b5a1060,
context: ssl_certificate_by_lua*, client: 127.0.0.1, server: 0.0.0.0:8443
> content_by_lua(nginx.conf:131):2: id(slow): table: 0x7ff77b5a1060, client:
127.0.0.1, server: localhost, request: "GET /slow HTTP/2.0", host:
"127.0.0.1:8443"
> content_by_lua(nginx.conf:141):2: id(fast): table: 0x7ff77b5a1060, client:
127.0.0.1, server: localhost, request: "GET /fast HTTP/2.0", host:
"127.0.0.1:8443"
> log_by_lua(nginx.conf:123):2: data: slow_1, fast_1, id:table:
0x7ff77b5a1060 while logging request, client: 127.0.0.1, server: localhost,
request: "GET /fast HTTP/2.0", host: "127.0.0.1:8443"
> log_by_lua(nginx.conf:123):4: nil?(after set to nil): false while logging
request, client: 127.0.0.1, server: localhost, request: "GET /fast HTTP/2.0",
host: "127.0.0.1:8443"
> log_by_lua(nginx.conf:123):5: data(after set to nil): slow_1, fast_1 while
logging request, client: 127.0.0.1, server: localhost, request: "GET /fast
HTTP/2.0", host: "127.0.0.1:8443"
> log_by_lua(nginx.conf:123):2: data: slow_1, fast_1, slow_2, id:table:
0x7ff77b5a1060 while logging request, client: 127.0.0.1, server: localhost,
request: "GET /slow HTTP/2.0", host: "127.0.0.1:8443"
> log_by_lua(nginx.conf:123):4: nil?(after set to nil): false while logging
request, client: 127.0.0.1, server: localhost, request: "GET /slow HTTP/2.0",
host: "127.0.0.1:8443"
> log_by_lua(nginx.conf:123):5: data(after set to nil): slow_1, fast_1,
slow_2 while logging request, client: 127.0.0.1, server: localhost, request:
"GET /slow HTTP/2.0", host: "127.0.0.1:8443"
> ```
>
> Key Observations from the Logs:
>
> * ngx.ctx.tracing is shared across different requests on the same
connection, leading to data interference between requests.
> * Setting ngx.ctx.tracing = nil does not achieve the expected result; the
value persists.
>
> Based on the implementation relate to ngx.ctx
[get_ctx_table](https://github.com/openresty/lua-resty-core/blob/master/lib/resty/core/ctx.lua#L67),
the ngx.ctx accessed during the SSL phase acts as the metatable for the
ngx.ctx accessed during the request processing phase.
>
> In the example above:
>
> * The value retrieved from ngx.ctx.tracing is always the one initialized
during the SSL phase.
> * Setting ngx.ctx.tracing to nil in the request phase does nothing because
the request-level ngx.ctx table does not actually contain the tracing key
itself. When access it again, Lua continues to look up the key along the
metatable and finding the value stored in the SSL-phase ngx.ctx.
>
> In this specific issue, the PR fixes the crash, the tracing data still
interference across concurrent requests.
WDYT @Baoyuantop @membphis @nic-6443 ? I can confirm that this PR solves the
crash issue, but I am not sure how to write test case to confirm no data
interference between both requests. One idea is to check span using
opentelemetry but ig we can do it in separate PR? i'm working on a solution
right now just want to have ur guys opinion.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
