diff --git a/tools/performance_compare.py b/tools/performance_compare.py index 6a6c599351b0..07f21ebe3d2b 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)) @@ -181,8 +181,14 @@ 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) - except (OSError, ValueError, OverflowError) as error: + 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 4016f14439f1..b40de66ed39a 100644 --- a/tools/tests/test_performance_compare.py +++ b/tools/tests/test_performance_compare.py @@ -1,4 +1,6 @@ import copy +from functools import lru_cache +import io import json import os import subprocess @@ -27,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 _ 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() @@ -177,6 +192,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): + for opening, closing in (("[", "]"), ('{"nested":', "}")): + with self.subTest(opening=opening): + 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): + 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": ' + excessive_json_value(opening, closing) + "}" + path.write_text("\n\n" + record + "\n", encoding="utf-8") + original = (self.before.read_bytes(), self.after.read_bytes()) + 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("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} + for depth in range(16): + metadata = {"nested": [metadata]} + self.baseline["extra"] = metadata + self.write(self.before, self.baseline) + self.write(self.after, self.candidate) + 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(), "") + 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 self.candidate["gpu"] = "GPU | \nline"