Inconsistent behaviour in query string matching with spaces #370
Comments
|
hi @WillMorrisonEnspi , I think this behavior is consistent.
This behaviour is partially documented (and unfortunatelly missing from the built html doc, that will be fixed): String matching: https://github.com/csernazs/pytest-httpserver/blob/master/pytest_httpserver/httpserver.py#L188 Mapping matching: https://github.com/csernazs/pytest-httpserver/blob/master/pytest_httpserver/httpserver.py#L188 Also, there's a howto about this: https://pytest-httpserver.readthedocs.io/en/latest/howto.html#matching-query-parameters Here, you specified an already quoted value in the dict, so it got double-quoted in pytest-httpserver: Here, you specified an unquoted string, which then did't match with the quoted one sent by requests as server compares it with httpserver.expect_request("/test", query_string="foo=bar baz").respond_with_data("OK")pytest-httpserver provides you an API to define your own query matcher, so the first failed case can be solved by this: class MyQueryStringMatcher(QueryMatcher):
def __init__(self, expected_string: str):
parsed = urllib.parse.parse_qsl(expected_string) # Parse query string into key-value pairs
self._encoded_query_string = urllib.parse.urlencode(parsed, quote_via=urllib.parse.quote)
def get_comparing_values(self, request_query_string: bytes) -> tuple:
return (self._encoded_query_string, request_query_string.decode("utf-8"))
def test_query_string_with_spaces_string_fails(httpserver: HTTPServer):
httpserver.expect_request("/test", query_string=MyQueryStringMatcher("foo=bar baz")).respond_with_data("OK")
url = httpserver.url_for("/test") + "?foo=bar baz"
requests.get(url)
httpserver.check_assertions()The other case can be solved by this: class QuotedDictMatcher(MappingQueryMatcher):
def __init__(self, query_dict: dict[str, str]):
unquoted_dict = {k: urllib.parse.unquote(v) for k, v in query_dict.items()}
super().__init__(unquoted_dict)
def test_query_string_is_encoded_dict_fails(httpserver: HTTPServer):
httpserver.expect_request("/test", query_string=QuotedDictMatcher({"foo": "bar%20baz"})).respond_with_data("OK")
url = httpserver.url_for("/test") + "?foo=bar baz"
requests.get(url)
httpserver.check_assertions()Changing querystring matcher in pytest-httpserver to handle your case would be an API breaking, and it would break the current cases. Zsolt |
This is exactly the part I find inconsistent. Strings must already be quoted, while dict values should not already be quoted.
I did read through the docs, but this was not explained there. The String matcher documents what happens when you pass string vs bytes, and says that the bytes must match the incoming request exactly (that's correct and matches the behaviour from my reproduction test). However, the Mapping matcher docs don't mention anything about quoting. I would read this as I was also thrown off by the fact that the assertion messages from the Mapping matcher don't show the actual level of percent decoding happening. Compare: Perhaps the Mapping matcher's
This is a bit tricky to handle, I agree. Options below.
Footnotes
|
The main idea behind this is that pytest-httpserver cannot quote this: I also think that interpreting the One might says it is: Other might say: You can say that
Yes, I agree document needs to be extended with quoting details.
Thats a good idea.
If existing tests provide
I think this can be done. The matcher interface is designed for this, to provide more flexibility. I agree writing Alternatively there would be a
That would be the easiest for sure, but the question is how this can be implemented in a way that:
Welp, this also introduces ambiguity, if one wants to check if their client properly quotes the query parameters, then this won't check that. Zsolt |
|
Summarizing the conversation so far.
If 2. and 3. had already been implemented I would not have filed this bug (because I'd figure out that the behaviour is just the documented API), so I don't consider 4. to be required to resolve this issue. However, at some point I might want to write a test that checks if a client properly quotes query parameters independently of their order, so 4. is still a good feature to add. |
|
Yes, that's correct, I agree with all the points. Thanks for the summary! I'll do 2..4 points sometime in the near future. Zsolt |
It seems like the type (string vs dict) of
query_stringaffects the behaviour of the resulting matcher when the query string in the request has percent-encoding. Specifically, a string typed argument toquery_stringmust match exactly the query string in the request. However, a dict typed argument toquery_stringmust match the request's query string after unquoting.Reproduced with pytest-httpserver version 1.1.0
test file:
Output:
The text was updated successfully, but these errors were encountered: