Log message for revision 91979: LP #281156: 'AccessControl.SecurityInfo.secureModule' dropped ModuleSecurity for failed imports, obscuring later attempts to import the same broken module. 'AccessControl.ZopeGuards.guarded_import' mapped some Unauthorized exceptions onto ImportErrors: don't do that! Also, removed mutable defaults from argument list, improved tests.
Changed: U Zope/branches/Zope-2_8-branch/doc/CHANGES.txt U Zope/branches/Zope-2_8-branch/lib/python/AccessControl/SecurityInfo.py U Zope/branches/Zope-2_8-branch/lib/python/AccessControl/ZopeGuards.py U Zope/branches/Zope-2_8-branch/lib/python/AccessControl/tests/testModuleSecurity.py U Zope/branches/Zope-2_8-branch/lib/python/Products/PythonScripts/tests/testPythonScript.py -=- Modified: Zope/branches/Zope-2_8-branch/doc/CHANGES.txt =================================================================== --- Zope/branches/Zope-2_8-branch/doc/CHANGES.txt 2008-10-10 14:44:57 UTC (rev 91978) +++ Zope/branches/Zope-2_8-branch/doc/CHANGES.txt 2008-10-10 15:03:26 UTC (rev 91979) @@ -8,6 +8,14 @@ Bugs fixed + - 'AccessControl.ZopeGuards.guarded_import' mapped some Unauthorized + exceptions onto ImportErrors: don't do that! Also, removed + mutable defaults from argument list, improved tests. + + - LP #281156: 'AccessControl.SecurityInfo.secureModule' dropped + ModuleSecurity for failed imports, obscuring later attempts to + import the same broken module. + - LP #142667: Updated to ZODB-3.4.5 to fix problem with product auto-refresh. Modified: Zope/branches/Zope-2_8-branch/lib/python/AccessControl/SecurityInfo.py =================================================================== --- Zope/branches/Zope-2_8-branch/lib/python/AccessControl/SecurityInfo.py 2008-10-10 14:44:57 UTC (rev 91978) +++ Zope/branches/Zope-2_8-branch/lib/python/AccessControl/SecurityInfo.py 2008-10-10 15:03:26 UTC (rev 91979) @@ -207,10 +207,10 @@ modsec = _moduleSecurity.get(mname, None) if modsec is None: return - del _moduleSecurity[mname] if imp: __import__(mname, *imp) + del _moduleSecurity[mname] module = sys.modules[mname] modsec.apply(module.__dict__) _appliedModuleSecurity[mname] = modsec Modified: Zope/branches/Zope-2_8-branch/lib/python/AccessControl/ZopeGuards.py =================================================================== --- Zope/branches/Zope-2_8-branch/lib/python/AccessControl/ZopeGuards.py 2008-10-10 14:44:57 UTC (rev 91978) +++ Zope/branches/Zope-2_8-branch/lib/python/AccessControl/ZopeGuards.py 2008-10-10 15:03:26 UTC (rev 91979) @@ -259,31 +259,29 @@ return map(f, *safe_seqs) safe_builtins['map'] = guarded_map -def guarded_import(mname, globals={}, locals={}, fromlist=None): +def guarded_import(mname, globals=None, locals=None, fromlist=None): + if fromlist is None: + fromlist = () + if '*' in fromlist: + raise Unauthorized, "'from %s import *' is not allowed" % mname + if globals is None: + globals = {} + if locals is None: + locals = {} mnameparts = mname.split('.') firstmname = mnameparts[0] validate = getSecurityManager().validate module = load_module(None, None, mnameparts, validate, globals, locals) - if module is not None: - if fromlist is None: - fromlist = () - try: - for name in fromlist: - if name == '*': - raise ImportError, ('"from %s import *" is not allowed' - % mname) - v = getattr(module, name, None) - if v is None: - v = load_module(module, mname, [name], validate, - globals, locals) - if not validate(module, module, name, v): - raise Unauthorized - else: - return __import__(mname, globals, locals, fromlist) - except Unauthorized, why: - raise ImportError, ('import of "%s" from "%s" is unauthorized. %s' - % (name, mname, why)) - raise ImportError, 'import of "%s" is unauthorized' % mname + if module is None: + raise Unauthorized, "import of '%s' is unauthorized" % mname + for name in fromlist: + v = getattr(module, name, None) + if v is None: + v = load_module(module, mname, [name], validate, globals, locals) + if not validate(module, module, name, v): + raise Unauthorized, ("import of '%s.%s' is unauthorized" + % (mname, name)) + return __import__(mname, globals, locals, fromlist) safe_builtins['__import__'] = guarded_import class GuardedListType: Modified: Zope/branches/Zope-2_8-branch/lib/python/AccessControl/tests/testModuleSecurity.py =================================================================== --- Zope/branches/Zope-2_8-branch/lib/python/AccessControl/tests/testModuleSecurity.py 2008-10-10 14:44:57 UTC (rev 91978) +++ Zope/branches/Zope-2_8-branch/lib/python/AccessControl/tests/testModuleSecurity.py 2008-10-10 15:03:26 UTC (rev 91979) @@ -10,51 +10,44 @@ # FOR A PARTICULAR PURPOSE # ############################################################################## -"""Module Import Tests -""" +import unittest -__rcs_id__='$Id$' -__version__='$Revision: 1.4 $'[11:-2] +class ModuleSecurityTests(unittest.TestCase): -import os, sys, unittest + def setUp(self): + from AccessControl import ModuleSecurityInfo as MSI + MSI('AccessControl.tests.mixed_module').declarePublic('pub') + MSI('AccessControl.tests.public_module').declarePublic('pub') + MSI('AccessControl.tests.public_module.submodule').declarePublic('pub') -import Testing -import ZODB -from AccessControl import User -from AccessControl import Unauthorized, ModuleSecurityInfo -from AccessControl.ZopeGuards import guarded_import + def tearDown(self): + import sys + for module in ('AccessControl.tests.public_module', + 'AccessControl.tests.public_module.submodule', + 'AccessControl.tests.mixed_module', + 'AccessControl.tests.mixed_module.submodule', + 'AccessControl.tests.private_module', + 'AccessControl.tests.private_module.submodule', + ): + if module in sys.modules: + del sys.modules[module] -ModuleSecurityInfo('AccessControl.tests.mixed_module').declarePublic('pub') - -ModuleSecurityInfo('AccessControl.tests.public_module').declarePublic('pub') -ModuleSecurityInfo('AccessControl.tests.public_module.submodule' - ).declarePublic('pub') - -class SecurityTests(unittest.TestCase): - def assertUnauth(self, module, fromlist): - try: - guarded_import(module, fromlist=fromlist) - except (Unauthorized, ImportError): - # Passed the test. - pass - else: - assert 0, ('Did not protect module instance %s, %s' % - (`module`, `fromlist`)) + from zExceptions import Unauthorized + from AccessControl.ZopeGuards import guarded_import + self.assertRaises(Unauthorized, + guarded_import, module, fromlist=fromlist) def assertAuth(self, module, fromlist): - try: - guarded_import(module, fromlist=fromlist) - except (Unauthorized, ImportError): - assert 0, ('Did not expose module instance %s, %s' % - (`module`, `fromlist`)) + from AccessControl.ZopeGuards import guarded_import + guarded_import(module, fromlist=fromlist) def testPrivateModule(self): - for name in '', '.submodule': - for fromlist in (), ('priv',): - self.assertUnauth( - 'AccessControl.tests.private_module%s' % name, - fromlist) + self.assertUnauth('AccessControl.tests.private_module', ()) + self.assertUnauth('AccessControl.tests.private_module', ('priv',)) + self.assertUnauth('AccessControl.tests.private_module.submodule', ()) + self.assertUnauth('AccessControl.tests.private_module.submodule', + ('priv',)) def testMixedModule(self): self.assertAuth('AccessControl.tests.mixed_module', ()) @@ -63,19 +56,25 @@ self.assertUnauth('AccessControl.tests.mixed_module.submodule', ()) def testPublicModule(self): - for name in '', '.submodule': - for fromlist in (), ('pub',): - self.assertAuth( - 'AccessControl.tests.public_module%s' % name, - fromlist) + self.assertAuth('AccessControl.tests.public_module', ()) + self.assertAuth('AccessControl.tests.public_module', ('pub',)) + self.assertAuth('AccessControl.tests.public_module.submodule', ()) + self.assertAuth('AccessControl.tests.public_module.submodule', + ('pub',)) -def test_suite(): - suite = unittest.TestSuite() - suite.addTest( unittest.makeSuite( SecurityTests ) ) - return suite + def test_public_module_asterisk_not_allowed(self): + self.assertUnauth('AccessControl.tests.public_module', ('*',)) -def main(): - unittest.TextTestRunner().run(test_suite()) + def test_failed_import_keeps_MSI(self): + # LP #281156 + from AccessControl import ModuleSecurityInfo as MSI + from AccessControl.SecurityInfo import _moduleSecurity as MS + from AccessControl.ZopeGuards import guarded_import + MSI('AccessControl.tests.nonesuch').declarePublic('pub') + self.failUnless('AccessControl.tests.nonesuch' in MS) + self.assertRaises(ImportError, + guarded_import, 'AccessControl.tests.nonesuch', ()) + self.failUnless('AccessControl.tests.nonesuch' in MS) -if __name__ == '__main__': - main() +def test_suite(): + return unittest.makeSuite(ModuleSecurityTests) Modified: Zope/branches/Zope-2_8-branch/lib/python/Products/PythonScripts/tests/testPythonScript.py =================================================================== --- Zope/branches/Zope-2_8-branch/lib/python/Products/PythonScripts/tests/testPythonScript.py 2008-10-10 14:44:57 UTC (rev 91978) +++ Zope/branches/Zope-2_8-branch/lib/python/Products/PythonScripts/tests/testPythonScript.py 2008-10-10 15:03:26 UTC (rev 91979) @@ -231,8 +231,9 @@ self.assertPSRaises(SyntaxError, path='subversive_except') def testBadImports(self): - self.assertPSRaises(ImportError, body="from string import *") - self.assertPSRaises(ImportError, body="import mmap") + from zExceptions import Unauthorized + self.assertPSRaises(Unauthorized, body="from string import *") + self.assertPSRaises(Unauthorized, body="import mmap") def testAttributeAssignment(self): # It's illegal to assign to attributes of anything that _______________________________________________ Zope-Checkins maillist - Zope-Checkins@zope.org http://mail.zope.org/mailman/listinfo/zope-checkins