From a18d5f01cf4417670c28b5154f21dbeb8359bd3f Mon Sep 17 00:00:00 2001 From: Ijtaba Hussain Date: Mon, 3 Apr 2023 22:29:02 +0500 Subject: [PATCH 1/8] Suppress warnings for test cases for unsafe input The asserted result is the None value (unsafe input rejected by mailcap) --- Lib/test/test_mailcap.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_mailcap.py b/Lib/test/test_mailcap.py index 819dc80a266433..6765095c89239f 100644 --- a/Lib/test/test_mailcap.py +++ b/Lib/test/test_mailcap.py @@ -127,14 +127,18 @@ def test_subst(self): (["", "audio/*", "foo.txt"], ""), (["echo foo", "audio/*", "foo.txt"], "echo foo"), (["echo %s", "audio/*", "foo.txt"], "echo foo.txt"), - (["echo %t", "audio/*", "foo.txt"], None), + (["echo %t", "audio/*", "foo.txt"], None, {"warn_type": mailcap.UnsafeMailcapInput}), (["echo %t", "audio/wav", "foo.txt"], "echo audio/wav"), (["echo \\%t", "audio/*", "foo.txt"], "echo %t"), (["echo foo", "audio/*", "foo.txt", plist], "echo foo"), (["echo %{total}", "audio/*", "foo.txt", plist], "echo 3") ] for tc in test_cases: - self.assertEqual(mailcap.subst(*tc[0]), tc[1]) + if len(tc) == 3: + with warnings_helper.check_warnings(('', tc[2]["warn_type"]), quiet=True): + self.assertEqual(mailcap.subst(*tc[0]), tc[1]) + else: + self.assertEqual(mailcap.subst(*tc[0]), tc[1]) class GetcapsTest(unittest.TestCase): @@ -212,7 +216,8 @@ def test_findmatch(self): ('"An audio fragment"', audio_basic_entry)), ([c, "audio/*"], {"filename": fname}, - (None, None)), + (None, None), + {"warn_type": mailcap.UnsafeMailcapInput}), ([c, "audio/wav"], {"filename": fname}, ("/usr/local/bin/showaudio audio/wav", audio_entry)), @@ -247,7 +252,11 @@ def test_test(self): def _run_cases(self, cases): for c in cases: - self.assertEqual(mailcap.findmatch(*c[0], **c[1]), c[2]) + if len(c) == 4: + with warnings_helper.check_warnings(('', c[3]["warn_type"]), quiet=True): + self.assertEqual(mailcap.findmatch(*c[0], **c[1]), c[2]) + else: + self.assertEqual(mailcap.findmatch(*c[0], **c[1]), c[2]) if __name__ == '__main__': From f412f07d1eac0920563c363a21664bd790a82042 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Thu, 6 Apr 2023 05:36:25 +0400 Subject: [PATCH 2/8] Run patchcheck.py --- Lib/test/test_mailcap.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_mailcap.py b/Lib/test/test_mailcap.py index 6765095c89239f..bb59f5b484cf53 100644 --- a/Lib/test/test_mailcap.py +++ b/Lib/test/test_mailcap.py @@ -255,7 +255,7 @@ def _run_cases(self, cases): if len(c) == 4: with warnings_helper.check_warnings(('', c[3]["warn_type"]), quiet=True): self.assertEqual(mailcap.findmatch(*c[0], **c[1]), c[2]) - else: + else: self.assertEqual(mailcap.findmatch(*c[0], **c[1]), c[2]) From 605d93f41ec0d2dec020e18f58f292e5657a8b8b Mon Sep 17 00:00:00 2001 From: Ijtaba Hussain Date: Thu, 6 Apr 2023 11:13:45 +0500 Subject: [PATCH 3/8] Rework unsafe input warnings into a new test --- Lib/test/test_mailcap.py | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/Lib/test/test_mailcap.py b/Lib/test/test_mailcap.py index bb59f5b484cf53..4eddcade97c3a4 100644 --- a/Lib/test/test_mailcap.py +++ b/Lib/test/test_mailcap.py @@ -127,18 +127,13 @@ def test_subst(self): (["", "audio/*", "foo.txt"], ""), (["echo foo", "audio/*", "foo.txt"], "echo foo"), (["echo %s", "audio/*", "foo.txt"], "echo foo.txt"), - (["echo %t", "audio/*", "foo.txt"], None, {"warn_type": mailcap.UnsafeMailcapInput}), (["echo %t", "audio/wav", "foo.txt"], "echo audio/wav"), (["echo \\%t", "audio/*", "foo.txt"], "echo %t"), (["echo foo", "audio/*", "foo.txt", plist], "echo foo"), (["echo %{total}", "audio/*", "foo.txt", plist], "echo 3") ] for tc in test_cases: - if len(tc) == 3: - with warnings_helper.check_warnings(('', tc[2]["warn_type"]), quiet=True): - self.assertEqual(mailcap.subst(*tc[0]), tc[1]) - else: - self.assertEqual(mailcap.subst(*tc[0]), tc[1]) + self.assertEqual(mailcap.subst(*tc[0]), tc[1]) class GetcapsTest(unittest.TestCase): @@ -214,10 +209,6 @@ def test_findmatch(self): ([c, "audio/basic"], {"key": "description", "filename": fname}, ('"An audio fragment"', audio_basic_entry)), - ([c, "audio/*"], - {"filename": fname}, - (None, None), - {"warn_type": mailcap.UnsafeMailcapInput}), ([c, "audio/wav"], {"filename": fname}, ("/usr/local/bin/showaudio audio/wav", audio_entry)), @@ -250,13 +241,22 @@ def test_test(self): ] self._run_cases(cases) + def test_unsafe_mailcap_input(self): + c = MAILCAPDICT + plist = ["total=*"] + + with self.assertWarnsRegex(mailcap.UnsafeMailcapInput,'Refusing to substitute parameter.*into a shell command'): + self.assertEqual(mailcap.subst("echo %{total}", "audio/wav", "foo.txt", plist), None) + + with self.assertWarnsRegex(mailcap.UnsafeMailcapInput,'Refusing to substitute MIME type.*into a shell'): + self.assertEqual(mailcap.subst("echo %t", "audio/*", "foo.txt"), None) + + with self.assertWarnsRegex(mailcap.UnsafeMailcapInput,'Refusing to use mailcap with filename.*Use a safe temporary filename.'): + self.assertEqual(mailcap.findmatch(c, "audio/wav", filename="foo*.txt"), (None, None)) + def _run_cases(self, cases): for c in cases: - if len(c) == 4: - with warnings_helper.check_warnings(('', c[3]["warn_type"]), quiet=True): - self.assertEqual(mailcap.findmatch(*c[0], **c[1]), c[2]) - else: - self.assertEqual(mailcap.findmatch(*c[0], **c[1]), c[2]) + self.assertEqual(mailcap.findmatch(*c[0], **c[1]), c[2]) if __name__ == '__main__': From 12e8373475803a9b3fc79c9cadcca07fc46dd13f Mon Sep 17 00:00:00 2001 From: Ijtaba Hussain Date: Thu, 6 Apr 2023 20:55:42 +0500 Subject: [PATCH 4/8] Style improvements --- Lib/test/test_mailcap.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/Lib/test/test_mailcap.py b/Lib/test/test_mailcap.py index 4eddcade97c3a4..63bbf2d4d67921 100644 --- a/Lib/test/test_mailcap.py +++ b/Lib/test/test_mailcap.py @@ -242,17 +242,20 @@ def test_test(self): self._run_cases(cases) def test_unsafe_mailcap_input(self): - c = MAILCAPDICT plist = ["total=*"] - with self.assertWarnsRegex(mailcap.UnsafeMailcapInput,'Refusing to substitute parameter.*into a shell command'): + with self.assertWarnsRegex(mailcap.UnsafeMailcapInput, + 'Refusing to substitute parameter.*into a shell command'): self.assertEqual(mailcap.subst("echo %{total}", "audio/wav", "foo.txt", plist), None) - - with self.assertWarnsRegex(mailcap.UnsafeMailcapInput,'Refusing to substitute MIME type.*into a shell'): + + with self.assertWarnsRegex(mailcap.UnsafeMailcapInput, + 'Refusing to substitute MIME type.*into a shell'): self.assertEqual(mailcap.subst("echo %t", "audio/*", "foo.txt"), None) - - with self.assertWarnsRegex(mailcap.UnsafeMailcapInput,'Refusing to use mailcap with filename.*Use a safe temporary filename.'): - self.assertEqual(mailcap.findmatch(c, "audio/wav", filename="foo*.txt"), (None, None)) + + with self.assertWarnsRegex(mailcap.UnsafeMailcapInput, + 'Refusing to use mailcap with filename.*Use a safe temporary filename.'): + self.assertEqual(mailcap.findmatch(MAILCAPDICT, + "audio/wav", filename="foo*.txt"), (None, None)) def _run_cases(self, cases): for c in cases: From 80d0ef7ee6caf4d169a92a26619732c2fdd1193f Mon Sep 17 00:00:00 2001 From: Ijtaba Hussain Date: Thu, 6 Apr 2023 21:40:23 +0500 Subject: [PATCH 5/8] Formatting improvements Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> --- Lib/test/test_mailcap.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Lib/test/test_mailcap.py b/Lib/test/test_mailcap.py index 63bbf2d4d67921..4348cbf41880c9 100644 --- a/Lib/test/test_mailcap.py +++ b/Lib/test/test_mailcap.py @@ -243,7 +243,6 @@ def test_test(self): def test_unsafe_mailcap_input(self): plist = ["total=*"] - with self.assertWarnsRegex(mailcap.UnsafeMailcapInput, 'Refusing to substitute parameter.*into a shell command'): self.assertEqual(mailcap.subst("echo %{total}", "audio/wav", "foo.txt", plist), None) From 8bd668d8c7997a8c94ed736aaaea8480080e478d Mon Sep 17 00:00:00 2001 From: Ijtaba Hussain Date: Thu, 6 Apr 2023 21:46:15 +0500 Subject: [PATCH 6/8] Further formatting improvements --- Lib/test/test_mailcap.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_mailcap.py b/Lib/test/test_mailcap.py index 4348cbf41880c9..0156d58088d2f6 100644 --- a/Lib/test/test_mailcap.py +++ b/Lib/test/test_mailcap.py @@ -253,8 +253,8 @@ def test_unsafe_mailcap_input(self): with self.assertWarnsRegex(mailcap.UnsafeMailcapInput, 'Refusing to use mailcap with filename.*Use a safe temporary filename.'): - self.assertEqual(mailcap.findmatch(MAILCAPDICT, - "audio/wav", filename="foo*.txt"), (None, None)) + unsafe_filename = mailcap.findmatch(MAILCAPDICT,"audio/wav", filename="foo*.txt") + self.assertEqual(unsafe_filename, (None, None)) def _run_cases(self, cases): for c in cases: From 820564dbba474394853f9e55b5f62978d573607e Mon Sep 17 00:00:00 2001 From: Ijtaba Hussain Date: Thu, 6 Apr 2023 21:54:49 +0500 Subject: [PATCH 7/8] Formatting improvements --- Lib/test/test_mailcap.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_mailcap.py b/Lib/test/test_mailcap.py index 0156d58088d2f6..fb9054399f6ea1 100644 --- a/Lib/test/test_mailcap.py +++ b/Lib/test/test_mailcap.py @@ -242,10 +242,10 @@ def test_test(self): self._run_cases(cases) def test_unsafe_mailcap_input(self): - plist = ["total=*"] with self.assertWarnsRegex(mailcap.UnsafeMailcapInput, 'Refusing to substitute parameter.*into a shell command'): - self.assertEqual(mailcap.subst("echo %{total}", "audio/wav", "foo.txt", plist), None) + unsafe_param = mailcap.subst("echo %{total}", "audio/wav", "foo.txt", ["total=*"]) + self.assertEqual(unsafe_param, None) with self.assertWarnsRegex(mailcap.UnsafeMailcapInput, 'Refusing to substitute MIME type.*into a shell'): From bf5f22d3ac83ae4de838b9b8a969d9b45ac42149 Mon Sep 17 00:00:00 2001 From: Ijtaba Hussain Date: Thu, 6 Apr 2023 21:59:16 +0500 Subject: [PATCH 8/8] Trim new test function code to less than 80 chars[F --- Lib/test/test_mailcap.py | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/Lib/test/test_mailcap.py b/Lib/test/test_mailcap.py index fb9054399f6ea1..8a94b0cb1f27c7 100644 --- a/Lib/test/test_mailcap.py +++ b/Lib/test/test_mailcap.py @@ -243,17 +243,26 @@ def test_test(self): def test_unsafe_mailcap_input(self): with self.assertWarnsRegex(mailcap.UnsafeMailcapInput, - 'Refusing to substitute parameter.*into a shell command'): - unsafe_param = mailcap.subst("echo %{total}", "audio/wav", "foo.txt", ["total=*"]) + 'Refusing to substitute parameter.*' + 'into a shell command'): + unsafe_param = mailcap.subst("echo %{total}", + "audio/wav", + "foo.txt", + ["total=*"]) self.assertEqual(unsafe_param, None) with self.assertWarnsRegex(mailcap.UnsafeMailcapInput, - 'Refusing to substitute MIME type.*into a shell'): - self.assertEqual(mailcap.subst("echo %t", "audio/*", "foo.txt"), None) + 'Refusing to substitute MIME type' + '.*into a shell'): + unsafe_mimetype = mailcap.subst("echo %t", "audio/*", "foo.txt") + self.assertEqual(unsafe_mimetype, None) with self.assertWarnsRegex(mailcap.UnsafeMailcapInput, - 'Refusing to use mailcap with filename.*Use a safe temporary filename.'): - unsafe_filename = mailcap.findmatch(MAILCAPDICT,"audio/wav", filename="foo*.txt") + 'Refusing to use mailcap with filename.*' + 'Use a safe temporary filename.'): + unsafe_filename = mailcap.findmatch(MAILCAPDICT, + "audio/wav", + filename="foo*.txt") self.assertEqual(unsafe_filename, (None, None)) def _run_cases(self, cases):