Repository navigation
Encode query keys and values before building request URLs - #514
Kayvan-Zahiri wants to merge 1 commit into
Conversation
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request replaces manual query string construction with urllib.parse.urlencode in Adyen/util.py and introduces a new test suite in test/QueryEncodingTest.py. The review feedback recommends enhancing the query encoding by utilizing quote_via=quote to ensure spaces are encoded as %20 (for RFC 3986 compliance) instead of +, and enabling doseq=True to correctly handle list or sequence parameters. Additionally, it is suggested to update the test suite to verify these encoding behaviors.
| import copy | ||
| import hashlib | ||
| import hmac | ||
| from urllib.parse import urlencode |
There was a problem hiding this comment.
Import quote from urllib.parse to support encoding spaces as %20 instead of + in query parameters, ensuring compliance with RFC 3986.
| from urllib.parse import urlencode | |
| from urllib.parse import quote, urlencode |
|
|
||
| def get_query(query_parameters): | ||
| return "?" + "&".join([f"{k}={v}" for k, v in query_parameters.items()]) | ||
| return "?" + urlencode(query_parameters) |
There was a problem hiding this comment.
By default, urlencode uses quote_plus which encodes spaces as +. While + is standard for application/x-www-form-urlencoded POST bodies, RFC 3986 defines + as a reserved character in URLs, and spaces should be encoded as %20 in query strings. Some web servers or API gateways do not decode + to spaces in query parameters, which can lead to unexpected behavior (e.g., receiving literal + characters instead of spaces). Using quote_via=quote ensures spaces are encoded as %20, which is universally compatible and compliant with RFC 3986.
Additionally, using doseq=True ensures that if any query parameter contains a list or sequence of values, it is correctly encoded as repeated parameters (e.g., param=a¶m=b) rather than stringifying the sequence.
| return "?" + urlencode(query_parameters) | |
| return "?" + urlencode(query_parameters, doseq=True, quote_via=quote) |
| def test_reserved_characters_round_trip(self): | ||
| parameters = { | ||
| "reference": "Invoice A&B + 10% #café", | ||
| "filter[key]&name": "x=y?z/1", | ||
| "empty": "", | ||
| "pageNumber": 0, | ||
| } | ||
| encoded = get_query(parameters) | ||
| self.assertEqual( | ||
| parse_qs(urlsplit("https://example.test/" + encoded).query, keep_blank_values=True), | ||
| {key: [str(value)] for key, value in parameters.items()}, | ||
| ) | ||
| self.assertEqual(urlsplit("https://example.test/" + encoded).fragment, "") |
There was a problem hiding this comment.
Update the test to verify that spaces are encoded as %20 (not +) and to cover list/sequence parameters using doseq=True.
| def test_reserved_characters_round_trip(self): | |
| parameters = { | |
| "reference": "Invoice A&B + 10% #café", | |
| "filter[key]&name": "x=y?z/1", | |
| "empty": "", | |
| "pageNumber": 0, | |
| } | |
| encoded = get_query(parameters) | |
| self.assertEqual( | |
| parse_qs(urlsplit("https://example.test/" + encoded).query, keep_blank_values=True), | |
| {key: [str(value)] for key, value in parameters.items()}, | |
| ) | |
| self.assertEqual(urlsplit("https://example.test/" + encoded).fragment, "") | |
| def test_reserved_characters_round_trip(self): | |
| parameters = { | |
| "reference": "Invoice A&B + 10% #café", | |
| "filter[key]&name": "x=y?z/1", | |
| "empty": "", | |
| "pageNumber": 0, | |
| "list_param": ["a", "b"], | |
| } | |
| encoded = get_query(parameters) | |
| self.assertNotIn("+", encoded) | |
| self.assertEqual( | |
| parse_qs(urlsplit("https://example.test/" + encoded).query, keep_blank_values=True), | |
| {key: [str(v) for v in value] if isinstance(value, list) else [str(value)] for key, value in parameters.items()}, | |
| ) | |
| self.assertEqual(urlsplit("https://example.test/" + encoded).fragment, "") |
get_querydirectly interpolates query keys and values. Reserved characters are therefore interpreted as URL syntax:&creates extra parameters,+becomes a space, and#truncates the transmitted query by starting a fragment. Unicode and percent characters are also left unencoded.Use
urllib.parse.urlencodefor the existing scalar query dictionary. Preserve the leading?, numeric values, insertion order, and empty-dictionary output. No generated service or template changes are needed; this changes the shared handwritten utility.Validation:
parse_qs.unittest discover -s test -p '*Test.py'suite passes: 184 tests (plus an existing nested 12-test run).git diff --checkpasses.