SimpleItem's getId implementation is prone to infinite recursion

[Martin Aspeli]

In SimpleItem, we have the following::

    security.declarePublic('getId')
    def getId(self):
        """Return the id of the object as a string.

        This method should be used in preference to accessing an id attribute
        of an object directly. The getId method is public.
        """
        name=getattr(self, 'id', None)
        if callable(name):
            return name()
        if name is not None:
            return name
        if hasattr(self, '__name__'):
            return self.__name__
        raise AttributeError, 'This object has no id'

    # Alias id to __name__, which will make tracebacks a good bit nicer:
    __name__=ComputedAttribute(lambda self: self.getId())

If self.id isn't set or is None, and __name__ has not been set or overridden, this will always result in infinite recursion. self.getId() calls self.__name__ which calls self.getId().

[Andreas Jung (ajung)]

I can not reproduce this error. Unittest?

[Martin Aspeli (optilude)]

Hi Andreas,

I'll take me some time to do a unit test, but I can try. In the meantime, though, it should be obvious from inspection:

 - Something calls object.getId()
 - getattr(self, 'id', None) returns None
 - The third if statement is invoked, so it returns self.__name__
 - __name__ is a ComputedAttribute
 - When called, this calls self.getId()
 - Infinite loop

I think you could reproduce by doing:

 o.id = None
 o.getId()

I'm not quite sure whether we want __name__ or id to take precedence, though. My gut feel is that if one is set and the other isn't (regardless of which one that may be) then both __name__ and getId() should return the value of the one that's set. If neither is set, we raise an attribute error. If both are set, then getId() returns the value of 'id' and __name__ is the value as set.

[Andreas Jung (ajung)]

"""
I think you could reproduce by doing:

 o.id = None
 o.getId()
"""

I tried something like that from a half-baked unittest but I it did not trigger the problem.

[Colin Watson (cjwatson)]

The zope2 project on Launchpad has been archived at the request of the Zope developers (see https://answers.launchpad.net/launchpad/+question/683589 and https://answers.launchpad.net/launchpad/+question/685285). If this bug is still relevant, please refile it at https://github.com/zopefoundation/zope2.