Skip to content

fix: ctx being contaminated due to a new feature of openresty 1.19#3105

Merged
spacewander merged 4 commits into
apache:masterfrom
nic-chen:ctx-reused
Dec 24, 2020
Merged

fix: ctx being contaminated due to a new feature of openresty 1.19#3105
spacewander merged 4 commits into
apache:masterfrom
nic-chen:ctx-reused

Conversation

@nic-chen

Copy link
Copy Markdown
Member

What this PR does / why we need it:

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.

fix #3079

Pre-submission checklist:

  • 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 test: add TLS testcase in apisix #3103
  • Have you modified the corresponding document?
    we don't need to modify docs for this change.
  • Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

@nic-chen
nic-chen requested a review from membphis December 22, 2020 13:14

@moonming moonming left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is a good title of PR, and we need test cases

@membphis

Copy link
Copy Markdown
Member

related issue: #3103

Comment thread apisix/init.lua
end
ngx_exit(-1)
end
ngx.ctx = nil

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need comments

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed.

Comment thread apisix/init.lua
ngx_ctx.api_ctx = api_ctx
end
local api_ctx = core.tablepool.fetch("api_ctx", 0, 32)
ngx_ctx.api_ctx = api_ctx

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed.

@nic-chen

Copy link
Copy Markdown
Member Author

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

@nic-chen nic-chen changed the title fix: the bug that traffic is forwarded to wrong upstream due to ctx being contaminated fix: ctx being contaminated due to a new feature of openresty 1.19 Dec 22, 2020
Comment thread apisix/init.lua
@membphis

Copy link
Copy Markdown
Member

@nic-chen CI failed

@spacewander
spacewander merged commit 5de9990 into apache:master Dec 24, 2020
@membphis

Copy link
Copy Markdown
Member

@nic-chen please update your source code editor to avoid those issue: 7f00004

@nic-chen

Copy link
Copy Markdown
Member Author

@nic-chen please update your source code editor to avoid those issue: 7f00004

OK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

apisix转发流量错误

6 participants