Log message for revision 71127: fix #2235 for real now Changed: U Zope/branches/Zope-2_8-branch/doc/CHANGES.txt U Zope/branches/Zope-2_8-branch/lib/python/Products/ZCatalog/ZCatalog.py U Zope/branches/Zope-2_8-branch/lib/python/Products/ZCatalog/tests/testCatalog.py
-=- Modified: Zope/branches/Zope-2_8-branch/doc/CHANGES.txt =================================================================== --- Zope/branches/Zope-2_8-branch/doc/CHANGES.txt 2006-11-15 05:32:07 UTC (rev 71126) +++ Zope/branches/Zope-2_8-branch/doc/CHANGES.txt 2006-11-15 07:48:38 UTC (rev 71127) @@ -8,8 +8,10 @@ Bugs fixed - - Collector #2235: ZCatalog.manage_catalogObject was triggering __len__ - of objects that implement it, like containers. + - Collector #2235: A number of ZCatalog methods were doing boolean + evaluation of objects that implemented __len__ instead of checking + them against None. Replace a number of "if not obj" with + "if obj is not None". - Fix yet another resTructuredText glitch, and add tests (test backported from 2.9, which was not in fact vulnerable). Modified: Zope/branches/Zope-2_8-branch/lib/python/Products/ZCatalog/ZCatalog.py =================================================================== --- Zope/branches/Zope-2_8-branch/lib/python/Products/ZCatalog/ZCatalog.py 2006-11-15 05:32:07 UTC (rev 71126) +++ Zope/branches/Zope-2_8-branch/lib/python/Products/ZCatalog/ZCatalog.py 2006-11-15 07:48:38 UTC (rev 71127) @@ -232,7 +232,7 @@ for url in urls: obj = self.resolve_path(url) - if obj is not None: + if obj is None: obj = self.resolve_url(url, REQUEST) if obj is not None: self.catalog_object(obj, url) @@ -297,7 +297,7 @@ p = paths[i] obj = self.resolve_path(p) - if not obj and hasattr(self, 'REQUEST'): + if obj is None and hasattr(self, 'REQUEST'): obj = self.resolve_url(p, self.REQUEST) if obj is not None: try: @@ -615,8 +615,8 @@ def getobject(self, rid, REQUEST=None): """Return a cataloged object given a 'data_record_id_' """ - obj = self.aq_parent.unrestrictedTraverse(self.getpath(rid)) - if not obj: + obj = self.aq_parent.unrestrictedTraverse(self.getpath(rid), None) + if obj is None: if REQUEST is None: REQUEST=self.REQUEST obj = self.resolve_url(self.getpath(rid), REQUEST) Modified: Zope/branches/Zope-2_8-branch/lib/python/Products/ZCatalog/tests/testCatalog.py =================================================================== --- Zope/branches/Zope-2_8-branch/lib/python/Products/ZCatalog/tests/testCatalog.py 2006-11-15 05:32:07 UTC (rev 71126) +++ Zope/branches/Zope-2_8-branch/lib/python/Products/ZCatalog/tests/testCatalog.py 2006-11-15 07:48:38 UTC (rev 71127) @@ -28,6 +28,7 @@ from AccessControl.SecurityManagement import setSecurityManager from AccessControl.SecurityManagement import noSecurityManager from AccessControl import Unauthorized +from Acquisition import Implicit from Products.ZCatalog import Vocabulary from Products.ZCatalog.Catalog import Catalog from Products.ZCatalog.Catalog import CatalogError @@ -159,7 +160,32 @@ def __nonzero__(self): return False +# make objects with failing __len__ and __nonzero__ +class dummyLenFail(zdummy): + def __init__(self, num, fail): + zdummy.__init__(self, num) + self.fail = fail + def __len__(self): + self.fail("__len__() was called") + +class dummyNonzeroFail(zdummy): + def __init__(self, num, fail): + zdummy.__init__(self, num) + self.fail = fail + + def __nonzero__(self): + self.fail("__nonzero__() was called") + +class fakeparent(Implicit): + # fake parent mapping unrestrictedTraverse to + # catalog.resolve_path as simulated by TestZCatalog + def __init__(self, d): + self.d = d + + def unrestrictedTraverse(self, path, default=None): + return self.d.get(path, default) + class TestZCatalog(unittest.TestCase): def setUp(self): @@ -246,28 +272,30 @@ result = self._catalog(title='9999') self.assertEquals(1, len(result)) - def test_manage_catalogObject_does_not_trigger_boolean_eval(self): - # make objects with __len__ and __nonzero__ - class mydummy1: - def __init__(self, fail): - self.fail = fail - def __len__(self): - self.fail("__len__() was called") - class mydummy2: - def __init__(self, fail): - self.fail = fail - def __nonzero__(self): - self.fail("__nonzero__() was called") - # store them to be found by the catalog - self.d['0'] = mydummy1(self.fail) - self.d['1'] = mydummy2(self.fail) + def testBooleanEvalOn_manage_catalogObject_(self): + self.d['11'] = dummyLenFail(11, self.fail) + self.d['12'] = dummyNonzeroFail(12, self.fail) # create a fake response that doesn't bomb on manage_catalogObject() class myresponse: def redirect(self, url): pass # this next call should not fail - self._catalog.manage_catalogObject(None, myresponse(), 'URL1', urls=('0', '1')) + self._catalog.manage_catalogObject(None, myresponse(), 'URL1', urls=('11', '12')) + def testBooleanEvalOn_refreshCatalog_getobject(self): + # wrap catalog under the fake parent + catalog = self._catalog.__of__(fakeparent(self.d)) + # replace entries to test refreshCatalog + self.d['0'] = dummyLenFail(0, self.fail) + self.d['1'] = dummyNonzeroFail(1, self.fail) + # this next call should not fail + catalog.refreshCatalog() + + for uid in ('0', '1'): + rid = self._catalog.getrid(uid) + # neither should these + catalog.getobject(rid) + class dummy(ExtensionClass.Base): att1 = 'att1' att2 = 'att2' _______________________________________________ Zope-Checkins maillist - Zope-Checkins@zope.org http://mail.zope.org/mailman/listinfo/zope-checkins