From 5d27e5c16cba8a6dd0730588cc4442abb0bb2da3 Mon Sep 17 00:00:00 2001 From: Tyson Smith Date: Wed, 27 Dec 2023 10:07:07 -0800 Subject: [PATCH 1/4] [replay] Remove --include-test No longer helpful and makes the default behavior unintuitive. --- grizzly/reduce/args.py | 2 +- grizzly/replay/args.py | 6 ------ grizzly/replay/replay.py | 4 +--- 3 files changed, 2 insertions(+), 10 deletions(-) diff --git a/grizzly/reduce/args.py b/grizzly/reduce/args.py index cd8cf778..7a54397a 100644 --- a/grizzly/reduce/args.py +++ b/grizzly/reduce/args.py @@ -24,7 +24,7 @@ def __init__(self): super().__init__() # these arguments have defaults that vary from ReplayCommonArgs - self.parser.set_defaults(include_test=True, logs=Path.cwd()) + self.parser.set_defaults(logs=Path.cwd()) reduce_args = self.parser.add_argument_group("Reduce Arguments") reduce_args.add_argument( diff --git a/grizzly/replay/args.py b/grizzly/replay/args.py index 6f1a2033..585dcd8a 100644 --- a/grizzly/replay/args.py +++ b/grizzly/replay/args.py @@ -57,12 +57,6 @@ def __init__(self): "--sig", type=Path, help="Signature (JSON) file to match." ) - self.reporter_grp.add_argument( - "--include-test", - action="store_true", - help="Include the testcase when reporting results.", - ) - def sanity_check(self, args): super().sanity_check(args) diff --git a/grizzly/replay/replay.py b/grizzly/replay/replay.py index f3b5ba22..54190509 100644 --- a/grizzly/replay/replay.py +++ b/grizzly/replay/replay.py @@ -708,9 +708,7 @@ def main(cls, args): if args.fuzzmanager: cls.report_to_fuzzmanager(results, testcases, args.tool) else: - cls.report_to_filesystem( - args.logs, results, testcases if args.include_test else None - ) + cls.report_to_filesystem(args.logs, results, testcases) return Exit.SUCCESS if success else Exit.FAILURE except ConfigError as exc: From 6db03c5b755d73fa88a7c43a47a15e58938a343d Mon Sep 17 00:00:00 2001 From: Tyson Smith Date: Fri, 29 Dec 2023 11:56:43 -0800 Subject: [PATCH 2/4] [replay] Update --test-index default Use last test by default when --no-harness is used. --- grizzly/reduce/args.py | 9 ++++++--- grizzly/reduce/test_main.py | 15 ++++++++++----- grizzly/replay/args.py | 7 +++++-- grizzly/replay/crash.py | 5 ++++- grizzly/replay/test_args.py | 9 +++++++-- grizzly/replay/test_main.py | 16 ++++++++-------- grizzly/replay/test_main_fm.py | 20 ++++++++++++++++++++ 7 files changed, 60 insertions(+), 21 deletions(-) diff --git a/grizzly/reduce/args.py b/grizzly/reduce/args.py index 7a54397a..ef4a6e69 100644 --- a/grizzly/reduce/args.py +++ b/grizzly/reduce/args.py @@ -112,7 +112,7 @@ class ReduceFuzzManagerIDArgs(ReduceCommonArgs): def __init__(self): """Initialize argument parser.""" super().__init__() - self.parser.add_argument("input", type=int, help="FuzzManager ID to replay") + self.parser.add_argument("input", type=int, help="FuzzManager ID to reduce") self.parser.add_argument( "--no-repro-quality", @@ -125,6 +125,7 @@ def __init__(self): self.parser.add_argument( "--test-index", + default=[], type=int, nargs="+", help="Select a testcase to run when multiple testcases are loaded. " @@ -135,8 +136,10 @@ def __init__(self): def sanity_check(self, args): super().sanity_check(args) - if args.no_harness and not args.test_index: - self.parser.error("'--no-harness' requires '--test-index'") + if args.no_harness and len(args.test_index) > 1: + self.parser.error( + "'--test-index' only supports a single value with '--no-harness'" + ) class ReduceFuzzManagerIDQualityArgs(ReduceFuzzManagerIDArgs): diff --git a/grizzly/reduce/test_main.py b/grizzly/reduce/test_main.py index 6b60132c..66d80f1b 100644 --- a/grizzly/reduce/test_main.py +++ b/grizzly/reduce/test_main.py @@ -72,8 +72,13 @@ def test_args_03(tmp_path, capsys): ReduceFuzzManagerIDArgs().parse_args([str(exe), "123"]) # error cases with raises(SystemExit): - ReduceFuzzManagerIDArgs().parse_args([str(exe), "123", "--no-harness"]) - assert "error: '--no-harness' requires '--test-index'" in capsys.readouterr()[-1] + ReduceFuzzManagerIDArgs().parse_args( + [str(exe), "123", "--no-harness", "--test-index", "0", "1"] + ) + assert ( + "error: '--test-index' only supports a single value with '--no-harness'" + in capsys.readouterr()[-1] + ) def test_args_04(tmp_path): @@ -169,7 +174,7 @@ def test_main_exit( relaunch=1, repeat=1, sig=sig, - test_index=None, + test_index=[], timeout=10, **kwargs ) @@ -243,7 +248,7 @@ def test_main_https_support(mocker, tmp_path, https_supported): relaunch=1, repeat=1, sig=None, - test_index=None, + test_index=[], use_http=False, time_limit=1, timeout=1, @@ -277,7 +282,7 @@ def test_main_load_assets_and_env(mocker, tmp_path): relaunch=1, repeat=1, sig=None, - test_index=None, + test_index=[], time_limit=1, timeout=1, ) diff --git a/grizzly/replay/args.py b/grizzly/replay/args.py index 585dcd8a..bfe51c60 100644 --- a/grizzly/replay/args.py +++ b/grizzly/replay/args.py @@ -118,6 +118,7 @@ def __init__(self): self.parser.add_argument( "--test-index", + default=[], type=int, nargs="+", help="Select a testcase to run when multiple testcases are loaded. " @@ -128,8 +129,10 @@ def __init__(self): def sanity_check(self, args): super().sanity_check(args) - if args.no_harness and not args.test_index: - self.parser.error("'--no-harness' requires '--test-index'") + if args.no_harness and len(args.test_index) > 1: + self.parser.error( + "'--test-index' only supports a single value with '--no-harness'" + ) class ReplayFuzzManagerIDQualityArgs(ReplayFuzzManagerIDArgs): diff --git a/grizzly/replay/crash.py b/grizzly/replay/crash.py index 702228a8..5c973c3d 100644 --- a/grizzly/replay/crash.py +++ b/grizzly/replay/crash.py @@ -52,11 +52,14 @@ def modify_args(args, crash, bucket): LOG.warning("Failed to generate signature from crash data: %s", exc) args.original_crash_id = args.input + # use the newest test case when not using a harness and test_index is not specified + if args.no_harness and not args.test_index: + args.test_index = [-1] args.input = crash.testcases(subset=args.test_index) + # set tool name using crash entry if args.tool is None: LOG.info("Setting default --tool=%s from CrashEntry", crash.tool) args.tool = crash.tool - # load signature if needed if bucket is not None: args.sig = bucket.signature_path() diff --git a/grizzly/replay/test_args.py b/grizzly/replay/test_args.py index b4fc669a..1f1a82a0 100644 --- a/grizzly/replay/test_args.py +++ b/grizzly/replay/test_args.py @@ -88,8 +88,13 @@ def test_replay_args_03(tmp_path, capsys): ReplayFuzzManagerIDArgs().parse_args([str(exe), "123"]) # error cases with raises(SystemExit): - ReplayFuzzManagerIDArgs().parse_args([str(exe), "123", "--no-harness"]) - assert "error: '--no-harness' requires '--test-index'" in capsys.readouterr()[-1] + ReplayFuzzManagerIDArgs().parse_args( + [str(exe), "123", "--no-harness", "--test-index", "0", "1"] + ) + assert ( + "error: '--test-index' only supports a single value with '--no-harness'" + in capsys.readouterr()[-1] + ) def test_replay_args_04(capsys, tmp_path): diff --git a/grizzly/replay/test_main.py b/grizzly/replay/test_main.py index 4d1d4547..6dd557a1 100644 --- a/grizzly/replay/test_main.py +++ b/grizzly/replay/test_main.py @@ -75,7 +75,7 @@ def test_main_01(mocker, server, tmp_path): repeat=4, rr=False, sig=tmp_path / "sig.json", - test_index=None, + test_index=[], time_limit=10, timeout=None, valgrind=False, @@ -148,7 +148,7 @@ def test_main_02(mocker, server, tmp_path, repro_results): repeat=1, rr=False, sig=tmp_path / "sig.json", - test_index=None, + test_index=[], time_limit=10, timeout=None, valgrind=False, @@ -212,7 +212,7 @@ def test_main_03(mocker, load_plugin, load_testcases, signature, result): repeat=1, rr=False, sig=signature, - test_index=None, + test_index=[], time_limit=10, timeout=None, valgrind=False, @@ -253,7 +253,7 @@ def test_main_04(mocker, tmp_path): repeat=1, rr=False, sig=None, - test_index=None, + test_index=[], time_limit=10, timeout=None, valgrind=False, @@ -309,7 +309,7 @@ def test_main_05(mocker, server, tmp_path): repeat=1, rr=False, sig=None, - test_index=None, + test_index=[], time_limit=1, timeout=None, valgrind=False, @@ -384,7 +384,7 @@ def test_main_06( repeat=1, rr=rr, sig=None, - test_index=None, + test_index=[], time_limit=10, timeout=None, valgrind=valgrind, @@ -436,7 +436,7 @@ def test_main_07(mocker, server, tmp_path): repeat=1, rr=False, sig=None, - test_index=None, + test_index=[], time_limit=10, timeout=None, tool=None, @@ -514,7 +514,7 @@ def test_main_09(mocker, server, tmp_path): repeat=1, sig=None, use_http=False, - test_index=None, + test_index=[], time_limit=1, timeout=1, ) diff --git a/grizzly/replay/test_main_fm.py b/grizzly/replay/test_main_fm.py index 41009fc1..27703a6c 100644 --- a/grizzly/replay/test_main_fm.py +++ b/grizzly/replay/test_main_fm.py @@ -59,6 +59,26 @@ def test_modify_args(tmp_path, mocker, arg_tool, signature): assert mod.sig == signature +@mark.parametrize( + "no_harness, org_index, expected", + [ + # no harness default + (True, [], [-1]), + # no harness user specified + (True, [1], [1]), + # harness default + (False, [], []), + # harness user specified + (False, [1], [1]), + ], +) +def test_modify_args_test_index(mocker, no_harness, org_index, expected): + """test modify_args()""" + args = mocker.Mock(no_harness=no_harness, test_index=org_index) + crash = mocker.Mock(spec=CrashEntry, tool="crash_tool") + assert modify_args(args, crash, None).test_index == expected + + @mark.parametrize( "crashes, main_exit_codes, result, arg_sig, arg_tool", [ From 6b8777e01bb3bdc23978e31cd2c581aa820499e6 Mon Sep 17 00:00:00 2001 From: Tyson Smith Date: Fri, 29 Dec 2023 15:12:05 -0800 Subject: [PATCH 3/4] Rename --logs to --output --- grizzly/args.py | 12 ++++++------ grizzly/main.py | 2 +- grizzly/reduce/args.py | 2 +- grizzly/reduce/core.py | 2 +- grizzly/replay/args.py | 6 +++--- grizzly/replay/replay.py | 4 ++-- grizzly/replay/test_args.py | 4 ++-- grizzly/replay/test_main.py | 31 +++++++++++++------------------ grizzly/test_args.py | 6 +++--- 9 files changed, 32 insertions(+), 37 deletions(-) diff --git a/grizzly/args.py b/grizzly/args.py index a28badb9..b7d7cd46 100644 --- a/grizzly/args.py +++ b/grizzly/args.py @@ -213,8 +213,8 @@ def __init__(self): " only applies to OOMs detected by Grizzly. (default: %(default)s)", ) self.reporter_grp.add_argument( - "-l", - "--logs", + "-o", + "--output", default=Path.cwd(), type=Path, help="Location to save logs and test cases. (default: %(default)s)", @@ -287,14 +287,14 @@ def sanity_check(self, args): self.parser.error("--log-limit must be >= 0") args.log_limit *= 1_048_576 - # if logs is specified, we need it to be a directory (whether existent or not) - if args.logs and args.logs.is_file(): - self.parser.error("--logs cannot be a file") - if args.memory < 0: self.parser.error("--memory must be >= 0") args.memory *= 1_048_576 + # if output is specified, it must be a directory (if it exists) + if args.output and args.output.is_file(): + self.parser.error("--output cannot be a file") + if args.no_harness: if args.time_limit is not None: self.parser.error("--time-limit cannot be used with --no-harness") diff --git a/grizzly/main.py b/grizzly/main.py index 3c8000a9..f4c21b1d 100644 --- a/grizzly/main.py +++ b/grizzly/main.py @@ -95,7 +95,7 @@ def main(args): reporter = FuzzManagerReporter(args.tool or f"grizzly-{adapter.name}") LOG.info("Results will be reported via FuzzManager (%s)", reporter.tool) else: - reporter = FilesystemReporter(args.logs / "results") + reporter = FilesystemReporter(args.output / "results") LOG.info("Results will be stored in '%s'", reporter.report_path) reporter.display_logs = args.smoke_test or reporter.display_logs diff --git a/grizzly/reduce/args.py b/grizzly/reduce/args.py index ef4a6e69..a64e6492 100644 --- a/grizzly/reduce/args.py +++ b/grizzly/reduce/args.py @@ -24,7 +24,7 @@ def __init__(self): super().__init__() # these arguments have defaults that vary from ReplayCommonArgs - self.parser.set_defaults(logs=Path.cwd()) + self.parser.set_defaults(output=Path.cwd()) reduce_args = self.parser.add_argument_group("Reduce Arguments") reduce_args.add_argument( diff --git a/grizzly/reduce/core.py b/grizzly/reduce/core.py index c74e35db..98f4c171 100644 --- a/grizzly/reduce/core.py +++ b/grizzly/reduce/core.py @@ -864,7 +864,7 @@ def main(cls, args): target, testcases, args.strategies, - args.logs, + args.output, any_crash=args.any_crash, expect_hang=expect_hang, idle_delay=args.idle_delay, diff --git a/grizzly/replay/args.py b/grizzly/replay/args.py index bfe51c60..31be6f40 100644 --- a/grizzly/replay/args.py +++ b/grizzly/replay/args.py @@ -14,7 +14,7 @@ class ReplayCommonArgs(CommonArgs): def __init__(self): super().__init__() - self.parser.set_defaults(logs=None) + self.parser.set_defaults(output=None) replay_args = self.parser.add_argument_group("Replay Arguments") replay_args.set_defaults(entry_point=None) @@ -66,8 +66,8 @@ def sanity_check(self, args): if args.idle_threshold and args.idle_delay <= 0: self.parser.error("--idle-delay value must be positive") - if args.logs is None and (args.pernosco or args.rr): - self.parser.error("--logs must be set when using rr") + if args.output is None and (args.pernosco or args.rr): + self.parser.error("--output must be set when using rr") if args.min_crashes < 1: self.parser.error("--min-crashes value must be positive") diff --git a/grizzly/replay/replay.py b/grizzly/replay/replay.py index 54190509..8ee89fdc 100644 --- a/grizzly/replay/replay.py +++ b/grizzly/replay/replay.py @@ -694,7 +694,7 @@ def main(cls, args): LOG.info("Results detected, signature does not match") else: LOG.info("No results detected") - if results and (args.logs or args.fuzzmanager): + if results and (args.output or args.fuzzmanager): # add target assets to test cases if not target.asset_mgr.is_empty(): for test in testcases: @@ -708,7 +708,7 @@ def main(cls, args): if args.fuzzmanager: cls.report_to_fuzzmanager(results, testcases, args.tool) else: - cls.report_to_filesystem(args.logs, results, testcases) + cls.report_to_filesystem(args.output, results, testcases) return Exit.SUCCESS if success else Exit.FAILURE except ConfigError as exc: diff --git a/grizzly/replay/test_args.py b/grizzly/replay/test_args.py index 1f1a82a0..51808779 100644 --- a/grizzly/replay/test_args.py +++ b/grizzly/replay/test_args.py @@ -47,10 +47,10 @@ def test_replay_args_01(capsys, mocker, tmp_path): (["--min-crashes", "0"], "error: --min-crashes value must be positive", 1), # test invalid repeat value (["--repeat", "-1"], "error: --repeat value must be positive", 1), - # test running with rr without --logs set + # test running with rr without --output set param( ["--rr"], - "error: --logs must be set when using rr", + "error: --output must be set when using rr", 1, marks=[mark.skipif(system() != "Linux", reason="Linux only")], ), diff --git a/grizzly/replay/test_main.py b/grizzly/replay/test_main.py index 6dd557a1..3d0fe6af 100644 --- a/grizzly/replay/test_main.py +++ b/grizzly/replay/test_main.py @@ -49,7 +49,7 @@ def test_main_01(mocker, server, tmp_path): load_target.return_value.return_value = target with TestCase("test.html", "adpt") as src: src.env_vars["TEST_VAR"] = "100" - src.add_from_bytes(b"test", "test.html") + src.add_from_bytes(b"test", src.entry_point) src.dump(tmp_path / "testcase", include_details=True) # setup args log_path = tmp_path / "logs" @@ -66,9 +66,9 @@ def test_main_01(mocker, server, tmp_path): ignore=["fake", "timeout"], input=[tmp_path / "testcase"], launch_attempts=3, - logs=log_path, min_crashes=2, no_harness=False, + output=log_path, pernosco=False, post_launch_delay=0, relaunch=1, @@ -139,9 +139,9 @@ def test_main_02(mocker, server, tmp_path, repro_results): ignore=[], input=[tmp_path / "test.html"], launch_attempts=3, - logs=tmp_path / "logs", min_crashes=2, no_harness=True, + output=tmp_path / "logs", pernosco=False, post_launch_delay=-1, relaunch=1, @@ -292,6 +292,9 @@ def test_main_05(mocker, server, tmp_path): ) asset = tmp_path / "sample_asset" asset.touch() + input_path = tmp_path / "input" + input_path.mkdir() + log_path = tmp_path / "logs" # setup args args = mocker.Mock( asset=[["from_cmdline", str(asset)]], @@ -300,9 +303,11 @@ def test_main_05(mocker, server, tmp_path): idle_delay=0, idle_threshold=0, ignore=[], + input=[input_path], launch_attempts=3, min_crashes=1, no_harness=True, + output=log_path, pernosco=False, post_launch_delay=-1, relaunch=1, @@ -314,17 +319,10 @@ def test_main_05(mocker, server, tmp_path): timeout=None, valgrind=False, ) - log_path = tmp_path / "logs" - args.logs = log_path - input_path = tmp_path / "input" - input_path.mkdir() # build a test case - entry_point = input_path / "test.html" - entry_point.touch() with TestCase("test.html", "test-adapter") as src: - src.add_from_file(entry_point) + src.add_from_bytes(b"test", src.entry_point) src.dump(input_path, include_details=True) - args.input = [input_path] with AssetManager(base_path=tmp_path) as asset_mgr: target.asset_mgr = asset_mgr assert ReplayManager.main(args) == Exit.SUCCESS @@ -414,7 +412,7 @@ def test_main_07(mocker, server, tmp_path): target.save_logs = _fake_save_logs load_target.return_value.return_value = target with TestCase("test.html", "adpt") as src: - src.add_from_bytes(b"test", "test.html") + src.add_from_bytes(b"test", src.entry_point) src.dump(tmp_path / "testcase", include_details=True) # setup args args = mocker.Mock( @@ -427,9 +425,9 @@ def test_main_07(mocker, server, tmp_path): ignore=[], input=[tmp_path / "testcase"], launch_attempts=1, - logs=None, min_crashes=1, no_harness=False, + output=None, pernosco=False, post_launch_delay=-1, relaunch=1, @@ -507,8 +505,8 @@ def test_main_09(mocker, server, tmp_path): idle_threshold=0, input=[input_path], launch_attempts=1, - logs=None, min_crashes=1, + output=None, post_launch_delay=-1, relaunch=1, repeat=1, @@ -518,9 +516,6 @@ def test_main_09(mocker, server, tmp_path): time_limit=1, timeout=1, ) - # build a test case - entry_point = input_path / "test.html" - entry_point.touch() # build test case and asset asset = tmp_path / "sample_asset" asset.touch() @@ -529,7 +524,7 @@ def test_main_09(mocker, server, tmp_path): with TestCase("test.html", "test-adapter") as src: src.assets = asset_mgr.assets src.assets_path = asset_mgr.path - src.add_from_file(entry_point) + src.add_from_bytes(b"", src.entry_point) src.dump(input_path, include_details=True) # this will load the previously created test case and asset from the filesystem try: diff --git a/grizzly/test_args.py b/grizzly/test_args.py index 2585439a..47a656c8 100644 --- a/grizzly/test_args.py +++ b/grizzly/test_args.py @@ -143,14 +143,14 @@ def test_common_args_03(capsys, mocker, tmp_path, args, msg, targets): def test_common_args_04(tmp_path): - """test CommonArgs.parse_args() '--logs' must be dir""" + """test CommonArgs.parse_args() '--output' must be dir""" fake_bin = tmp_path / "fake.bin" fake_bin.touch() # test with file with raises(SystemExit): - CommonArgs().parse_args([str(fake_bin), "--logs", str(fake_bin)]) + CommonArgs().parse_args([str(fake_bin), "--output", str(fake_bin)]) # test with dir - CommonArgs().parse_args([str(fake_bin), "--logs", str(tmp_path)]) + CommonArgs().parse_args([str(fake_bin), "--output", str(tmp_path)]) def test_common_args_05(mocker): From b4d6e1fcb93baa392f8adca91fcce0744ff13d7e Mon Sep 17 00:00:00 2001 From: Tyson Smith Date: Fri, 29 Dec 2023 15:14:49 -0800 Subject: [PATCH 4/4] Remove TestCase.landing_page --- grizzly/common/storage.py | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/grizzly/common/storage.py b/grizzly/common/storage.py index e4c0d2d4..7feb5bc2 100644 --- a/grizzly/common/storage.py +++ b/grizzly/common/storage.py @@ -350,22 +350,6 @@ def _find_entry_point(path): raise TestCaseLoadFailure("Could not determine entry point") return entry_point - @property - def landing_page(self): - """TestCase.landing_page is deprecated! - Should be replaced with TestCase.entry_point. - - Args: - None - - Returns: - str: TestCase.entry_point. - """ - LOG.warning( - "'TestCase.landing_page' deprecated, use 'TestCase.entry_point' in adapter" - ) - return self.entry_point - @classmethod def load(cls, path, entry_point=None, catalog=False): """Load a TestCase.