fix: Return None for malformed feature flag payloads

.sampo/changesets/steadfast-baroness-vellamo.md

@@ -0,0 +1,5 @@
+---
+pypi/posthog: patch
+---
+
+Return None for malformed, empty, or whitespace-only serialized JSON feature flag payloads instead of returning the raw string or raising a JSONDecodeError. Legacy bulk getters (`get_all_flags_and_payloads`, `get_feature_flags_and_payloads`, and `get_feature_payloads`) validate payloads but preserve valid serialized JSON for compatibility with callers using `json.loads()`. Single-flag and `evaluate_flags()` snapshot payload getters continue returning decoded values, including JSON strings (such as `""`), false, and zero. Reject non-JSON constants such as NaN and Infinity and isolate decoder-limit failures so flag values and healthy sibling payloads remain available.

posthog/client.py

@@ -101,7 +101,8 @@
     remote_config,
     reset_sessions,
 )
-from posthog.types import (
+from .types import (
+    _parse_flag_payload,
     FeatureFlag,
     FeatureFlagError,
     FeatureFlagResult,
@@ -342,20 +343,6 @@ def _parse_has_experiment(value: Any) -> Optional[bool]:
     return value if isinstance(value, bool) else None
 
 
-def _parse_flag_payload(raw_payload: Any) -> Optional[Any]:
-    """Flag payloads are stored as JSON strings, both in the ``/flags`` response
-    metadata and in the local-evaluation flag definitions, so decode them before
-    handing them to callers. A string that isn't valid JSON is passed through as-is."""
-    if isinstance(raw_payload, str):
-        if not raw_payload:
-            return None
-        try:
-            return json.loads(raw_payload)
-        except (json.JSONDecodeError, TypeError):
-            return raw_payload
-    return raw_payload
-
-
 def _metadata_has_experiment(metadata: Any) -> Optional[bool]:
     """Server-reported experiment linkage from flag metadata; ``None`` when absent
     (e.g. ``LegacyFlagMetadata``, which doesn't carry the field)."""
@@ -1390,9 +1377,9 @@ def get_feature_payloads(
         disable_geoip: Optional[bool] = None,
         flag_keys_to_evaluate: Optional[list[str]] = None,
         device_id: Optional[str] = None,
-    ) -> dict[str, str]:
+    ) -> dict[str, Optional[str]]:
         """
-        Get feature flag payloads for a user.
+        Get feature flag payloads for a user, preserving valid serialized JSON.
 
         Args:
             distinct_id: The distinct ID of the user.
@@ -1421,7 +1408,10 @@ def get_feature_payloads(
             flag_keys_to_evaluate,
             device_id=device_id,
         )
-        return to_payloads(resp_data) or {}
+        return {
+            key: _parse_flag_payload(payload, decode=False)
+            for key, payload in (to_payloads(resp_data) or {}).items()
+        }
 
     def get_feature_flags_and_payloads(
         self,
@@ -1463,7 +1453,14 @@ def get_feature_flags_and_payloads(
             flag_keys_to_evaluate,
             device_id=device_id,
         )
-        return to_flags_and_payloads(resp)
+        response = to_flags_and_payloads(resp)
+        payloads = response.get("featureFlagPayloads")
+        if payloads is not None:
+            response["featureFlagPayloads"] = {
+                key: _parse_flag_payload(payload, decode=False)
+                for key, payload in payloads.items()
+            }
+        return response
 
     def get_flags_decision(
         self,
@@ -4410,12 +4407,18 @@ def get_all_flags_and_payloads(
                     flag_keys_to_evaluate=flag_keys_to_evaluate,
                     device_id=device_id,
                 )
-                return to_flags_and_payloads(decide_response)
+                response = to_flags_and_payloads(decide_response)
             except Exception as e:
                 self.log.exception(
                     f"[FEATURE FLAGS] Unable to get feature flags and payloads: {e}"
                 )
 
+        payloads = response.get("featureFlagPayloads")
+        if payloads is not None:
+            response["featureFlagPayloads"] = {
+                key: _parse_flag_payload(payload, decode=False)
+                for key, payload in payloads.items()
+            }
         return response
 
     def evaluate_flags(

posthog/test/test_client.py

@@ -4547,7 +4547,7 @@ def test_get_feature_flag_result_with_empty_string_payload(self, patch_batch_pos
         self.assertIsNotNone(result)
         self.assertEqual(result.key, "test-flag")
         self.assertEqual(result.get_value(), "empty-variant")
-        self.assertEqual(result.payload, "")  # Should be empty string, not None
+        self.assertIsNone(result.payload)
 
     @mock.patch("posthog.client.batch_post")
     def test_get_all_flags_and_payloads_with_empty_string(self, patch_batch_post):
@@ -4585,7 +4585,7 @@ def test_get_all_flags_and_payloads_with_empty_string(self, patch_batch_post):
                     "multivariate": {
                         "variants": [{"key": "variant2", "rollout_percentage": 100}]
                     },
-                    "payloads": {"variant2": "normal payload"},
+                    "payloads": {"variant2": '"normal payload"'},
                 },
             },
         ]
@@ -4598,11 +4598,10 @@ def test_get_all_flags_and_payloads_with_empty_string(self, patch_batch_post):
         self.assertEqual(result["featureFlags"]["empty-payload-flag"], "variant1")
         self.assertEqual(result["featureFlags"]["normal-payload-flag"], "variant2")
 
-        # Check that empty string payload is included (not filtered out)
         self.assertIn("empty-payload-flag", result["featureFlagPayloads"])
-        self.assertEqual(result["featureFlagPayloads"]["empty-payload-flag"], "")
+        self.assertIsNone(result["featureFlagPayloads"]["empty-payload-flag"])
         self.assertEqual(
-            result["featureFlagPayloads"]["normal-payload-flag"], "normal payload"
+            result["featureFlagPayloads"]["normal-payload-flag"], '"normal payload"'
         )
 
     def test_context_tags_added(self):

posthog/test/test_evaluate_flags.py

@@ -404,10 +404,10 @@ def test_local_payloads_are_parsed(self, patch_flags):
         self.assertEqual(patch_flags.call_count, 0)
 
     @mock.patch("posthog.client.flags")
-    def test_non_json_local_payload_is_passed_through(self, patch_flags):
+    def test_non_json_local_payload_returns_none(self, patch_flags):
         flags = self.client.evaluate_flags("user-1")
 
-        self.assertEqual(flags.get_flag_payload("plain-payload"), "not json")
+        self.assertIsNone(flags.get_flag_payload("plain-payload"))
         self.assertEqual(patch_flags.call_count, 0)
 
     @mock.patch("posthog.client.flags")

posthog/test/test_feature_flags.py

@@ -1263,7 +1263,7 @@ def test_get_all_flags_and_payloads_with_no_fallback(
         ]
         self.assertEqual(
             client.get_all_flags_and_payloads("distinct_id")["featureFlagPayloads"],
-            {"beta-feature": "new"},
+            {"beta-feature": None},
         )
         # /flags is not called because this can be evaluated locally
         self.assertEqual(patch_flags.call_count, 0)
@@ -1403,7 +1403,7 @@ def test_get_all_flags_and_payloads_with_fallback_but_only_local_evaluation_set(
             client.get_all_flags_and_payloads(
                 "distinct_id", only_evaluate_locally=True
             )["featureFlagPayloads"],
-            {"beta-feature": "some-payload"},
+            {"beta-feature": None},
         )
         self.assertEqual(patch_flags.call_count, 0)
         self.assertEqual(patch_capture.call_count, 0)

posthog/test/test_flag_payload_parsing.py

@@ -0,0 +1,221 @@
+import json
+import sys
+from unittest.mock import patch
+
+import pytest
+
+from posthog.client import Client
+from posthog.types import _parse_flag_payload
+
+
+@pytest.mark.parametrize(
+    "api, local",
+    [
+        ("payload", True),
+        ("payload", False),
+        ("snapshot", True),
+        ("snapshot", False),
+        ("bulk", True),
+        ("bulk", False),
+        ("remote_bulk", False),
+        ("remote_payloads", False),
+    ],
+)
+@pytest.mark.parametrize(
+    "raw, expected",
+    [
+        ('{"broken":', None),
+        ("not json", None),
+        ("   ", None),
+        ("", None),
+        ("NaN", None),
+        ("Infinity", None),
+        ("-Infinity", None),
+        ('{"nested": [NaN]}', None),
+        ("[Infinity, -Infinity]", None),
+        pytest.param("[" * 20000, None, id="decoder-recursion-limit"),
+        pytest.param(
+            "9" * (getattr(sys, "get_int_max_str_digits", lambda: 0)() + 1),
+            None,
+            id="decoder-integer-limit",
+            marks=pytest.mark.skipif(
+                not getattr(sys, "get_int_max_str_digits", lambda: 0)(),
+                reason="Integer conversion limit is unavailable or disabled",
+            ),
+        ),
+        ("[1, 2]", [1, 2]),
+        ('{"ok": true}', {"ok": True}),
+        ('  {"ok": true}\n', {"ok": True}),
+        ('"text"', "text"),
+        ('"123"', "123"),
+        ('"true"', "true"),
+        ('"NaN"', "NaN"),
+        ('"Infinity"', "Infinity"),
+        ('""', ""),
+        ("false", False),
+        ("0", 0),
+        ("null", None),
+        ({"decoded": True}, {"decoded": True}),
+        (None, None),
+    ],
+)
+def test_payload_parsing(local, api, raw, expected):
+    client = Client("test-key", send=False)
+    if local:
+        client.feature_flags = [
+            {
+                "id": 1,
+                "key": "test-flag",
+                "active": True,
+                "filters": {
+                    "groups": [{"properties": [], "rollout_percentage": 100}],
+                    "payloads": {"true": raw},
+                },
+            },
+            {
+                "id": 2,
+                "key": "healthy",
+                "active": True,
+                "filters": {
+                    "groups": [{"properties": [], "rollout_percentage": 100}],
+                    "payloads": {"true": '{"ok": true}'},
+                },
+            },
+        ]
+    response = {
+        "flags": {
+            "test-flag": {
+                "key": "test-flag",
+                "enabled": True,
+                "variant": None,
+                "reason": {"code": "condition_match", "description": "Matched"},
+                "metadata": {"id": 1, "version": 1, "payload": raw},
+            },
+            "healthy": {
+                "enabled": True,
+                "metadata": {"payload": '{"ok": true}'},
+            },
+        }
+    }
+    try:
+        with (
+            patch.object(client, "load_feature_flags"),
+            patch("posthog.client.flags", return_value=response) as request,
+        ):
+            if api in ("bulk", "remote_bulk"):
+                bulk = (
+                    client.get_all_flags_and_payloads(
+                        "user", only_evaluate_locally=local
+                    )
+                    if api == "bulk"
+                    else client.get_feature_flags_and_payloads("user")
+                )
+                assert bulk["featureFlags"] == {"test-flag": True, "healthy": True}
+                assert bulk["featureFlagPayloads"]["healthy"] == '{"ok": true}'
+                result = bulk["featureFlagPayloads"].get("test-flag")
+            elif api == "remote_payloads":
+                payloads = client.get_feature_payloads("user")
+                assert payloads["healthy"] == '{"ok": true}'
+                result = payloads.get("test-flag")
+            elif api == "payload":
+                with pytest.warns(DeprecationWarning):
+                    result = client.get_feature_flag_payload(
+                        "test-flag", "user", only_evaluate_locally=local
+                    )
+            else:
+                result = client.evaluate_flags("user", only_evaluate_locally=local)
+                assert result.get_flag_payload("healthy") == {"ok": True}
+                assert result.get_flag("test-flag") is True
+                result = result.get_flag_payload("test-flag")
+            if api in ("bulk", "remote_bulk", "remote_payloads") and isinstance(
+                raw, str
+            ):
+                if expected is not None or raw == "null":
+                    assert result == raw
+                    assert json.loads(result) == expected
+                else:
+                    assert result is None
+            else:
+                assert result == expected
+                assert type(result) is type(expected)
+            assert request.call_count == (0 if local else 1)
+    finally:
+        client.shutdown()
+
+
+@pytest.mark.parametrize(
+    "raw", ['{"private":', "", "   ", "NaN", "Infinity", "-Infinity"]
+)
+@pytest.mark.parametrize("decode", [True, False])
+def test_parse_failure_logs_without_payload(raw, decode, caplog):
+    with caplog.at_level("WARNING", logger="posthog"):
+        for _ in range(10):
+            assert _parse_flag_payload(raw, decode=decode) is None
+    assert not caplog.records
+
+    with caplog.at_level("DEBUG", logger="posthog"):
+        assert _parse_flag_payload(raw, decode=decode) is None
+    assert len(caplog.records) == 1
+    assert caplog.records[0].levelname == "DEBUG"
+    assert caplog.records[0].getMessage().removeprefix("[PostHog] ") == (
+        "[FEATURE FLAGS] Unable to parse flag payload as JSON"
+    )
+    assert caplog.records[0].exc_info is None
+
+
+@pytest.mark.parametrize("local", [True, False])
+@pytest.mark.parametrize("value", [True, False, "blue"])
+@pytest.mark.parametrize("raw", ['{"broken":', "", "   "])
+def test_invalid_payload_preserves_flag_getters(local, value, raw):
+    client = Client("test-key", send=False)
+    filters = {
+        "groups": [{"properties": [], "rollout_percentage": 100 if value else 0}],
+        "payloads": {str(value).lower(): raw},
+    }
+    if isinstance(value, str):
+        filters["multivariate"] = {
+            "variants": [{"key": value, "rollout_percentage": 100}]
+        }
+    if local:
+        client.feature_flags = [
+            {"id": 1, "key": "test-flag", "active": True, "filters": filters}
+        ]
+    response = {
+        "flags": {
+            "test-flag": {
+                "enabled": value is not False,
+                "variant": value if isinstance(value, str) else None,
+                "reason": {"code": "condition_match", "description": "Matched"},
+                "metadata": {"id": 1, "version": 1, "payload": raw},
+            }
+        }
+    }
+    try:
+        with (
+            patch.object(client, "load_feature_flags"),
+            patch("posthog.client.flags", return_value=response) as request,
+        ):
+            result = client.get_feature_flag_result(
+                "test-flag", "user", only_evaluate_locally=local
+            )
+            assert result is not None
+            assert result.key == "test-flag"
+            assert result.get_value() == value
+            assert result.enabled is (value is not False)
+            assert result.variant == (value if isinstance(value, str) else None)
+            assert result.payload is None
+            assert result.reason == (None if local else "Matched")
+            with pytest.warns(DeprecationWarning):
+                assert (
+                    client.get_feature_flag(
+                        "test-flag", "user", only_evaluate_locally=local
+                    )
+                    == value
+                )
+            with pytest.warns(DeprecationWarning):
+                assert client.feature_enabled(
+                    "test-flag", "user", only_evaluate_locally=local
+                ) is (value is not False)
+            assert request.call_count == (0 if local else 3)
+    finally:
+        client.shutdown()