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

TestResponse needs to handle multiple set-cookie headers #799

Closed
simonw opened this issue Jun 5, 2020 · 2 comments
Closed

TestResponse needs to handle multiple set-cookie headers #799

simonw opened this issue Jun 5, 2020 · 2 comments
Labels

Comments

@simonw
Copy link
Owner

simonw commented Jun 5, 2020

Seeing this test failure on #798:

_______________________ test_auth_token _______________________
app_client = <tests.fixtures.TestClient object at 0x11285c910>
    def test_auth_token(app_client):
        "The /-/auth-token endpoint sets the correct cookie"
        assert app_client.ds._root_token is not None
        path = "/-/auth-token?token={}".format(app_client.ds._root_token)
        response = app_client.get(path, allow_redirects=False,)
        assert 302 == response.status
        assert "/" == response.headers["Location"]
>       assert {"id": "root"} == app_client.ds.unsign(response.cookies["ds_actor"], "actor")
E       KeyError: 'ds_actor'
datasette/tests/test_auth.py:12: KeyError

It looks like that's happening because the ASGI middleware is adding another set-cookie header - but those two set-cookie headers are combined into one when the TestResponse is constructed:

datasette/tests/fixtures.py

Lines 113 to 127 in 0c064c5

assert start["type"] == "http.response.start"
headers = dict(
[(k.decode("utf8"), v.decode("utf8")) for k, v in start["headers"]]
)
status = start["status"]
# Now loop until we run out of response.body
body = b""
while True:
message = await instance.receive_output(2)
messages.append(message)
assert message["type"] == "http.response.body"
body += message["body"]
if not message.get("more_body"):
break
response = TestResponse(status, headers, body)

@simonw simonw added the tests label Jun 5, 2020
@simonw
Copy link
Owner Author

simonw commented Jun 5, 2020

This really needs a MultiValueDict ala Django: https://github.com/django/django/blob/24b82cd201e21060fbc02117dc16d1702877a1f3/django/utils/datastructures.py#L42

Turns out I have one of these in Datasette already - RequestParameters from 81be313

The name isn't quite right though.

@simonw
Copy link
Owner Author

simonw commented Jun 5, 2020

I'm going to rename that MultiParams and use it in both places.

@simonw simonw closed this as completed Jun 5, 2020
simonw added a commit that referenced this issue Jun 5, 2020
Closes #793.

* Rename RequestParameters to MultiParams, refs #799
* Allow tuples as well as lists in MultiParams, refs #799
* Use csrftokens when running tests, refs #799
* Use new csrftoken() function, refs simonw/asgi-csrf#7
* Check for Vary: Cookie hedaer, refs simonw/asgi-csrf#8
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

1 participant