Repository navigation
fix: preserve empty JSON payloads in alternative transports - #510
Shubham-Padkonde wants to merge 2 commits 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 updates the HTTP client to properly support empty JSON payloads (like {}) by changing truthiness checks on the json parameter to explicit is not None checks. It also adds comprehensive unit tests to verify this behavior across different HTTP client transports. The review comments point out potential TypeError bugs in both the pycurl and urllib request implementations when both json and data are None, as urlencode(data) would be executed before any validation. Code suggestions are provided to validate that at least one of these parameters is present before processing.
| raw_store = json | ||
|
|
||
| # Set the request body. | ||
| raw_request = json_lib.dumps(json) if json else urlencode(data) | ||
| raw_request = json_lib.dumps(json) if json is not None else urlencode(data) |
There was a problem hiding this comment.
If both json and data are None, calling urlencode(data) on line 103 will raise a TypeError. Adding a check to ensure at least one of them is provided makes the error handling robust and consistent across transports.
| raw_store = json | |
| # Set the request body. | |
| raw_request = json_lib.dumps(json) if json else urlencode(data) | |
| raw_request = json_lib.dumps(json) if json is not None else urlencode(data) | |
| if json is None and not data: | |
| raise ValueError("Please provide either a json or a data field.") | |
| raw_store = json | |
| # Set the request body. | |
| raw_request = json_lib.dumps(json) if json is not None else urlencode(data) |
| if method == "POST" or method == "PATCH": | ||
| # Store regular dict to return later: | ||
| raw_store = json | ||
| raw_request = json_lib.dumps(json) if json else urlencode(data) | ||
| raw_request = json_lib.dumps(json) if json is not None else urlencode(data) | ||
| url_request = Request(url, data=raw_request.encode("utf8"), method=method) | ||
| raw_request = raw_store | ||
| if json: | ||
| if json is not None: | ||
| url_request.add_header("Content-Type", "application/json") | ||
| elif not data: | ||
| raise ValueError("Please provide either a json or a data field.") |
There was a problem hiding this comment.
If both json and data are None (their default values), evaluating urlencode(data) on line 221 will raise a TypeError before the code can reach the elif not data: check on line 226 to raise the more descriptive ValueError. Checking if both are missing at the beginning of the POST/PATCH block avoids this TypeError and ensures the intended ValueError is raised.
| if method == "POST" or method == "PATCH": | |
| # Store regular dict to return later: | |
| raw_store = json | |
| raw_request = json_lib.dumps(json) if json else urlencode(data) | |
| raw_request = json_lib.dumps(json) if json is not None else urlencode(data) | |
| url_request = Request(url, data=raw_request.encode("utf8"), method=method) | |
| raw_request = raw_store | |
| if json: | |
| if json is not None: | |
| url_request.add_header("Content-Type", "application/json") | |
| elif not data: | |
| raise ValueError("Please provide either a json or a data field.") | |
| if method == "POST" or method == "PATCH": | |
| if json is None and not data: | |
| raise ValueError("Please provide either a json or a data field.") | |
| # Store regular dict to return later: | |
| raw_store = json | |
| raw_request = json_lib.dumps(json) if json is not None else urlencode(data) | |
| url_request = Request(url, data=raw_request.encode("utf8"), method=method) | |
| raw_request = raw_store | |
| if json is not None: | |
| url_request.add_header("Content-Type", "application/json") |
With neither json nor data, urllib and pycurl called urlencode(None) and raised a TypeError before the existing ValueError check could run. Check for a missing body first in both transports, and test that a ValueError is raised.
|
|
Good catch: a missing body is now checked before encoding, so it raises the intended ValueError. Added tests. |
This branch has not been deployed
1 participant
Description
Passing
json={}to the urllib or pycurl transport currently falls through tourlencode(None)and raises TypeError before sending a request. The requests transport already sends the empty object correctly.Distinguish an absent JSON argument from an empty object when selecting the request body and Content-Type. This keeps POST and PATCH payloads consistent across the three transports.
Tested scenarios
A local HTTP server verifies the received method, application/json header, and
{}body for POST and PATCH using requests, urllib, and real pycurl. The four urllib/pycurl subcases fail before the fix and pass afterward.python -m unittest discover -s test -p '*Test.py' -q: 184 tests pass on Python 3.13. Changed-file Ruff lint and formatting checks pass. No live payment API calls were made.