From 148bf3749d9c12d55017bbd9556bb0529af6de35 Mon Sep 17 00:00:00 2001 From: Jeff Allen Date: Sat, 27 Jan 2024 09:19:25 +0000 Subject: [PATCH] Fully reject writes to read-only members of sys (#299) Prior to this change it was possible to sneak a write (or delete) past the checks in sys.__setattr__ and sys.__delattr__ because those checks assumed names would be interned. Strengthens and re-enables test_sys_jy. Fixes #238. --- Lib/test/regrtest.py | 1 - Lib/test/test_sys_jy.py | 44 +++++++++++++++++++++++--- src/org/python/core/PySystemState.java | 30 +++++++++++++----- 3 files changed, 62 insertions(+), 13 deletions(-) diff --git a/Lib/test/regrtest.py b/Lib/test/regrtest.py index 2d675a7e1..f2fb6d42c 100644 --- a/Lib/test/regrtest.py +++ b/Lib/test/regrtest.py @@ -1345,7 +1345,6 @@ def skip_conditional_support(test_module,module_name): # fails on Windows standalone too, but more embarassing as java specific test_subprocess_jy - test_sys_jy # OSError handling wide-character filename test_asyncore test_compileall diff --git a/Lib/test/test_sys_jy.py b/Lib/test/test_sys_jy.py index 2545730de..9dcca2753 100644 --- a/Lib/test/test_sys_jy.py +++ b/Lib/test/test_sys_jy.py @@ -7,7 +7,7 @@ import tempfile import unittest from test import test_support -from test.test_support import is_jython, is_jython_nt +from test.test_support import is_jython, is_jython_nt, is_jython_posix class SysTest(unittest.TestCase): @@ -41,19 +41,50 @@ def test_name(self): self.assert_('foo' not in str(sys)) sys.__name__ = 'sys' - def test_readonly(self): + def test_readonly_delete(self): def deleteClass(): del sys.__class__ self.assertRaises(TypeError, deleteClass) def deleteDict(): del sys.__dict__ self.assertRaises(TypeError, deleteDict) + def deleteBuiltins(): del sys.builtins + self.assertRaises(TypeError, deleteBuiltins) + + def deletePrefix(): del sys.exec_prefix + self.assertRaises(TypeError, deletePrefix) + + def deleteManager(): del sys.packageManager + self.assertRaises(TypeError, deleteManager) + + def deleteRegistry(): del sys.registry + self.assertRaises(TypeError, deleteRegistry) + + def deleteWarn(): del sys.warnoptions + self.assertRaises(TypeError, deleteWarn) + + def deletePrefix2(): sys.__delattr__('_'.join(('exec', 'prefix'))) + self.assertRaises(TypeError, deletePrefix2) + + def test_readonly_assign(self): def assignClass(): sys.__class__ = object self.assertRaises(TypeError, assignClass) def assignDict(): sys.__dict__ = {} self.assertRaises(TypeError, assignDict) + def assignPrefix(): sys.exec_prefix = "xxx" + self.assertRaises(TypeError, assignPrefix) + + def assignManager(): sys.packageManager = object() + self.assertRaises(TypeError, assignManager) + + def assignRegistry(): sys.registry = {} + self.assertRaises(TypeError, assignRegistry) + + def assignPrefix2(): sys.__setattr__('_'.join(('exec', 'prefix')), "xxx") + self.assertRaises(TypeError, assignPrefix2) + def test_resetmethod(self): gde = sys.getdefaultencoding sys.getdefaultencoding = 5 @@ -181,6 +212,7 @@ def test_nonexisting_import_from_unicodepath(self): sys.path.append(u'/home/tr\xf6\xf6t') self.assertRaises(ImportError, __import__, 'non_existing_module') + @unittest.skipIf(is_jython_posix, "FIXME: path is not found") def test_import_from_unicodepath(self): # \xf6 = german o umlaut moduleDir = tempfile.mkdtemp(suffix=u'tr\xf6\xf6t') @@ -201,14 +233,15 @@ def test_import_from_unicodepath(self): os.remove(moduleFile) finally: os.rmdir(moduleDir) - self.assertFalse(os.path.exists(moduleDir)) + self.assertFalse(os.path.exists(moduleDir)) + class SysEncodingTest(unittest.TestCase): # Adapted from CPython 2.7 test_sys to exercise setting Jython registry # values related to encoding and error policy. - @unittest.skipIf(is_jython_nt, "FIXME: fails probably due to issue 2312") + @unittest.skipIf(is_jython_nt, "FIXME: fails probably due to bjo 2312") def test_ioencoding(self): # adapted from CPython v2.7 test_sys import subprocess, os env = dict(os.environ) @@ -257,6 +290,8 @@ def check(code, encoding=None, errors=None): self.assertEqual(check(0xa2, None, "backslashreplace"), r"\xa2") self.assertEqual(check(0xa2, "cp850"), "\xbd") + +@unittest.skipIf(is_jython, "Failing: possibly incorrect test expectation") class SysArgvTest(unittest.TestCase): def test_unicode_argv(self): @@ -271,6 +306,7 @@ def test_unicode_argv(self): stdout=subprocess.PIPE) self.assertEqual(p.stdout.read().decode("utf-8"), zhongwen) + class InteractivePromptTest(unittest.TestCase): # TODO ps1, ps2 being defined for interactive usage should be # captured by test_doctest, however, it would be ideal to add diff --git a/src/org/python/core/PySystemState.java b/src/org/python/core/PySystemState.java index 7014dae81..80abf09a2 100644 --- a/src/org/python/core/PySystemState.java +++ b/src/org/python/core/PySystemState.java @@ -276,17 +276,31 @@ void reload() throws PyIgnoreMethodTag { } private static void checkReadOnly(String name) { - if (name == "__dict__" || name == "__class__" || name == "registry" || name == "exec_prefix" - || name == "packageManager") { - throw Py.TypeError("readonly attribute"); + switch (name) { + case "__dict__": + case "__class__": + case "registry": + case "exec_prefix": + case "packageManager": + throw Py.TypeError("readonly attribute"); + default: + // pass } } private static void checkMustExist(String name) { - if (name == "__dict__" || name == "__class__" || name == "registry" || name == "exec_prefix" - || name == "platform" || name == "packageManager" || name == "builtins" - || name == "warnoptions") { - throw Py.TypeError("readonly attribute"); + switch (name) { + case "__dict__": + case "__class__": + case "registry": + case "exec_prefix": + case "platform": + case "packageManager": + case "builtins": + case "warnoptions": + throw Py.TypeError("readonly attribute"); + default: + // pass } } @@ -430,7 +444,7 @@ public PyObject __findattr_ex__(String name) { @Override public void __setattr__(String name, PyObject value) { checkReadOnly(name); - if (name == "builtins") { + if ("builtins".equals(name)) { setBuiltins(value); } else { PyObject ret = getType().lookup(name); // xxx fix fix fix