Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3.8] Backport fix for setting cookies #5233

Merged
merged 9 commits into from
Nov 29, 2020
1 change: 1 addition & 0 deletions CHANGES/5233.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix cookies disappearing from HTTPExceptions.
1 change: 1 addition & 0 deletions aiohttp/web_protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,7 @@ async def _handle_request(
resp = Response(
Copy link
Member

Choose a reason for hiding this comment

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

I'm sorry but the fix is incorrect.
This line was backported by the incident.
Since in aiohttp 3.x HttpException is derived from Response we should just use it as-is without the creation of intermediate response object.
I believe this line was incorrectly backported by #4771

I apologize for the regression.

Copy link
Member Author

Choose a reason for hiding this comment

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

No problem, I'll just update this with the reverted code then.

Appears to actually have been this commit, which moved some of the logic into a new _handle_request() method:
29eccad#diff-2126b277e07e3fdd05e7a81da456accf24e5515a46c78c48a79db4eb166043e4L373

Copy link
Member Author

Choose a reason for hiding this comment

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

Apparently, not so easy. Now getting ServerDisconnectedError in the tests.
Will take a look later in the week, unless you beat me to it.

status=exc.status, reason=exc.reason, text=exc.text, headers=exc.headers
)
resp._cookies = exc._cookies
reset = await self.finish_response(request, resp, start_time)
except asyncio.CancelledError:
raise
Expand Down
28 changes: 28 additions & 0 deletions tests/test_web_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,3 +203,31 @@ def test_HTTPException_retains_cause() -> None:
tb = "".join(format_exception(ei.type, ei.value, ei.tb))
assert "CustomException" in tb
assert "direct cause" in tb


async def test_HTTPException_retains_cookie(aiohttp_client):
@web.middleware
async def middleware(request, handler):
try:
return await handler(request)
except web.HTTPException as exc:
exc.set_cookie("foo", request["foo"])
raise exc

async def save(request):
request["foo"] = "works"
raise web.HTTPFound("/show")

async def show(request):
return web.Response(text=request.cookies["foo"])

app = web.Application(middlewares=[middleware])
app.router.add_route("GET", "/save", save)
app.router.add_route("GET", "/show", show)
client = await aiohttp_client(app)

resp = await client.get("/save")
assert resp.status == 200
assert str(resp.url)[-5:] == "/show"
text = await resp.text()
assert text == "works"