Skip to content

Fix Python 3 bytes/str incompatibility in simplejson and postback methods #78

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.

Sign up for GitHub

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

Merged
vnatakam merged 3 commits into from
Apr 23, 2026
Merged

Fix Python 3 bytes/str incompatibility in simplejson and postback methods #78

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e97071c
Fix Python 3 bytes/str incompatibility in simplejson and postback met…
vnatakam Apr 17, 2026
0973712
Address PR review comments
vnatakam Apr 20, 2026
cc949ff
Update test_hardbounce_valid_json mock to return dict matching real g…
vnatakam Apr 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 6 additions & 10 deletions sailthru/sailthru_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,15 @@ def get_signature_string(params, secret):
"""
str_list = [str(item) for item in extract_params(params)]
str_list.sort()
return (secret + ''.join(str_list)).encode('utf-8')
return secret + ''.join(str_list)

def get_signature_hash(params, secret):
"""
Returns an MD5 hash of the signature string for an API call.
@param params: dictionary values to generate signature hash
@param sercret: secret string
"""
return hashlib.md5(get_signature_string(params, secret)).hexdigest()
return hashlib.md5(get_signature_string(params, secret).encode('utf-8')).hexdigest()


class SailthruClient(object):
Expand Down Expand Up @@ -589,14 +589,10 @@ def receive_verify_post(self, post_params):

send_response = self.get_send(post_params['send_id'])

try:
send_body = send_response.get_body()
send_json = json.loads(send_body)
if 'email' not in send_body:
return False
if send_json['email'] != post_params['email']:
return False
except ValueError:
send_json = send_response.get_body()
if not send_json or 'email' not in send_json:
Comment thread
vnatakam marked this conversation as resolved.
Outdated
return False
if send_json['email'] != post_params['email']:
return False

return True
Expand Down
4 changes: 2 additions & 2 deletions sailthru/sailthru_response.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ def __init__(self, response):
self.json_error = None

try:
self.json = json.loads(response.content)
except ValueError as e:
self.json = json.loads(response.text)
except (ValueError, TypeError) as e:
self.json = None
self.json_error = str(e)

Expand Down
2 changes: 1 addition & 1 deletion setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from setuptools import setup, find_packages

setup(name='sailthru-client',
version='2.3.5',
version='2.3.6',
Comment thread
vnatakam marked this conversation as resolved.
packages=find_packages(),
description='Python client for Sailthru API',
long_description=open('README.md').read(),
Expand Down
2 changes: 1 addition & 1 deletion test/test_sailthru_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ def test_check_for_valid_actions(self):

def test_receive_verify_post(self):
mock_http_request = MagicMock()
mock_http_request.return_value.get_body.return_value = '{"email":"menglander@sailthru.com"}'
mock_http_request.return_value.get_body.return_value = {"email": "menglander@sailthru.com"}
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test now mocks get_body() as a dict (matching SailthruResponse.get_body(as_dictionary=True)), but other tests in this file still mock get_body() as a JSON string. Updating those mocks to return dicts as well would prevent tests from accidentally passing due to string-substring checks and better reflect real behavior.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown

@bge-kernel-panic bge-kernel-panic Apr 17, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does seem like an unrelated change.

Copy link
Copy Markdown
Contributor Author

@vnatakam vnatakam Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keeping this change as it is directly required by the code fix. The old test returned a JSON string from get_body() because the old code called json.loads() on it. Since we removed that redundant json.loads() call (the real get_body() already returns a parsed dict), the mock must also return a dict — otherwise the test would be testing behaviour that no longer exists in production code.

Copy link
Copy Markdown

@bge-kernel-panic bge-kernel-panic Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't all other tests also be updated then? i.e. test_hardbounce_valid_json etc

Copy link
Copy Markdown
Contributor Author

@vnatakam vnatakam Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — updated in cc949ff. The only other test using a JSON string mock was test_hardbounce_valid_json. The other two hardbounce tests (test_hardbounce_invalid_json and test_hardbounce_blast_invalid_json) already return None which is appropriate for their error-case scenarios, so those are fine as-is.


self.client._http_request = mock_http_request

Expand Down