Skip to content

Commit

Permalink
Fix response body handling to by bytes-based in all cases (#132)
Browse files Browse the repository at this point in the history
* Always handle response bodies as bytes

* Internally handle request bodies (and matchers) as bytes

* Fix history rst

* Correct history to reflect drafted nature of v2

* Ensure body is tested matching regex pattern type
  • Loading branch information
sarayourfriend committed Jun 27, 2024
1 parent 495f699 commit df93cbb
Show file tree
Hide file tree
Showing 25 changed files with 263 additions and 114 deletions.
32 changes: 32 additions & 0 deletions History.rst
Original file line number Diff line number Diff line change
@@ -1,6 +1,38 @@
History
=======

Draft v2.0.0 / Unreleased
-------------------------

* **Breaking change**: Remove ``Response::body``'s ``binary`` parameter and enforce a keyword argument for ``chunked``

* The ``binary`` parameter is no longer needed since responses are now always byte-encoded in all cases (see below).
* A keyword argument is now enforced for ``chunked`` to ensure unnamed arguments meant for the removed ``binary`` parameter cannot be confused as ``chunked`` arguments.

* Only return byte-encoded response bodies, matching the bahviour of all supported client libraries

* This is possible because for all supported client libraries, pook mocks the actual response sent to the
client library over the wire, which will, in all cases, be bytes. Client libraries that support reading
response content as a string or other formats will continue to work as expected, because they'll always
be handling bytes from pook.
* This causes no change to application code for users of pook, except for users of the standard library ``urllib``,
for which this also fixed a bug where non-bytes bodies were returned by pook in certain cases. This is impossible
in real application code. If you rely on pook to mock ``urllib`` responses and have code that handles non-bytes response
bodies, that code can be safely deleted (provided the only reason it was there was pook in the first place).

* **Breaking change**: Remove ``Mock::body``'s ``binary`` parameter.

* This parameter was already unused due to a bug in the code (it was not passed through to the ``BodyMatcher``),
so this will not cause any changes to tests that relied on it: it didn't do anything to begin with.
* The breaking change is simply the removal of the unused parameter, which should be deleted from tests using pook.
* Pook's code has also been updated to remove all cases where non-bytes objects were being handled. Instead, the body
of any interecepted request will always be stored as bytes, and only decoded when necessary for individual downstream
matchers (JSON, XML).

* Correct documentation strings for ``XMLMatcher`` and ``JSONMatcher`` to no longer suggest they can handle regex matchers

* These classes never implemented regex matching

v1.4.3 / 2024-02-23
-------------------

Expand Down
3 changes: 3 additions & 0 deletions src/pook/assertion.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ def matches(x, y, regex_expr=False):
# Parse regex expression, if needed
x = strip_regex(x) if regex_expr and isregex_expr(x) else x

if isinstance(getattr(x, "pattern", None), str) and hasattr(y, "decode"):
y = y.decode("utf-8", "backslashescape")

# Assert regular expression via unittest matchers
return test_case().assertRegex(y, x) or True

Expand Down
6 changes: 1 addition & 5 deletions src/pook/interceptors/aiohttp.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,11 +115,7 @@ async def _on_request(
_res._headers = multidict.CIMultiDictProxy(multidict.CIMultiDict(headers))

if res._body:
_res.content = SimpleContent(
res._body
if res._binary
else res._body.encode("utf-8", errors="replace"),
)
_res.content = SimpleContent(res._body)
else:
# Define `_content` attribute with an empty string to
# force do not read from stream (which won't exists)
Expand Down
2 changes: 1 addition & 1 deletion src/pook/interceptors/http.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def getresponse():

# Path reader
def read():
return res._body or ""
return res._body or b""

mockres.read = read

Expand Down
3 changes: 0 additions & 3 deletions src/pook/interceptors/urllib3.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,6 @@ def _on_request(

if is_chunked_response(headers):
body_chunks = body if isinstance(body, list) else [body]
body_chunks = [
chunk if res._binary else chunk.encode() for chunk in body_chunks
]

body = ClientHTTPResponse(MockSock)
body.fp = FakeChunkedResponseBody(body_chunks)
Expand Down
10 changes: 0 additions & 10 deletions src/pook/matchers/body.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,6 @@ class BodyMatcher(BaseMatcher):
regular expression based matching.
"""

def __init__(self, *args, **kwargs):
self.binary = kwargs.pop("binary", False)
super().__init__(*args, **kwargs)

@BaseMatcher.matcher
def match(self, req):
expectation = self.expectation

# Decode bytes input
if isinstance(expectation, bytes) and not self.binary:
expectation = expectation.decode("utf-8")

return self.compare(self.expectation, req.body)
31 changes: 19 additions & 12 deletions src/pook/matchers/json.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,26 @@
import json
from .base import BaseMatcher
from ..assertion import equal


class JSONMatcher(BaseMatcher):
"""
JSONMatcher implements a JSON body matcher supporting strict structure
and regular expression based comparisons.
Match JSON documents of equivalent value.
JSON documents are matched on the structured data in the document,
rather than on the strict organisation of the document.
The following two JSON snippets are treated as identical by this matcher:
{"a": "one", "b": ["two"]}
... is considered idential to ...
{"b": ["two"], "a": "one"}
In other words, the order does not matter in comparison.
Use ``BodyMatcher`` to strictly match the exact textual structure.
"""

def __init__(self, data):
Expand All @@ -16,15 +31,7 @@ def __init__(self, data):

@BaseMatcher.matcher
def match(self, req):
body = req.body

if isinstance(body, str):
try:
body = json.loads(body)
except Exception:
return False

x = json.dumps(self.expectation, sort_keys=True, indent=4)
y = json.dumps(body, sort_keys=True, indent=4)
y = json.dumps(req.json, sort_keys=True, indent=4)

return self.compare(x, y)
return equal(x, y)
12 changes: 3 additions & 9 deletions src/pook/matchers/json_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,13 @@ def __init__(self, schema):

@BaseMatcher.matcher
def match(self, req):
body = req.body
req_json = req.json

if isinstance(body, str):
try:
body = json.loads(body)
except Exception:
return False

if not body:
if not req_json:
return False

try:
validate(body, self.expectation)
validate(req_json, self.expectation)
except Exception:
return False

Expand Down
29 changes: 23 additions & 6 deletions src/pook/matchers/xml.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,29 @@
import json
import xmltodict
from .base import BaseMatcher
from ..assertion import equal


class XMLMatcher(BaseMatcher):
"""
XMLMatcher implements a XML body matcher supporting both strict structure
comparison and regular expression.
Match XML documents of equivalent structural value.
XML documents are matched on the structured data in the document,
rather than on the strict organisation of the document.
The following two XML snippets are treated as identical by this matcher:
<a value="one"></a>
<b>two</b>
... is considered idential to ...
<b>two</b>
<a value="one"></a>
In other words, the order does not matter in comparison.
Use ``BodyMatcher`` to strictly match the exact textual structure.
"""

def __init__(self, data):
Expand All @@ -19,13 +36,13 @@ def compare(self, data):
x = json.dumps(xmltodict.parse(data), sort_keys=True)
y = json.dumps(self.expectation, sort_keys=True)

return x == y
return equal(x, y)

@BaseMatcher.matcher
def match(self, req):
data = req.body
xml = req.xml

if not isinstance(data, str):
if not isinstance(xml, str):
return False

return self.compare(data)
return self.compare(xml)
17 changes: 9 additions & 8 deletions src/pook/mock.py
Original file line number Diff line number Diff line change
Expand Up @@ -396,21 +396,23 @@ def params(self, params):
self.add_matcher(matcher("QueryMatcher", params))
return self

def body(self, body, binary=False):
def body(self, body):
"""
Defines the body data to match.
``body`` argument can be a ``str``, ``binary`` or a regular expression.
``body`` argument can be a ``str``, ``bytes`` or a regular expression.
Arguments:
body (str|binary|regex): body data to match.
binary (bool): prevent decoding body as text when True.
body (str|bytes|regex): body data to match.
Returns:
self: current Mock instance.
"""
if hasattr(body, "encode"):
body = body.encode("utf-8", "backslashreplace")

self._request.body = body
self.add_matcher(matcher("BodyMatcher", body, binary=False))
self.add_matcher(matcher("BodyMatcher", body))
return self

def json(self, json):
Expand Down Expand Up @@ -468,9 +470,8 @@ def file(self, path):
Returns:
self: current Mock instance.
"""
with open(path, "r") as f:
self.body(str(f.read()))
return self
with open(path, "rb") as f:
return self.body(f.read())

def add_matcher(self, matcher):
"""
Expand Down
23 changes: 10 additions & 13 deletions src/pook/request.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class Request(object):
headers (dict): HTTP headers to match.
query (dict): URL query params to match. Complementely to URL
defined query params.
body (str|regex): request body payload to match.
body (bytes|regex): request body payload to match.
json (str|dict|list): JSON payload body structure to match.
xml (str): XML payload data structure to match.
Expand All @@ -28,13 +28,13 @@ class Request(object):
headers (dict): HTTP headers to match.
query (dict): URL query params to match. Complementely to URL
defined query params.
body (str|regex): request body payload to match.
body (bytes|regex): request body payload to match.
json (str|dict|list): JSON payload body structure to match.
xml (str): XML payload data structure to match.
"""

# Store keys
keys = ("method", "headers", "body", "url", "query")
keys = ("method", "headers", "body", "url", "query", "xml", "json")

def __init__(self, method="GET", **kw):
self._url = None
Expand Down Expand Up @@ -111,32 +111,29 @@ def body(self):

@body.setter
def body(self, body):
if hasattr(body, "decode"):
try:
body = body.decode("utf-8", "strict")
except Exception:
pass
if hasattr(body, "encode"):
body = body.encode("utf-8", "backslashreplace")

self._body = body

@property
def json(self):
return _json.loads(self._body)
return _json.loads(self.body.decode("utf-8"))

@json.setter
def json(self, data):
if isinstance(data, str):
self._body = data
self.body = data
else:
self._body = _json.dumps(data)
self.body = _json.dumps(data)

@property
def xml(self):
return self._body
return self.body.decode("utf-8")

@xml.setter
def xml(self, data):
self._body = data
self.body = data

def copy(self):
"""
Expand Down
Loading

0 comments on commit df93cbb

Please sign in to comment.