From 4be5952b8384387dc0ec86183eae4e3b6c35d115 Mon Sep 17 00:00:00 2001 From: Bram910 Date: Sat, 24 Oct 2026 13:32:46 +0200 Subject: [PATCH 1/3] fix(tools): report excessive summary JSON nesting --- tools/performance_compare.py | 4 +- tools/tests/test_performance_compare.py | 53 +++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/tools/performance_compare.py b/tools/performance_compare.py index 6a6c599351b0..429a0858ba7a 100644 --- a/tools/performance_compare.py +++ b/tools/performance_compare.py @@ -73,7 +73,7 @@ def load_summary(path, trace=None): record = json.loads(line) if not isinstance(record, dict): raise ValueError("summary must be an object") - except ValueError as error: + except (ValueError, RecursionError) as error: raise ValueError(f"{path}:{line_number}: {error}") from error if trace is None or record.get("trace") == trace: records.append((line_number, record)) @@ -182,7 +182,7 @@ def main(argv=None): if args.require_matching_context and report["configuration_differences"]: raise ValueError("recorded configuration differs: " + ", ".join(report["configuration_differences"])) output = json.dumps(report, indent=2, ensure_ascii=False, allow_nan=False) + "\n" if args.json else markdown(report) - except (OSError, ValueError, OverflowError) as error: + except (OSError, ValueError, OverflowError, RecursionError) as error: parser.exit(2, f"{parser.prog}: {error}\n") sys.stdout.write(output) diff --git a/tools/tests/test_performance_compare.py b/tools/tests/test_performance_compare.py index 4016f14439f1..a5079fe48dc0 100644 --- a/tools/tests/test_performance_compare.py +++ b/tools/tests/test_performance_compare.py @@ -1,4 +1,5 @@ import copy +import io import json import os import subprocess @@ -177,6 +178,58 @@ def test_percentage_overflow_is_an_error(self): self.assertEqual(result.stdout, "") self.assertIn("finite numeric range", result.stderr) + def test_excessive_json_nesting_reports_the_input_location(self): + depth = 100000 + for opening, closing in (("[", "]"), ('{"nested":', "}")): + with self.subTest(opening=opening): + record = json.dumps(self.baseline)[:-1] + ', "extra": ' + opening * depth + "0" + closing * depth + "}" + self.before.write_text("\n\n" + record + "\n", encoding="utf-8") + with self.assertRaisesRegex(ValueError, r"before.jsonl:3:") as error: + performance_compare.load_summary(self.before) + self.assertIsInstance(error.exception.__cause__, RecursionError) + + def test_excessive_json_nesting_produces_only_a_cli_error(self): + depth = 100000 + for path in (self.before, self.after): + for opening, closing in (("[", "]"), ('{"nested":', "}")): + self.write(self.before, self.baseline) + self.write(self.after, self.candidate) + record = json.dumps(self.baseline)[:-1] + ', "extra": ' + opening * depth + "0" + closing * depth + "}" + path.write_text("\n\n" + record + "\n", encoding="utf-8") + original = (self.before.read_bytes(), self.after.read_bytes()) + for args in ((), ("--json",)): + with self.subTest(path=path.name, opening=opening, args=args): + result = self.run_cli(*args) + self.assertEqual(result.returncode, 2, result.stderr) + self.assertEqual(result.stdout, "") + self.assertIn(f"{path}:3:", result.stderr) + self.assertNotIn("Traceback", result.stderr) + self.assertEqual((self.before.read_bytes(), self.after.read_bytes()), original) + + def test_json_report_recursion_produces_only_a_cli_error(self): + self.write(self.before, self.baseline) + self.write(self.after, self.candidate) + with patch.object(performance_compare.json, "dumps", side_effect=RecursionError("report nesting is too deep")), \ + patch.object(sys, "stdout", new_callable=io.StringIO) as output, \ + patch.object(sys, "stderr", new_callable=io.StringIO) as errors: + with self.assertRaises(SystemExit) as error: + performance_compare.main([str(self.before), str(self.after), "--json"]) + self.assertEqual(error.exception.code, 2) + self.assertEqual(output.getvalue(), "") + self.assertIn("report nesting is too deep", errors.getvalue()) + + def test_supported_nested_metadata_is_preserved(self): + metadata = {"value": 7} + for depth in range(16): + metadata = {"nested": [metadata]} + self.baseline["extra"] = metadata + self.write(self.before, self.baseline) + self.write(self.after, self.candidate) + result = self.run_cli("--json") + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(json.loads(result.stdout)["baseline"]["extra"], metadata) + self.assertEqual(self.run_cli().returncode, 0) + def test_markdown_reports_deltas_and_escapes_table_values(self): self.candidate["frame_ms"]["max"] = 60.0 self.candidate["gpu"] = "GPU | \nline" From b1db8bb5fdbe901091f403ac9cb14e71612d46d2 Mon Sep 17 00:00:00 2001 From: Bram910 Date: Sat, 24 Oct 2026 14:45:54 +0200 Subject: [PATCH 2/3] test(tools): bound summary nesting regression workload --- tools/tests/test_performance_compare.py | 53 ++++++++++++++++--------- 1 file changed, 35 insertions(+), 18 deletions(-) diff --git a/tools/tests/test_performance_compare.py b/tools/tests/test_performance_compare.py index a5079fe48dc0..67781ba46b01 100644 --- a/tools/tests/test_performance_compare.py +++ b/tools/tests/test_performance_compare.py @@ -1,4 +1,5 @@ import copy +from functools import lru_cache import io import json import os @@ -28,6 +29,19 @@ "title_id": "PPSA00000", "title": "Example", "content_version": "01.002.000"} +@lru_cache(maxsize=2) +def excessive_json_value(opening, closing): + depth = sys.getrecursionlimit() + 64 + for attempt in range(8): + value = opening * depth + "0" + closing * depth + try: + json.loads(value) + except RecursionError: + return opening * (depth + 128) + "0" + closing * (depth + 128) + depth *= 2 + raise AssertionError("JSON decoder accepted every bounded nesting probe") + + class PerformanceCompareTests(unittest.TestCase): def setUp(self): self.directory = tempfile.TemporaryDirectory() @@ -179,32 +193,30 @@ def test_percentage_overflow_is_an_error(self): self.assertIn("finite numeric range", result.stderr) def test_excessive_json_nesting_reports_the_input_location(self): - depth = 100000 for opening, closing in (("[", "]"), ('{"nested":', "}")): with self.subTest(opening=opening): - record = json.dumps(self.baseline)[:-1] + ', "extra": ' + opening * depth + "0" + closing * depth + "}" + record = json.dumps(self.baseline)[:-1] + ', "extra": ' + excessive_json_value(opening, closing) + "}" self.before.write_text("\n\n" + record + "\n", encoding="utf-8") with self.assertRaisesRegex(ValueError, r"before.jsonl:3:") as error: performance_compare.load_summary(self.before) self.assertIsInstance(error.exception.__cause__, RecursionError) def test_excessive_json_nesting_produces_only_a_cli_error(self): - depth = 100000 - for path in (self.before, self.after): - for opening, closing in (("[", "]"), ('{"nested":', "}")): + cases = ((self.before, "[", "]", ()), (self.before, '{"nested":', "}", ("--json",)), + (self.after, "[", "]", ("--json",)), (self.after, '{"nested":', "}", ())) + for path, opening, closing, args in cases: + with self.subTest(path=path.name, opening=opening, args=args): self.write(self.before, self.baseline) self.write(self.after, self.candidate) - record = json.dumps(self.baseline)[:-1] + ', "extra": ' + opening * depth + "0" + closing * depth + "}" + record = json.dumps(self.baseline)[:-1] + ', "extra": ' + excessive_json_value(opening, closing) + "}" path.write_text("\n\n" + record + "\n", encoding="utf-8") original = (self.before.read_bytes(), self.after.read_bytes()) - for args in ((), ("--json",)): - with self.subTest(path=path.name, opening=opening, args=args): - result = self.run_cli(*args) - self.assertEqual(result.returncode, 2, result.stderr) - self.assertEqual(result.stdout, "") - self.assertIn(f"{path}:3:", result.stderr) - self.assertNotIn("Traceback", result.stderr) - self.assertEqual((self.before.read_bytes(), self.after.read_bytes()), original) + result = self.run_cli(*args) + self.assertEqual(result.returncode, 2, result.stderr) + self.assertEqual(result.stdout, "") + self.assertIn(f"{path}:3:", result.stderr) + self.assertNotIn("Traceback", result.stderr) + self.assertEqual((self.before.read_bytes(), self.after.read_bytes()), original) def test_json_report_recursion_produces_only_a_cli_error(self): self.write(self.before, self.baseline) @@ -225,10 +237,15 @@ def test_supported_nested_metadata_is_preserved(self): self.baseline["extra"] = metadata self.write(self.before, self.baseline) self.write(self.after, self.candidate) - result = self.run_cli("--json") - self.assertEqual(result.returncode, 0, result.stderr) - self.assertEqual(json.loads(result.stdout)["baseline"]["extra"], metadata) - self.assertEqual(self.run_cli().returncode, 0) + for args in ((), ("--json",)): + with self.subTest(args=args), patch.object(sys, "stdout", new_callable=io.StringIO) as output, \ + patch.object(sys, "stderr", new_callable=io.StringIO) as errors: + performance_compare.main([str(self.before), str(self.after), *args]) + self.assertEqual(errors.getvalue(), "") + if args: + self.assertEqual(json.loads(output.getvalue())["baseline"]["extra"], metadata) + else: + self.assertIn("# Performance comparison", output.getvalue()) def test_markdown_reports_deltas_and_escapes_table_values(self): self.candidate["frame_ms"]["max"] = 60.0 From 8ffac634b69dd595e391403ff1e47484a3c45680 Mon Sep 17 00:00:00 2001 From: Bram910 Date: Sat, 24 Oct 2026 21:33:51 +0200 Subject: [PATCH 3/3] fix(tools): identify JSON report recursion errors --- tools/performance_compare.py | 8 +++++++- tools/tests/test_performance_compare.py | 17 +++++++---------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/tools/performance_compare.py b/tools/performance_compare.py index 429a0858ba7a..07f21ebe3d2b 100644 --- a/tools/performance_compare.py +++ b/tools/performance_compare.py @@ -181,7 +181,13 @@ def main(argv=None): report = compare(load_summary(args.baseline, args.baseline_trace), load_summary(args.candidate, args.candidate_trace)) if args.require_matching_context and report["configuration_differences"]: raise ValueError("recorded configuration differs: " + ", ".join(report["configuration_differences"])) - output = json.dumps(report, indent=2, ensure_ascii=False, allow_nan=False) + "\n" if args.json else markdown(report) + if args.json: + try: + output = json.dumps(report, indent=2, ensure_ascii=False, allow_nan=False) + "\n" + except RecursionError as error: + raise ValueError(f"JSON report: {error}") from error + else: + output = markdown(report) except (OSError, ValueError, OverflowError, RecursionError) as error: parser.exit(2, f"{parser.prog}: {error}\n") sys.stdout.write(output) diff --git a/tools/tests/test_performance_compare.py b/tools/tests/test_performance_compare.py index 67781ba46b01..b40de66ed39a 100644 --- a/tools/tests/test_performance_compare.py +++ b/tools/tests/test_performance_compare.py @@ -32,7 +32,7 @@ @lru_cache(maxsize=2) def excessive_json_value(opening, closing): depth = sys.getrecursionlimit() + 64 - for attempt in range(8): + for _ in range(8): value = opening * depth + "0" + closing * depth try: json.loads(value) @@ -228,7 +228,8 @@ def test_json_report_recursion_produces_only_a_cli_error(self): performance_compare.main([str(self.before), str(self.after), "--json"]) self.assertEqual(error.exception.code, 2) self.assertEqual(output.getvalue(), "") - self.assertIn("report nesting is too deep", errors.getvalue()) + self.assertIn("JSON report: report nesting is too deep", errors.getvalue()) + self.assertNotIn("Traceback", errors.getvalue()) def test_supported_nested_metadata_is_preserved(self): metadata = {"value": 7} @@ -237,15 +238,11 @@ def test_supported_nested_metadata_is_preserved(self): self.baseline["extra"] = metadata self.write(self.before, self.baseline) self.write(self.after, self.candidate) - for args in ((), ("--json",)): - with self.subTest(args=args), patch.object(sys, "stdout", new_callable=io.StringIO) as output, \ - patch.object(sys, "stderr", new_callable=io.StringIO) as errors: - performance_compare.main([str(self.before), str(self.after), *args]) + with patch.object(sys, "stdout", new_callable=io.StringIO) as output, \ + patch.object(sys, "stderr", new_callable=io.StringIO) as errors: + performance_compare.main([str(self.before), str(self.after), "--json"]) self.assertEqual(errors.getvalue(), "") - if args: - self.assertEqual(json.loads(output.getvalue())["baseline"]["extra"], metadata) - else: - self.assertIn("# Performance comparison", output.getvalue()) + self.assertEqual(json.loads(output.getvalue())["baseline"]["extra"], metadata) def test_markdown_reports_deltas_and_escapes_table_values(self): self.candidate["frame_ms"]["max"] = 60.0