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 2020/12/22 11:18:07 UTC
[GitHub] [apisix] nic-chen opened a new pull request #3105: fix: the bug that traffic is forwarded to wrong upstream due to ctx being contaminated
nic-chen opened a new pull request #3105:
URL: https://github.com/apache/apisix/pull/3105
### What this PR does / why we need it:
<!--- Why is this change required? What problem does it solve? -->
A new feature has been added in OpenResty 1.19.3.1:
`Shared ngx.ctx among SSL_* phases and the following phases.`
In the ssl phase, ctx is created but not cleared.
In the access phase, it is judged that if ctx already exists, it will not be created.
This causes a same client to access an `https` service and then access another `http` service, it will be reused the previous ctx, resulting in this bug.
<!--- If it fixes an open issue, please link to the issue here. -->
fix #3079
### Pre-submission checklist:
* [x] Did you explain what problem does this PR solve? Or what new features have been added?
* [ ] Have you added corresponding test cases?
we could add test cases later for quick fix. we could track it by #3103
* [ ] Have you modified the corresponding document?
we don't need to modify docs for this change.
* [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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #3105: fix: ctx being contaminated due to a new feature of openresty 1.19
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #3105:
URL: https://github.com/apache/apisix/pull/3105#discussion_r547998201
##########
File path: apisix/init.lua
##########
@@ -185,6 +185,9 @@ function _M.http_ssl_phase()
end
ngx_exit(-1)
Review comment:
If the SSL handshake fails, there will be no more HTTP requests.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #3105: fix: ctx being contaminated due to a new feature of openresty 1.19
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #3105:
URL: https://github.com/apache/apisix/pull/3105#discussion_r548341150
##########
File path: apisix/init.lua
##########
@@ -185,6 +185,9 @@ function _M.http_ssl_phase()
end
ngx_exit(-1)
Review comment:
@membphis
You are 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #3105: fix: the bug that traffic is forwarded to wrong upstream due to ctx being contaminated
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #3105:
URL: https://github.com/apache/apisix/pull/3105#issuecomment-749562845
related issue: https://github.com/apache/apisix/issues/3103
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #3105: fix: ctx being contaminated due to a new feature of openresty 1.19
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #3105:
URL: https://github.com/apache/apisix/pull/3105#discussion_r547584817
##########
File path: apisix/init.lua
##########
@@ -185,6 +185,9 @@ function _M.http_ssl_phase()
end
ngx_exit(-1)
Review comment:
Should we also clear the `ngx.ctx` 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-chen commented on a change in pull request #3105: fix: the bug that traffic is forwarded to wrong upstream due to ctx being contaminated
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #3105:
URL: https://github.com/apache/apisix/pull/3105#discussion_r547316345
##########
File path: apisix/init.lua
##########
@@ -185,6 +185,7 @@ function _M.http_ssl_phase()
end
ngx_exit(-1)
end
+ ngx.ctx = nil
Review comment:
fixed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #3105: fix: ctx being contaminated due to a new feature of openresty 1.19
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #3105:
URL: https://github.com/apache/apisix/pull/3105#issuecomment-751720209
@nic-chen please update your source code editor to avoid those issue: https://github.com/apache/apisix/pull/3105/commits/7f000047f74811cd4b8521139e6e51f5401b4a5d
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-chen commented on a change in pull request #3105: fix: the bug that traffic is forwarded to wrong upstream due to ctx being contaminated
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #3105:
URL: https://github.com/apache/apisix/pull/3105#discussion_r547316402
##########
File path: apisix/init.lua
##########
@@ -334,12 +335,8 @@ end
function _M.http_access_phase()
local ngx_ctx = ngx.ctx
- local api_ctx = ngx_ctx.api_ctx
-
- if not api_ctx then
- api_ctx = core.tablepool.fetch("api_ctx", 0, 32)
- ngx_ctx.api_ctx = api_ctx
- end
+ local api_ctx = core.tablepool.fetch("api_ctx", 0, 32)
+ ngx_ctx.api_ctx = api_ctx
Review comment:
fixed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-chen commented on pull request #3105: fix: ctx being contaminated due to a new feature of openresty 1.19
Posted by GitBox <gi...@apache.org>.
nic-chen commented on pull request #3105:
URL: https://github.com/apache/apisix/pull/3105#issuecomment-751720995
> @nic-chen please update your source code editor to avoid those issue: [7f00004](https://github.com/apache/apisix/commit/7f000047f74811cd4b8521139e6e51f5401b4a5d)
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #3105: fix: the bug that traffic is forwarded to wrong upstream due to ctx being contaminated
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #3105:
URL: https://github.com/apache/apisix/pull/3105#discussion_r547304087
##########
File path: apisix/init.lua
##########
@@ -334,12 +335,8 @@ end
function _M.http_access_phase()
local ngx_ctx = ngx.ctx
- local api_ctx = ngx_ctx.api_ctx
-
- if not api_ctx then
- api_ctx = core.tablepool.fetch("api_ctx", 0, 32)
- ngx_ctx.api_ctx = api_ctx
- end
+ local api_ctx = core.tablepool.fetch("api_ctx", 0, 32)
+ ngx_ctx.api_ctx = api_ctx
Review comment:
ditto
##########
File path: apisix/init.lua
##########
@@ -185,6 +185,7 @@ function _M.http_ssl_phase()
end
ngx_exit(-1)
end
+ ngx.ctx = nil
Review comment:
need comments
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander merged pull request #3105: fix: ctx being contaminated due to a new feature of openresty 1.19
Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #3105:
URL: https://github.com/apache/apisix/pull/3105
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #3105: fix: ctx being contaminated due to a new feature of openresty 1.19
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #3105:
URL: https://github.com/apache/apisix/pull/3105#issuecomment-750720872
@nic-chen CI failed
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-chen commented on pull request #3105: fix: the bug that traffic is forwarded to wrong upstream due to ctx being contaminated
Posted by GitBox <gi...@apache.org>.
nic-chen commented on pull request #3105:
URL: https://github.com/apache/apisix/pull/3105#issuecomment-749576300
> I don't think this is a good title of PR, and we need test cases
@moonming
we confirmed the changes in the user's env.
maybe we could add test cases later for quick fix. we could track it by #3103
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org