Python 3: port tests in cookie-store #24531

ziransun · 2020-07-09T13:07:51Z

No description provided.

cookie-store/resources/cookie_helper.py

ziransun · 2020-07-10T08:48:32Z

@Hexcles : I actually have some UnicodeEncodeErrors while running in Python 3 (happening before and after code updates with review comments ). For example, running change_eventhandler_for_http_cookie_and_set_cookie_headers.tentative.https.window.html throws

File "/home/ziran/WPT/wpt-commits/wpt3/cookie-store/resources/cookie_helper.py", line 46, in main
args = parse_qs(qd, keep_blank_values = True)
File "/usr/local/lib/python3.8/urllib/parse.py", line 678, in parse_qs
pairs = parse_qsl(qs, keep_blank_values, strict_parsing,
File "/usr/local/lib/python3.8/urllib/parse.py", line 745, in parse_qsl
value = _coerce_result(value)
File "/usr/local/lib/python3.8/urllib/parse.py", line 104, in _encode_result
return obj.encode(encoding, errors)
UnicodeEncodeError: 'ascii' codec can't encode character '\U0001f36a' in position 5: ordinal not in range(128)

and

...
UnicodeEncodeError: 'ascii' codec can't encode character '\ufffd' in position 17: ordinal not in range(128)

I couldn't see anything obvious at the moment. Any suggestions?

Hexcles · 2020-07-10T13:57:44Z

That's due to the change in behaviours of parse_qs, which now does decoding in Python 3: https://docs.python.org/3/library/urllib.parse.html#urllib.parse.parse_qs (see the encoding parameter).

You'd have to do something like:

if PY3:
    parse_qs(..., encoding='iso-8859-1')

(sigh)

ziransun · 2020-07-10T16:50:35Z

That's due to the change in behaviours of parse_qs, which now does decoding in Python 3: https://docs.python.org/3/library/urllib.parse.html#urllib.parse.parse_qs (see the encoding parameter).

You'd have to do something like:
if PY3:
    parse_qs(..., encoding='iso-8859-1')
(sigh)

The complaints have changed to :
UnicodeEncodeError: 'ascii' codec can't encode characters in position 5-8: ordinal not in range(128)
and
UnicodeEncodeError: 'ascii' codec can't encode characters in position 17-19: ordinal not in range(128)

I will try to dig a bit more on this. Meanwhile, if you see anything obvious, let me know. Thank you!

Hexcles · 2020-07-10T17:10:02Z

Maybe paste the full traceback and try to print out the problematic string.

ziransun · 2020-07-10T18:59:45Z

Maybe paste the full traceback and try to print out the problematic string.

0:02.28 INFO STDOUT: printing qd:
 0:02.28 INFO STDOUT: b'&set-cookie=HTTP-%F0%9F%8D%AA%3D%F0%9F%94%B5%3B%20path%3D%2F'
 0:02.28 WARNING Traceback (most recent call last):
  File "/home/ziran/WPT/wpt-commits/wpt3/tools/wptserve/wptserve/handlers.py", line 332, in __call__
    rv = self.func(request, response)
  File "/home/ziran/WPT/wpt-commits/wpt3/cookie-store/resources/cookie_helper.py", line 51, in main
    args = parse_qs(qd, keep_blank_values = True, encoding='iso-8859-1')
  File "/usr/local/lib/python3.8/urllib/parse.py", line 678, in parse_qs
    pairs = parse_qsl(qs, keep_blank_values, strict_parsing,
  File "/usr/local/lib/python3.8/urllib/parse.py", line 745, in parse_qsl
    value = _coerce_result(value)
  File "/usr/local/lib/python3.8/urllib/parse.py", line 104, in _encode_result
    return obj.encode(encoding, errors)
UnicodeEncodeError: 'ascii' codec can't encode characters in position 5-8: ordinal not in range(128)

 0:02.42 INFO STDOUT: printing qd:
 0:02.42 INFO STDOUT: b'charset=iso-8859-1&set-cookie=HTTP-cookie%3Dvalue%EF%BF%BD%3B%20path%3D%2F'
 0:02.42 WARNING Traceback (most recent call last):
  File "/home/ziran/WPT/wpt-commits/wpt3/tools/wptserve/wptserve/handlers.py", line 332, in __call__
    rv = self.func(request, response)
  File "/home/ziran/WPT/wpt-commits/wpt3/cookie-store/resources/cookie_helper.py", line 51, in main
    args = parse_qs(qd, keep_blank_values = True, encoding='iso-8859-1')
  File "/usr/local/lib/python3.8/urllib/parse.py", line 678, in parse_qs
    pairs = parse_qsl(qs, keep_blank_values, strict_parsing,
  File "/usr/local/lib/python3.8/urllib/parse.py", line 745, in parse_qsl
    value = _coerce_result(value)
  File "/usr/local/lib/python3.8/urllib/parse.py", line 104, in _encode_result
    return obj.encode(encoding, errors)
UnicodeEncodeError: 'ascii' codec can't encode characters in position 17-19: ordinal not in range(128)

And more here -

FAIL CookieStore agreed with HTTP headers agree on encoding non-ASCII cookies - assert_equals: CGI should have succeeded in setCookieStringHttp set-cookie: HTTP-🍪=🔵; path=/
{"error": {"code": 500, "message": "Traceback (most recent call last):\n  File \"/home/ziran/WPT/wpt-commits/wpt3/tools/wptserve/wptserve/handlers.py\", line 332, in __call__\n    rv = self.func(request, response)\n  File \"/home/ziran/WPT/wpt-commits/wpt3/cookie-store/resources/cookie_helper.py\", line 51, in main\n    args = parse_qs(qd, keep_blank_values = True, encoding='iso-8859-1')\n  File \"/usr/local/lib/python3.8/urllib/parse.py\", line 678, in parse_qs\n    pairs = parse_qsl(qs, keep_blank_values, strict_parsing,\n  File \"/usr/local/lib/python3.8/urllib/parse.py\", line 745, in parse_qsl\n    value = _coerce_result(value)\n  File \"/usr/local/lib/python3.8/urllib/parse.py\", line 104, in _encode_result\n    return obj.encode(encoding, errors)\nUnicodeEncodeError: 'ascii' codec can't encode characters in position 5-8: ordinal not in range(128)\n"}} expected true but got false
    at setCookieStringHttp (https://web-platform.test:8443/cookie-store/resources/cookie-test-helpers.js:108:3)
    at async https://web-platform.test:8443/cookie-store/change_eventhandler_for_http_cookie_and_set_cookie_headers.tentative.https.window.js:53:3
    at async Test.<anonymous> (https://web-platform.test:8443/cookie-store/resources/cookie-test-helpers.js:221:14)
FAIL Binary HTTP set/overwrite/delete observed in CookieStore - assert_equals: CGI should have succeeded in setCookieBinaryHttp set-cookie: HTTP-cookie=valueï¿½; path=/
{"error": {"code": 500, "message": "Traceback (most recent call last):\n  File \"/home/ziran/WPT/wpt-commits/wpt3/tools/wptserve/wptserve/handlers.py\", line 332, in __call__\n    rv = self.func(request, response)\n  File \"/home/ziran/WPT/wpt-commits/wpt3/cookie-store/resources/cookie_helper.py\", line 51, in main\n    args = parse_qs(qd, keep_blank_values = True, encoding='iso-8859-1')\n  File \"/usr/local/lib/python3.8/urllib/parse.py\", line 678, in parse_qs\n    pairs = parse_qsl(qs, keep_blank_values, strict_parsing,\n  File \"/usr/local/lib/python3.8/urllib/parse.py\", line 745, in parse_qsl\n    value = _coerce_result(value)\n  File \"/usr/local/lib/python3.8/urllib/parse.py\", line 104, in _encode_result\n    return obj.encode(encoding, errors)\nUnicodeEncodeError: 'ascii' codec can't encode characters in position 17-19: ordinal not in range(128)\n"}} expected true but got false
    at setCookieBinaryHttp (https://web-platform.test:8443/cookie-store/resources/cookie-test-helpers.js:140:3)
    at async https://web-platform.test:8443/cookie-store/change_eventhandler_for_http_cookie_and_set_cookie_headers.tentative.https.window.js:141:3
    at async Test.<anonymous> (https://web-platform.test:8443/cookie-store/resources/cookie-test-helpers.js:221:14)

ziransun · 2020-07-13T13:32:08Z

@Hexcles : Looking into python3.8/urllib/parse.py, obj.encode(encoding, errors) , a helpers for bytes handling, uses _implicit_encoding = 'ascii'.

I've updated code and decode qd before passing to parse_qs(). Tests passed for both Python 2 and python 3 in my local runs.

Do let me know if I have missed anything. Thank you!

inexorabletash

lgtm, but maybe some refactoring to pull the if PY3 logic out.

cookie-store/resources/cookie_helper.py

Hexcles

Thanks for the detailed investigation. Python 3 really does not like URLs in bytes, and uses ASCII as the implicit encoding unconditionally to coerce the results: https://github.com/python/cpython/blob/b4cd77de05e5bbaa6a4be90f710b787e0790c36f/Lib/urllib/parse.py#L90-L97

@ziransun could you try keeping request.url as text strings (use isomorphic_decode on L45 and remove the PY3-only decode on L49)?

cookie-store/resources/cookie_helper.py

ziransun · 2020-07-15T10:38:15Z

@ziransun could you try keeping request.url as text strings (use isomorphic_decode on L45 and remove the PY3-only decode on L49)?

In this case, in python 2 we are passing a unicode type string to quote() function. I guess it should be string type? A Python 2 run with this change threw KeyError, e.g.

File .../cookie-store/resources/cookie_helper.py", line 46, in quote_str
    return quote(cookie_str, '')
  File "/usr/lib/python2.7/urllib.py", line 1304, in quote
    return ''.join(map(quoter, s))
KeyError: u'\xef'

ziransun · 2020-07-21T15:43:28Z

@Hexcles Any further comments on this?

Hexcles

Sorry for the delay. This one slipped through...

cookie-store/resources/cookie_helper.py

Hexcles · 2020-07-22T16:05:11Z

cookie-store/resources/cookie_helper.py

    for set_cookie in args.get('set-cookie', []):
      if '=' in set_cookie.split(';', 1)[0]:
        name, rest = set_cookie.split('=', 1)


Missing prefixes here?

Prefixes were deliberately left out here. I think set_cookie here is a native string here. Is it okay?

Ahh right, because it's from parse_qs...

Unfortunately, this will be problematic if/when we start to enforce prefixes for all string literals. I think we'd need to add some logic to both quote_str and parse_qs_str to always return binary strings (add isomorphic_encode in the PY3 branch).

Thank @Hexcles! Code updated. Please review.

Hexcles · 2020-07-22T16:07:12Z

cookie-store/resources/cookie_helper.py

  body = '\r\n'.join(body)
-  headers.append(('content-length', str(len(body))))
+  headers.append((b'content-length', len(body)))
  return 200, headers, body


Looks like body is a native string.

Code updated.

cookie-store/resources/cookie_helper.py

Hexcles

LGTM. Thanks for bearing with me through this!

cookie-store/resources/cookie_helper.py

wpt-pr-bot added the cookie-store label Jul 9, 2020

wpt-pr-bot assigned pwnall Jul 9, 2020

wpt-pr-bot requested review from inexorabletash and pwnall July 9, 2020 13:08

ziransun added the python3 label Jul 9, 2020

ziransun assigned Hexcles and unassigned pwnall Jul 9, 2020

Hexcles reviewed Jul 9, 2020

View reviewed changes

inexorabletash reviewed Jul 14, 2020

View reviewed changes

cookie-store/resources/cookie_helper.py Outdated Show resolved Hide resolved

Hexcles reviewed Jul 14, 2020

View reviewed changes

cookie-store/resources/cookie_helper.py Outdated Show resolved Hide resolved

ziransun closed this Jul 21, 2020

ziransun reopened this Jul 21, 2020

Hexcles reviewed Jul 22, 2020

View reviewed changes

ziransun added 8 commits July 28, 2020 10:51

Python 3: port tests in cookie-store

d51b147

use byte ranges

fe51612

Use own string types in Python2 and Python 3

bf7c426

Remove whitespace

a49fa02

"set_cookie" should bytes in header

2a16d5f

wrap functions

b9378b8

Remove unnecessary checking

10a7255

Enforce prefix

283b115

ziransun force-pushed the cookie-store branch from 43f44a6 to 283b115 Compare July 28, 2020 14:01

Hexcles reviewed Jul 28, 2020

View reviewed changes

cookie-store/resources/cookie_helper.py Show resolved Hide resolved

Update cookie-store/resources/cookie_helper.py

2a6acee

Hexcles approved these changes Jul 28, 2020

View reviewed changes

Hexcles reviewed Jul 28, 2020

View reviewed changes

cookie-store/resources/cookie_helper.py Outdated Show resolved Hide resolved

Revert incorrect suggestion

517b72a

Hexcles merged commit 79d185a into web-platform-tests:master Jul 28, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Python 3: port tests in cookie-store #24531

Python 3: port tests in cookie-store #24531

ziransun commented Jul 9, 2020

ziransun commented Jul 10, 2020

Hexcles commented Jul 10, 2020

ziransun commented Jul 10, 2020

Hexcles commented Jul 10, 2020

ziransun commented Jul 10, 2020 •

edited

ziransun commented Jul 13, 2020

inexorabletash left a comment

Hexcles left a comment

ziransun commented Jul 15, 2020

ziransun commented Jul 21, 2020

Hexcles left a comment

Hexcles Jul 22, 2020

ziransun Jul 23, 2020

Hexcles Jul 27, 2020

ziransun Jul 28, 2020

Hexcles Jul 22, 2020

ziransun Jul 23, 2020

ziransun Jul 28, 2020

Hexcles left a comment

Python 3: port tests in cookie-store #24531

Python 3: port tests in cookie-store #24531

Conversation

ziransun commented Jul 9, 2020

ziransun commented Jul 10, 2020

Hexcles commented Jul 10, 2020

ziransun commented Jul 10, 2020

Hexcles commented Jul 10, 2020

ziransun commented Jul 10, 2020 • edited

ziransun commented Jul 13, 2020

inexorabletash left a comment

Choose a reason for hiding this comment

Hexcles left a comment

Choose a reason for hiding this comment

ziransun commented Jul 15, 2020

ziransun commented Jul 21, 2020

Hexcles left a comment

Choose a reason for hiding this comment

Hexcles Jul 22, 2020

Choose a reason for hiding this comment

ziransun Jul 23, 2020

Choose a reason for hiding this comment

Hexcles Jul 27, 2020

Choose a reason for hiding this comment

ziransun Jul 28, 2020

Choose a reason for hiding this comment

Hexcles Jul 22, 2020

Choose a reason for hiding this comment

ziransun Jul 23, 2020

Choose a reason for hiding this comment

ziransun Jul 28, 2020

Choose a reason for hiding this comment

Hexcles left a comment

Choose a reason for hiding this comment

ziransun commented Jul 10, 2020 •

edited