-
Notifications
You must be signed in to change notification settings - Fork 21
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. | ||
""" | ||
self.headers: list[tuple[str, str]] = headers if headers is not None else [] | ||
self._key = key | ||
|
@@ -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 commentThe 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 commentThe 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. |
||
self.query_params.append((key, value)) | ||
|
||
def write_timestamp(self, schema: Schema, value: datetime) -> None: | ||
key = self._key or schema.expect_trait(HTTPQueryTrait).key | ||
|
@@ -570,12 +570,28 @@ def __init__(self, query_params: list[tuple[str, str]]) -> None: | |
:param query_params: The list of query param tuples to append to. | ||
""" | ||
self._query_params = query_params | ||
self._delegate = CapturingSerializer() | ||
|
||
def entry(self, key: str, value_writer: Callable[[ShapeSerializer], None]): | ||
value_writer(self._delegate) | ||
assert self._delegate.result is not None # noqa: S101 | ||
self._query_params.append((key, urlquote(self._delegate.result, safe=""))) | ||
value_writer(HTTPQueryMapValueSerializer(key, self._query_params)) | ||
|
||
|
||
class HTTPQueryMapValueSerializer(SpecificShapeSerializer): | ||
def __init__(self, key: str, query_params: list[tuple[str, str]]) -> None: | ||
"""Initialize an HTTPQueryMapValueSerializer. | ||
|
||
:param key: The key of the query parameter. | ||
:param query_params: The list of query param tuples to append to. | ||
""" | ||
self._key = key | ||
self._query_params = query_params | ||
|
||
def write_string(self, schema: Schema, value: str) -> None: | ||
# Note: values are escaped when query params are joined | ||
self._query_params.append((self._key, value)) | ||
|
||
@contextmanager | ||
def begin_list(self, schema: Schema, size: int) -> Iterator[ShapeSerializer]: | ||
yield self | ||
|
||
|
||
class HostPrefixSerializer(SpecificShapeSerializer): | ||
|
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