From 6df39362dff17fa1172074fa838cc2bfa18cb3a8 Mon Sep 17 00:00:00 2001 From: Mahmoud Hashemi Date: Wed, 6 Sep 2017 15:59:34 -0700 Subject: [PATCH 1/4] add failing test mentioned in #11, comment out invalid tests mentioned in #38 --- hyperlink/test/test_url.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/hyperlink/test/test_url.py b/hyperlink/test/test_url.py index 94a30d95..ac081a19 100644 --- a/hyperlink/test/test_url.py +++ b/hyperlink/test/test_url.py @@ -532,10 +532,16 @@ def test_parseEqualSignInParamValue(self): """ u = URL.from_text('http://localhost/?=x=x=x') self.assertEqual(u.get(''), ['x=x=x']) - self.assertEqual(u.to_text(), 'http://localhost/?=x%3Dx%3Dx') + # TODO: see #38 + # self.assertEqual(u.to_text(), 'http://localhost/?=x%3Dx%3Dx') u = URL.from_text('http://localhost/?foo=x=x=x&bar=y') self.assertEqual(u.query, (('foo', 'x=x=x'), ('bar', 'y'))) - self.assertEqual(u.to_text(), 'http://localhost/?foo=x%3Dx%3Dx&bar=y') + # TODO: see #38 + # self.assertEqual(u.to_text(), 'http://localhost/?foo=x%3Dx%3Dx&bar=y') + + u = URL.from_text('https://example.com/?argument=3&argument=4&operator=%3D') + iri = u.to_iri() + self.assertEqual(iri.get('operator'), ['=']) def test_empty(self): """ From 83d5cb27c4886a7e573e7d08a3b894494b527c5a Mon Sep 17 00:00:00 2001 From: Mahmoud Hashemi Date: Wed, 6 Sep 2017 16:00:27 -0700 Subject: [PATCH 2/4] split encoding/decoding for URL query parameter keys and values. the equals sign (=) is a reserved character for keys, but can appear unescaped in values. see #38 for details. --- hyperlink/_url.py | 62 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 43 insertions(+), 19 deletions(-) diff --git a/hyperlink/_url.py b/hyperlink/_url.py index de8aca06..37c57663 100644 --- a/hyperlink/_url.py +++ b/hyperlink/_url.py @@ -156,8 +156,10 @@ def __nonzero__(self): _SCHEMELESS_PATH_DELIMS = _ALL_DELIMS - _SCHEMELESS_PATH_SAFE _FRAGMENT_SAFE = _UNRESERVED_CHARS | _PATH_SAFE | set(u'/?') _FRAGMENT_DELIMS = _ALL_DELIMS - _FRAGMENT_SAFE -_QUERY_SAFE = _UNRESERVED_CHARS | _FRAGMENT_SAFE - set(u'&=+') -_QUERY_DELIMS = _ALL_DELIMS - _QUERY_SAFE +_QUERY_VALUE_SAFE = _UNRESERVED_CHARS | _FRAGMENT_SAFE - set(u'&+') +_QUERY_VALUE_DELIMS = _ALL_DELIMS - _QUERY_VALUE_SAFE +_QUERY_KEY_SAFE = _UNRESERVED_CHARS | _QUERY_VALUE_SAFE - set(u'=') +_QUERY_KEY_DELIMS = _ALL_DELIMS - _QUERY_KEY_SAFE def _make_decode_map(delims, allow_percent=False): @@ -191,8 +193,10 @@ def _make_quote_map(safe_chars): _PATH_PART_QUOTE_MAP = _make_quote_map(_PATH_SAFE) _SCHEMELESS_PATH_PART_QUOTE_MAP = _make_quote_map(_SCHEMELESS_PATH_SAFE) _PATH_DECODE_MAP = _make_decode_map(_PATH_DELIMS) -_QUERY_PART_QUOTE_MAP = _make_quote_map(_QUERY_SAFE) -_QUERY_DECODE_MAP = _make_decode_map(_QUERY_DELIMS) +_QUERY_KEY_QUOTE_MAP = _make_quote_map(_QUERY_KEY_SAFE) +_QUERY_KEY_DECODE_MAP = _make_decode_map(_QUERY_KEY_DELIMS) +_QUERY_VALUE_QUOTE_MAP = _make_quote_map(_QUERY_VALUE_SAFE) +_QUERY_VALUE_DECODE_MAP = _make_decode_map(_QUERY_VALUE_DELIMS) _FRAGMENT_QUOTE_MAP = _make_quote_map(_FRAGMENT_SAFE) _FRAGMENT_DECODE_MAP = _make_decode_map(_FRAGMENT_DELIMS) _UNRESERVED_DECODE_MAP = dict([(k, v) for k, v in _HEX_CHAR_MAP.items() @@ -263,17 +267,28 @@ def _encode_path_parts(text_parts, rooted=False, has_scheme=True, return tuple(encoded_parts) -def _encode_query_part(text, maximal=True): +def _encode_query_key(text, maximal=True): """ Percent-encode a single query string key or value. """ if maximal: bytestr = normalize('NFC', text).encode('utf8') - return u''.join([_QUERY_PART_QUOTE_MAP[b] for b in bytestr]) - return u''.join([_QUERY_PART_QUOTE_MAP[t] if t in _QUERY_DELIMS else t + return u''.join([_QUERY_KEY_QUOTE_MAP[b] for b in bytestr]) + return u''.join([_QUERY_KEY_QUOTE_MAP[t] if t in _QUERY_KEY_DELIMS else t for t in text]) +def _encode_query_value(text, maximal=True): + """ + Percent-encode a single query string key or value. + """ + if maximal: + bytestr = normalize('NFC', text).encode('utf8') + return u''.join([_QUERY_VALUE_QUOTE_MAP[b] for b in bytestr]) + return u''.join([_QUERY_VALUE_QUOTE_MAP[t] + if t in _QUERY_VALUE_DELIMS else t for t in text]) + + def _encode_fragment_part(text, maximal=True): """Quote the fragment part of the URL. Fragments don't have subdelimiters, so the whole URL fragment can be passed. @@ -455,9 +470,14 @@ def _decode_path_part(text, normalize_case=False): _decode_map=_PATH_DECODE_MAP) -def _decode_query_part(text, normalize_case=False): +def _decode_query_key(text, normalize_case=False): return _percent_decode(text, normalize_case=normalize_case, - _decode_map=_QUERY_DECODE_MAP) + _decode_map=_QUERY_KEY_DECODE_MAP) + + +def _decode_query_value(text, normalize_case=False): + return _percent_decode(text, normalize_case=normalize_case, + _decode_map=_QUERY_VALUE_DECODE_MAP) def _decode_fragment_part(text, normalize_case=False): @@ -1199,9 +1219,9 @@ def to_uri(self): userinfo=new_userinfo, host=self.host.encode("idna").decode("ascii"), path=new_path, - query=tuple([tuple(_encode_query_part(x, maximal=True) - if x is not None else None - for x in (k, v)) + query=tuple([(_encode_query_key(k, maximal=True), + _encode_query_value(v, maximal=True) + if v is not None else None) for k, v in self.query]), fragment=_encode_fragment_part(self.fragment, maximal=True) ) @@ -1245,9 +1265,9 @@ def to_iri(self): host=textHost, path=[_decode_path_part(segment) for segment in self.path], - query=[tuple(_decode_query_part(x) - if x is not None else None - for x in (k, v)) + query=[(_decode_query_key(k), + _decode_query_value(v) + if v is not None else None) for k, v in self.query], fragment=_decode_fragment_part(self.fragment)) @@ -1283,10 +1303,14 @@ def to_text(self, with_password=False): has_scheme=bool(scheme), has_authority=bool(authority), maximal=False) - query_string = u'&'.join( - u'='.join((_encode_query_part(x, maximal=False) - for x in ([k] if v is None else [k, v]))) - for (k, v) in self.query) + query_parts = [] + for k, v in self.query: + if v is None: + query_parts.append(_encode_query_key(k, maximal=False)) + else: + query_parts.append(u'='.join((_encode_query_key(k, maximal=False), + _encode_query_value(v, maximal=False)))) + query_string = u'&'.join(query_parts) fragment = self.fragment From a7283b6df0479610bd679cbc906b325ff584b502 Mon Sep 17 00:00:00 2001 From: Mahmoud Hashemi Date: Wed, 6 Sep 2017 16:03:16 -0700 Subject: [PATCH 3/4] add test to check that equals signs are not unnecessarily escaped in URIs --- hyperlink/test/test_url.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hyperlink/test/test_url.py b/hyperlink/test/test_url.py index ac081a19..d52ab1bb 100644 --- a/hyperlink/test/test_url.py +++ b/hyperlink/test/test_url.py @@ -542,6 +542,8 @@ def test_parseEqualSignInParamValue(self): u = URL.from_text('https://example.com/?argument=3&argument=4&operator=%3D') iri = u.to_iri() self.assertEqual(iri.get('operator'), ['=']) + # assert that the equals is not unnecessarily escaped + self.assertEqual(iri.to_uri().get('operator'), ['=']) def test_empty(self): """ From ff248858c2b69451f4ad28e76ed76ac2042be709 Mon Sep 17 00:00:00 2001 From: Mahmoud Hashemi Date: Tue, 26 Mar 2019 09:41:55 -0700 Subject: [PATCH 4/4] update tests to reflect new unescaped-equal-signs-in-query-param-values regime (see #38). --- .tox-coveragerc | 2 ++ hyperlink/test/test_url.py | 6 ++---- tox.ini | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.tox-coveragerc b/.tox-coveragerc index 9e92524c..44178a43 100644 --- a/.tox-coveragerc +++ b/.tox-coveragerc @@ -3,6 +3,8 @@ branch = True source = hyperlink ../hyperlink +omit = + */flycheck_* [paths] source = diff --git a/hyperlink/test/test_url.py b/hyperlink/test/test_url.py index 3f653e1a..4585d5b9 100644 --- a/hyperlink/test/test_url.py +++ b/hyperlink/test/test_url.py @@ -542,12 +542,10 @@ def test_parseEqualSignInParamValue(self): """ u = URL.from_text('http://localhost/?=x=x=x') self.assertEqual(u.get(''), ['x=x=x']) - # TODO: see #38 - # self.assertEqual(u.to_text(), 'http://localhost/?=x%3Dx%3Dx') + self.assertEqual(u.to_text(), 'http://localhost/?=x=x=x') u = URL.from_text('http://localhost/?foo=x=x=x&bar=y') self.assertEqual(u.query, (('foo', 'x=x=x'), ('bar', 'y'))) - # TODO: see #38 - # self.assertEqual(u.to_text(), 'http://localhost/?foo=x%3Dx%3Dx&bar=y') + self.assertEqual(u.to_text(), 'http://localhost/?foo=x=x=x&bar=y') u = URL.from_text('https://example.com/?argument=3&argument=4&operator=%3D') iri = u.to_iri() diff --git a/tox.ini b/tox.ini index ab995a87..ef2ec9c5 100644 --- a/tox.ini +++ b/tox.ini @@ -4,7 +4,7 @@ envlist = py26,py27,py34,py35,py36,pypy,coverage-report,packaging [testenv] changedir = .tox deps = -rrequirements-test.txt -commands = coverage run --parallel --omit 'flycheck__*' --rcfile {toxinidir}/.tox-coveragerc -m pytest --doctest-modules {envsitepackagesdir}/hyperlink {posargs} +commands = coverage run --parallel --rcfile {toxinidir}/.tox-coveragerc -m pytest --doctest-modules {envsitepackagesdir}/hyperlink {posargs} # Uses default basepython otherwise reporting doesn't work on Travis where # Python 3.6 is only available in 3.6 jobs.