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: test non-UTF-8 bytes #26170

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Cookies: test non-UTF-8 bytes #26170

wants to merge 2 commits into from

Conversation

annevk
Copy link
Member

@annevk annevk commented Oct 19, 2020

For httpwg/http-extensions#1073.

I could not find a way to test this using the existing infrastructure (note that document.cookie does not support these kind of values). That might be another thing you want to look at @miketaylr? This approach isn't exactly great, but I wanted to at least capture this nuance somewhere.

response.headers.append(b"Set-Cookie", b"cookiesarebananas=\xFFtest\xFF")
response.content = "set"
elif "get" in request.GET:
response.content = urllib.quote(request.headers["Cookie"])
Copy link
Member

Choose a reason for hiding this comment

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

This will throw in Python 3, since quote lives in urllib.parse now.

I don't even know what the plans are for Python3 and tests, so this may be a non-issue if there is no plan, but something to consider:

try: 
    from urllib.parse import quote
except ImportError:
    from urllib import quote

And then calling quote directly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please use from six.moves.urllib.parse import quote :)

EDIT: And yes, new python file handlers should be py3 compatible please (we understand its a bit of a pain since the CI isn't py3 yet). High-level plan is for WPT to go Py3-first in the next few months, and Py3-only sometime around Feb 2021. (But this hasn't been ratified via RFC yet).

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, I think I'll hold off on merging this pending #26190.

Copy link
Member

Choose a reason for hiding this comment

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

@annevk I think it's safe to merge now.

Copy link
Member Author

Choose a reason for hiding this comment

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

I hadn't made it P3-compatible yet. 😊

I have now, but it also seems like this hasn't been decided by the IETF WG yet.

@foolip
Copy link
Member

foolip commented Mar 25, 2021

The wpt-chrome-dev-* CI failures here were likely due to #19360. A workaround was landed in #28238, so I'm closing and reopening to re-trigger the CI checks.

@foolip foolip closed this Mar 25, 2021
@foolip foolip reopened this Mar 25, 2021
johannhof added a commit to johannhof/wpt that referenced this pull request Jan 31, 2022
This is for httpwg/http-extensions#1707

I took some inspiration from web-platform-tests#26170 and used a python server instead of
a header file in order to better control which bytes are served as part
of the header.

The test runs exclusively on the élève. subdomain through opening a new
window that then runs all test and reports back to the main test window.
annevk pushed a commit that referenced this pull request Feb 7, 2022
This is for httpwg/http-extensions#1707.

I took some inspiration from #26170 and used a python server instead of a header file in order to better control which bytes are served as part of the header.

The test runs exclusively on the élève. subdomain through opening a new window that then runs all test and reports back to the main test window.
@annevk annevk closed this Feb 7, 2022
@annevk annevk reopened this Feb 7, 2022
mattwoodrow pushed a commit to mattwoodrow/wpt that referenced this pull request Feb 15, 2022
This is for httpwg/http-extensions#1707.

I took some inspiration from web-platform-tests#26170 and used a python server instead of a header file in order to better control which bytes are served as part of the header.

The test runs exclusively on the élève. subdomain through opening a new window that then runs all test and reports back to the main test window.
DanielRyanSmith pushed a commit to DanielRyanSmith/wpt that referenced this pull request Feb 28, 2022
This is for httpwg/http-extensions#1707.

I took some inspiration from web-platform-tests#26170 and used a python server instead of a header file in order to better control which bytes are served as part of the header.

The test runs exclusively on the élève. subdomain through opening a new window that then runs all test and reports back to the main test window.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants