Repository navigation
Conversation
resolve() and resolve_async() only cleared the routing context after a response was built, so an exception escaping a route handler left the previous invocation's context in place, including the cached Request. A warm Lambda container could then hand that stale Request to the next invocation. Clear the context in a finally block instead. The ALB response size validation no longer needs its own clean-up on error, and the JWT middleware test now expects the whole context to be gone after a failing handler rather than only the claims.
powertools-for-aws-oss-automation
Bot
added
the
size/M
|
Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need. |
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue number: closes #8538
Summary
Changes
resolve()andresolve_async()only cleared the routing context after the response was built, so when a route raised an unhandled exception the context (including the cached_request) carried over to the next invocation.finallyblock in bothresolve()andresolve_async().except Exception: self.clear_context()in the ALB response size validation, sinceresolve()/resolve_async()now handle it. The existing ALB tests still check the context is empty on each error path.Requestinjection across two failing invocations.test_failed_handler_cannot_leak_claims_into_later_invocationsexpectedapplication_valueto still be in the context after the handler raised. It now expects the context to be empty. I left the JWT middleware's ownfinallythat popsclaimsas it is.User experience
Before: after an unhandled exception, the next invocation on the same warm container could get the previous
Requestfromapp.request,Requestinjection orDepends(), plus any data added withappend_context. The repro in #8538 fails on its second case.After: the context is cleared after every invocation, whether the route returns or raises, as the docs say. The #8538 repro passes.
Tested locally with the full
pytest -m "not perf" --ignore tests/e2e, thetest_with_only_required_packagesandtest_with_auth_required_packagesnox sessions,ruff format --check,ruff check,mypyandty.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.