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

[vnatakam]

Summary

Fixes INT-1681. Resolves the same bytes/str issue originally identified in PR #63 (open since 2017).

Root cause: simplejson (a required dependency) does not accept bytes in Python 3, unlike stdlib json which does. The code was passing response.content (bytes) directly to json.loads, causing:

TypeError: Input string must be text, not bytes

This error was silently masked for users running without simplejson installed (stdlib json handles bytes fine since Python 3.6), but surfaces when simplejson is present — which it always is since it's in install_requires.

Changes

• sailthru_response.py: Use response.text (str) instead of response.content (bytes) when calling json.loads. Also broadened the exception handler to catch TypeError in addition to ValueError for robustness.
• sailthru_client.py — receive_verify_post: Removed redundant json.loads() call. get_body() already returns a parsed dict — calling json.loads on a dict fails. Now uses the dict directly.
• sailthru_client.py — get_signature_string: Was returning bytes due to .encode('utf-8'). Moved the encoding into get_signature_hash where hashlib.md5 actually needs bytes. get_signature_string now correctly returns a str.
• test/test_sailthru_client.py: Updated receive_verify_post mock to return a dict (matching real get_body() behaviour) instead of a JSON string.
• setup.py: Bumped version to 2.3.6.

[snyk-io]

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

[Copilot]

Pull request overview

Fixes Python 3 bytes/str incompatibilities that surface when simplejson is installed (as required), and aligns postback verification logic with the actual types returned by SailthruResponse.get_body().

Changes:

• Parse API responses via response.text and broaden JSON parse error handling to include TypeError.
• Fix signature generation to keep get_signature_string() as str and move UTF-8 encoding to the MD5 hashing step.
• Correct receive_verify_post() to use the dict returned by get_body() (and update the related unit test mock), plus bump package version.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
sailthru/sailthru_response.py	Switch JSON parsing to use response.text and catch (ValueError, TypeError) to avoid bytes inputs with simplejson.
sailthru/sailthru_client.py	Make signature generation consistently str until hashing; simplify verify postback by using parsed response dict directly.
test/test_sailthru_client.py	Update receive_verify_post test double to return a dict from get_body() to match real behavior.
setup.py	Bump package version to 2.3.6.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

send_response.get_body() can return non-dict JSON types (e.g., a JSON string) or a test double; in that case 'email' in send_json may be true but send_json['email'] will raise TypeError. Consider validating send_json is a dict (or catching TypeError/KeyError) before indexing into it.

Suggested change

	if not send_json or 'email' not in send_json:
	if not isinstance(send_json, dict) or 'email' not in send_json:

[bge-kernel-panic]

This is a good point. If it's an array or string 'email' not in send_json could still return false and yet you'd end up with issues later on.

Try:

if not isinstance(send_json, dict):
    return False
if send_json.get('email', None) != post_params['email']: # post_params is guaranteed to contain email, see line 575
    return False

[Copilot]

The package version is bumped here, but the codebase also embeds version strings elsewhere (e.g. sailthru/__init__.py has __version__ and sailthru/sailthru_http.py hardcodes the version in the User-Agent). Please update those (and add a 2.3.6 entry to CHANGES) so the distribution metadata and runtime-reported version stay consistent.

[bge-kernel-panic]

Agree with this.

[Copilot]

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.

[bge-kernel-panic]

This does seem like an unrelated change.