Skip to content

Commit 733ec9d

Browse files
kukginiclaude
andcommitted
Fix regressions from ujson removal and add regression tests
Fixes layered on top of #1676's ujson removal. Dropping the dependency resolves the four ujson CVEs affecting the previously pinned 1.33 (CVE-2022-31116, CVE-2022-31117, CVE-2026-44660, CVE-2026-54911; CVE-2021-45958 cited in #1676 only affects ujson >= 1.34). - channel.py: the lint fix in #1676 replaced `type(msg) != tuple` with `not isinstance(type(msg), tuple)`, which is always True and wrapped already-tuple messages into nested tuples, breaking handler routing. Restore the original semantics in a lint-clean form (`type(msg) is not tuple`), preserving NamedTuple wrapping. - zstack serializeMsg: ujson emitted compact JSON; stdlib json's default separators add whitespace, growing every wire message and breaking the exact-size assertions in stp_zmq/test/test_zstack.py calibrated to MSG_LEN_LIMIT. Pass separators=(',', ':') to keep the wire format byte-compatible. (Applied to scripts/test_zmq's copy as well.) - recorder: SimpleZStackWithRecorder records raw wire frames; ujson silently encoded bytes as UTF-8 strings while stdlib json raises TypeError, so recording mode (STACK_COMPANION=1) crashed on the first message. Add a bytes-decoding default hook and a bytes regression test. - Add regression tests pinning JsonSerializer's exact byte output (key ordering, compact separators, raw-UTF-8 non-ASCII, float repr, int-key coercion, top-level-bytes base64) since it feeds signing/hashing, plus characterisation of the nested-bytes TypeError. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: kukgini <kukgini@gmail.com>
1 parent b1ab347 commit 733ec9d

6 files changed

Lines changed: 124 additions & 4 deletions

File tree

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
"""
2+
Regression tests pinning the exact byte output of ``JsonSerializer`` after the
3+
removal of the ``ujson`` dependency (CVE-2022-31116 / CVE-2022-31117 /
4+
CVE-2021-45958).
5+
6+
``JsonSerializer`` feeds signing/hashing, so its output must stay byte-stable.
7+
ujson used to be the active encoder; these tests lock the stdlib-``json``
8+
``OrderedJsonEncoder`` output so any future drift (separators, key ordering,
9+
unicode escaping, float repr) is caught.
10+
11+
The encoder is configured with ``sort_keys=True``, ``separators=(',', ':')``
12+
and ``ensure_ascii=False`` -- i.e. sorted keys, no whitespace, and raw UTF-8
13+
for non-ASCII characters (not ``\\uXXXX`` escapes).
14+
"""
15+
import pytest
16+
17+
from common.serializers.json_serializer import JsonSerializer
18+
19+
20+
@pytest.fixture
21+
def sz():
22+
return JsonSerializer()
23+
24+
25+
def test_keys_sorted_and_compact(sz):
26+
# Insertion order must not affect output; keys are sorted, no whitespace.
27+
assert sz.serialize({'b': 2, 'a': 1, 'c': 3}, toBytes=False) == '{"a":1,"b":2,"c":3}'
28+
29+
30+
def test_non_ascii_kept_raw_utf8(sz):
31+
# ensure_ascii=False: characters are emitted as raw UTF-8, not \\uXXXX.
32+
assert sz.serialize({'name': 'héllo', 'kr': '한글', 'emoji': '🚀'},
33+
toBytes=False) == '{"emoji":"🚀","kr":"한글","name":"héllo"}'
34+
# And the bytes form is the UTF-8 encoding of that string.
35+
assert sz.serialize({'name': 'héllo'}) == '{"name":"héllo"}'.encode('utf-8')
36+
37+
38+
def test_float_repr_pinned(sz):
39+
assert sz.serialize({'a': 14.8639, 'b': -97.466179, 'c': 1.0, 'd': 1e20},
40+
toBytes=False) == '{"a":14.8639,"b":-97.466179,"c":1.0,"d":1e+20}'
41+
42+
43+
def test_int_keys_become_strings(sz):
44+
# json coerces non-string keys to strings and then sorts lexicographically.
45+
assert sz.serialize({3: 'c', 1: 'a', 2: 'b'}, toBytes=False) == '{"1":"a","2":"b","3":"c"}'
46+
47+
48+
def test_bool_and_none(sz):
49+
assert sz.serialize({'t': True, 'f': False, 'n': None},
50+
toBytes=False) == '{"f":false,"n":null,"t":true}'
51+
52+
53+
def test_empty_dict(sz):
54+
assert sz.serialize({}, toBytes=False) == '{}'
55+
56+
57+
def test_top_level_bytes_base64(sz):
58+
# The OrderedJsonEncoder.encode override base64-encodes a top-level
59+
# bytes/bytearray value (b'raw' -> base64 'cmF3').
60+
assert sz.serialize(b'raw', toBytes=False) == '"cmF3"'
61+
assert sz.serialize(bytearray(b'raw'), toBytes=False) == '"cmF3"'
62+
63+
64+
def test_round_trip(sz):
65+
data = {'name': 'Alice', 'n': 1, 'f': 1.5, 'b': True, 'list': [1, 'two', None]}
66+
assert sz.deserialize(sz.serialize(data)) == data
67+
assert sz.deserialize(sz.serialize(data, toBytes=False)) == data
68+
69+
70+
@pytest.mark.parametrize('value', [
71+
{'z': b'raw'}, # bytes nested in a dict value
72+
[b'raw'], # bytes nested in a list
73+
{'z': bytearray(b'raw')},
74+
])
75+
def test_nested_bytes_raise_typeerror(sz, value):
76+
"""
77+
KNOWN BEHAVIOURAL DIFFERENCE vs ujson.
78+
79+
The bytes special-case lives in ``OrderedJsonEncoder.encode`` and therefore
80+
only fires for a *top-level* bytes value (see test above). With ujson
81+
installed, bytes nested inside a container were silently encoded as UTF-8
82+
strings; stdlib ``json`` raises ``TypeError`` instead.
83+
84+
This characterises the current behaviour so the difference is explicit and
85+
tracked. If a future change needs nested raw bytes to round-trip, the fix
86+
belongs in ``OrderedJsonEncoder.default`` (not here).
87+
"""
88+
with pytest.raises(TypeError):
89+
sz.serialize(value)

plenum/common/channel.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ def _process_sync(self, msg: Any):
156156
# This is done so that messages can include additional metadata
157157
# isinstance is not used here because it returns true for NamedTuple
158158
# as well.
159-
if not isinstance(type(msg), tuple):
159+
if type(msg) is not tuple:
160160
msg = (msg,)
161161
handler = self._find_handler(msg[0])
162162
if handler is None:

plenum/recorder/recorder.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,18 @@ def add_to_store(self, key, val):
4646
existing = json.loads(existing)
4747
except KeyError:
4848
existing = []
49-
self.store.put(key, json.dumps([*existing, val]))
49+
self.store.put(key, json.dumps([*existing, val],
50+
default=self._bytes_to_str))
51+
52+
@staticmethod
53+
def _bytes_to_str(obj):
54+
# Wire messages and ZMQ identities are recorded as raw bytes; ujson
55+
# encoded them as UTF-8 strings, which stdlib json only does via
56+
# this hook.
57+
if isinstance(obj, (bytes, bytearray)):
58+
return obj.decode()
59+
raise TypeError('Object of type {} is not JSON '
60+
'serializable'.format(type(obj).__name__))
5061

5162
def register_replay_target(self, id, target: Callable):
5263
assert id not in self.replay_targets

plenum/test/recorder/test_recorder.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,22 @@ def test_add_to_recorder(recorder):
5656
i += 1
5757

5858

59+
def test_add_to_recorder_with_bytes(recorder):
60+
# SimpleZStackWithRecorder passes the raw wire frames: both the message
61+
# and the ZMQ identity arrive as undecoded bytes. ujson encoded bytes as
62+
# UTF-8 strings; stdlib json must keep accepting them.
63+
msg1, frm1 = b'{"msg": "m1"}', b'f1'
64+
msg2, to1 = b'{"msg": "m2"}', 't1'
65+
recorder.add_incoming(msg1, frm1)
66+
recorder.add_outgoing(msg2, to1)
67+
68+
all_msgs = []
69+
for _, v in recorder.store.iterator(include_value=True):
70+
all_msgs.extend(Recorder.get_parsed(v))
71+
assert Recorder.filter_incoming(all_msgs) == [['{"msg": "m1"}', 'f1']]
72+
assert Recorder.filter_outgoing(all_msgs) == [['{"msg": "m2"}', 't1']]
73+
74+
5975
def test_get_list_from_recorder(recorder):
6076
msg1, frm1 = 'm1', 'f1'
6177
msg2, frm2 = 'm2', 'f2'

scripts/test_zmq/test_zmq/zstack.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -556,7 +556,9 @@ def transmit(self, msg, uid, timeout=None, serialized=False, is_batch=False):
556556
@staticmethod
557557
def serializeMsg(msg):
558558
if isinstance(msg, Mapping):
559-
msg = json.dumps(msg)
559+
# Compact form: message size limits assume no separator
560+
# whitespace, as ujson emitted before it was removed.
561+
msg = json.dumps(msg, separators=(',', ':'))
560562
if isinstance(msg, str):
561563
msg = msg.encode()
562564
assert isinstance(msg, bytes)

stp_zmq/zstack.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -868,7 +868,9 @@ def transmit(self, msg, uid, timeout=None, serialized=False, is_batch=False):
868868
@staticmethod
869869
def serializeMsg(msg):
870870
if isinstance(msg, Mapping):
871-
msg = json.dumps(msg)
871+
# Compact form: message size limits assume no separator
872+
# whitespace, as ujson emitted before it was removed.
873+
msg = json.dumps(msg, separators=(',', ':'))
872874
if isinstance(msg, str):
873875
msg = msg.encode()
874876
assert isinstance(msg, bytes)

0 commit comments

Comments
 (0)