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

Cookies aren't being set when sending a HTTP 302. #5181

Closed
alanwguo opened this issue Oct 30, 2020 · 10 comments
Closed

Cookies aren't being set when sending a HTTP 302. #5181

alanwguo opened this issue Oct 30, 2020 · 10 comments
Labels

Comments

@alanwguo
Copy link

🐞 Describe the bug

This a repeat of the issue described here:
#3700

When setting a cookie and responding with an HTTP redirect, the Set-Cookie header is not set in the response.

💡 To Reproduce

  1. Define endpoint like below:
async def index(request: web.Request) -> web.Response:
    resp = web.HTTPFound("/")
    resp.set_cookie("anyscale-token", token)
    raise resp
  1. Call endpoint, e. Instead, no Set-Cookie header is in the response.

💡 Expected behavior

expect to see Set-Cookie header set

📋 Your version of the Python

$ python --version
Python 3.7.7

📋 Your version of the aiohttp/yarl/multidict distributions

$ python -m pip show aiohttp
Name: aiohttp
Version: 3.7.2
Summary: Async http client/server framework (asyncio)
Home-page: https://github.com/aio-libs/aiohttp
Author: Nikolay Kim
Author-email: [email protected]
License: Apache 2
Location: /root/anaconda3/lib/python3.7/site-packages
Requires: attrs, typing-extensions, async-timeout, yarl, chardet, multidict
Required-by: ray, google-auth, anyscale, aiohttp-cors
$ python -m pip show multidict
Name: multidict
Version: 4.7.6
Summary: multidict implementation
Home-page: https://github.com/aio-libs/multidict
Author: Andrew Svetlov
Author-email: [email protected]
License: Apache 2
Location: /root/anaconda3/lib/python3.7/site-packages
Requires: 
Required-by: yarl, aiohttp
$ python -m pip show yarl
Name: yarl
Version: 1.5.1
Summary: Yet another URL library
Home-page: https://github.com/aio-libs/yarl/
Author: Andrew Svetlov
Author-email: [email protected]
License: Apache 2
Location: /root/anaconda3/lib/python3.7/site-packages
Requires: idna, typing-extensions, multidict
Required-by: aiohttp

📋 Additional context

@alanwguo alanwguo added the bug label Oct 30, 2020
@derlih
Copy link
Contributor

derlih commented Oct 31, 2020

@alanwguo I think you shouldn't raise the response. You should return it.
Please try

async def index(request: web.Request) -> web.Response:
    resp = web.HTTPFound("/")
    resp.set_cookie("anyscale-token", token)
    return resp

@webknjaz
Copy link
Member

Raising should also work, like in other frameworks.

@derlih
Copy link
Contributor

derlih commented Nov 1, 2020

I've written the test to reproduce this issue:

async def test_httpfound_cookies_302(aiohttp_client):
    async def handler(_):
        resp = web.HTTPFound("/")
        resp.set_cookie("my-cookie", "cookie-value")
        return resp

    app = web.Application()
    app.router.add_get("/", handler)
    client = await aiohttp_client(app)
    resp = await client.get("/")
    assert "my-cookie" in resp.cookies

And I've got the following error:

Traceback (most recent call last):
  File "/home/dmitry/Sources/aiohttp/aiohttp/web_protocol.py", line 456, in _handle_request
    resp = await self._request_handler(request)
  File "/home/dmitry/Sources/aiohttp/aiohttp/web_app.py", line 348, in _handle
    resp = await handler(request)
  File "/home/dmitry/Sources/aiohttp/tests/test_web_functional.py", line 1917, in handler
    resp.set_cookie("my-cookie", "cookie-value")
AttributeError: 'HTTPFound' object has no attribute 'set_cookie'

@derlih
Copy link
Contributor

derlih commented Nov 1, 2020

Looks like related to #4277

@webknjaz
Copy link
Member

webknjaz commented Nov 1, 2020

It's an exception, not a response object, why would you expect response methods on it to exist?

@webknjaz
Copy link
Member

webknjaz commented Nov 1, 2020

Maybe you are right about #4277

@derlih
Copy link
Contributor

derlih commented Nov 14, 2020

I wasn't. #4277 is only for 4.0
May be #5233 will fix this issue.

@Dreamsorcerer
Copy link
Member

If it works correctly for you in aiohttp <3.7, then I'm pretty sure my fix will resolve the issue for you in 3.7 once merged.

@derlih
Copy link
Contributor

derlih commented Dec 22, 2020

@Dreamsorcerer as far as I understood your PR is merged to 3.8, but not to 3.7.
@asvetlov do you have any plans/date on releasing 3.8?

@Dreamsorcerer
Copy link
Member

Yes, the merge target got changed, so 3.8 now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants