Skip to content

Commit ebdbb3f

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 shared bytes-decoding default hook (plenum.common.util.json_default_bytes_to_str) 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 ebdbb3f

7 files changed

Lines changed: 100 additions & 4 deletions

File tree

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
"""
2+
Regression tests added when the ``ujson`` dependency was removed: pin the
3+
exact byte output of ``JsonSerializer`` (sorted keys, compact separators,
4+
raw UTF-8), since it feeds signing/hashing and must stay byte-stable.
5+
"""
6+
import pytest
7+
8+
from common.serializers.json_serializer import JsonSerializer
9+
10+
11+
@pytest.fixture
12+
def sz():
13+
return JsonSerializer()
14+
15+
16+
def test_keys_sorted_and_compact(sz):
17+
# Insertion order must not affect output; keys are sorted, no whitespace.
18+
assert sz.serialize({'b': 2, 'a': 1, 'c': 3}, toBytes=False) == '{"a":1,"b":2,"c":3}'
19+
20+
21+
def test_non_ascii_kept_raw_utf8(sz):
22+
# ensure_ascii=False: characters are emitted as raw UTF-8, not \\uXXXX.
23+
assert sz.serialize({'name': 'héllo', 'kr': '한글', 'emoji': '🚀'},
24+
toBytes=False) == '{"emoji":"🚀","kr":"한글","name":"héllo"}'
25+
# And the bytes form is the UTF-8 encoding of that string.
26+
assert sz.serialize({'name': 'héllo'}) == '{"name":"héllo"}'.encode('utf-8')
27+
28+
29+
def test_float_repr_pinned(sz):
30+
assert sz.serialize({'a': 14.8639, 'b': -97.466179, 'c': 1.0, 'd': 1e20},
31+
toBytes=False) == '{"a":14.8639,"b":-97.466179,"c":1.0,"d":1e+20}'
32+
33+
34+
def test_int_keys_become_strings(sz):
35+
# json coerces non-string keys to strings and then sorts lexicographically.
36+
assert sz.serialize({3: 'c', 1: 'a', 2: 'b'}, toBytes=False) == '{"1":"a","2":"b","3":"c"}'
37+
38+
39+
def test_bool_and_none(sz):
40+
assert sz.serialize({'t': True, 'f': False, 'n': None},
41+
toBytes=False) == '{"f":false,"n":null,"t":true}'
42+
43+
44+
def test_empty_dict(sz):
45+
assert sz.serialize({}, toBytes=False) == '{}'
46+
47+
48+
def test_top_level_bytes_base64(sz):
49+
# The OrderedJsonEncoder.encode override base64-encodes a top-level
50+
# bytes/bytearray value (b'raw' -> base64 'cmF3').
51+
assert sz.serialize(b'raw', toBytes=False) == '"cmF3"'
52+
assert sz.serialize(bytearray(b'raw'), toBytes=False) == '"cmF3"'
53+
54+
55+
def test_round_trip(sz):
56+
data = {'name': 'Alice', 'n': 1, 'f': 1.5, 'b': True, 'list': [1, 'two', None]}
57+
assert sz.deserialize(sz.serialize(data)) == data
58+
assert sz.deserialize(sz.serialize(data, toBytes=False)) == data
59+
60+
61+
@pytest.mark.parametrize('value', [
62+
{'z': b'raw'}, # bytes nested in a dict value
63+
[b'raw'], # bytes nested in a list
64+
{'z': bytearray(b'raw')},
65+
])
66+
def test_nested_bytes_raise_typeerror(sz, value):
67+
"""
68+
The bytes special-case in ``OrderedJsonEncoder.encode`` only fires for a
69+
top-level value; bytes nested in a container raise TypeError. A fix, if
70+
ever needed, belongs in ``OrderedJsonEncoder.default``.
71+
"""
72+
with pytest.raises(TypeError):
73+
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/common/util.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,14 @@ def z85_to_friendly(z):
342342
return z
343343

344344

345+
def json_default_bytes_to_str(obj):
346+
"""``json.dumps`` default hook decoding bytes/bytearray as UTF-8."""
347+
if isinstance(obj, (bytes, bytearray)):
348+
return obj.decode()
349+
raise TypeError('Object of type {} is not JSON '
350+
'serializable'.format(type(obj).__name__))
351+
352+
345353
def runWithLoop(loop, callback, *args, **kwargs):
346354
if loop.is_running():
347355
loop.call_soon(asyncio.ensure_future, callback(*args, **kwargs))

plenum/recorder/recorder.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import json
44
from typing import Callable
55

6+
from plenum.common.util import json_default_bytes_to_str
67
from storage.kv_store_rocksdb_int_keys import KeyValueStorageRocksdbIntKeys
78

89

@@ -46,7 +47,8 @@ def add_to_store(self, key, val):
4647
existing = json.loads(existing)
4748
except KeyError:
4849
existing = []
49-
self.store.put(key, json.dumps([*existing, val]))
50+
self.store.put(key, json.dumps([*existing, val],
51+
default=json_default_bytes_to_str))
5052

5153
def register_replay_target(self, id, target: Callable):
5254
assert id not in self.replay_targets

plenum/test/recorder/test_recorder.py

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

5858

59+
def test_add_to_recorder_with_bytes(recorder):
60+
msg1, frm1 = b'{"msg": "m1"}', b'f1'
61+
msg2, to1 = b'{"msg": "m2"}', 't1'
62+
recorder.add_incoming(msg1, frm1)
63+
recorder.add_outgoing(msg2, to1)
64+
65+
all_msgs = []
66+
for _, v in recorder.store.iterator(include_value=True):
67+
all_msgs.extend(Recorder.get_parsed(v))
68+
assert Recorder.filter_incoming(all_msgs) == [['{"msg": "m1"}', 'f1']]
69+
assert Recorder.filter_outgoing(all_msgs) == [['{"msg": "m2"}', 't1']]
70+
71+
5972
def test_get_list_from_recorder(recorder):
6073
msg1, frm1 = 'm1', 'f1'
6174
msg2, frm2 = 'm2', 'f2'

scripts/test_zmq/test_zmq/zstack.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -556,7 +556,7 @@ 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+
msg = json.dumps(msg, separators=(',', ':'))
560560
if isinstance(msg, str):
561561
msg = msg.encode()
562562
assert isinstance(msg, bytes)

stp_zmq/zstack.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -868,7 +868,7 @@ 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+
msg = json.dumps(msg, separators=(',', ':'))
872872
if isinstance(msg, str):
873873
msg = msg.encode()
874874
assert isinstance(msg, bytes)

0 commit comments

Comments
 (0)