-
Notifications
You must be signed in to change notification settings - Fork 20
Fix query binding #502
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
base: develop
Are you sure you want to change the base?
Fix query binding #502
Conversation
050dc5e
to
052fa4d
Compare
052fa4d
to
2b72123
Compare
@@ -486,7 +486,7 @@ def write_big_decimal(self, schema: Schema, value: Decimal) -> None: | |||
|
|||
def write_string(self, schema: Schema, value: str) -> None: | |||
key = self._key or schema.expect_trait(HTTPQueryTrait).key | |||
self.query_params.append((key, urlquote(value, safe=""))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this have any implications on our current signing setup? The signers can double encode but I'm wondering if we're setting that appropriately if we're removing this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't affect signing at all. The code generated bindings also produce an encoded query string.
@@ -328,7 +328,7 @@ def __init__( | |||
:py:class:`HTTPHeaderTrait` will be checked instead. Required when | |||
collecting list entries. | |||
:param headers: An optional list of header tuples to append to. If not | |||
set, one will be created. | |||
set, one will be created. Values appended will not be escaped. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is handling any required escaping now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They're escaped when they're joined
This fixes an issue where query values were being double-encoded, and also serialization of maps of lists in the query string
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.