launchpad-reviewers team mailing list archive

[Curtis Hovey <curtis.hovey@xxxxxxxxxxxxx>]

Review: Needs Fixing code

I do not think getTermByToken() verifies that the user is in the vocabulary. __contains__ does not verify that that the object or token is in the vocabulary. This is what I see in the base class:

    def getTermByToken(self, token):
        ...
            # This doesn't look like an email, so let's simply treat
            # it like a name.
            person = getUtility(IPersonSet).getByName(token)
            if person is None:
                raise LookupError(token)
            term = self.toTerm(person)
            if term is None:
                raise LookupError(token)
            return term

^ I think This simple lookup will get a suspended user, inclusive team, and private team. The first is not a valid user, the second is not valid for many cases, which is why we search, and the last works my accident because toTerm
-- 
https://code.launchpad.net/~wallyworld/launchpad/remove-some-disclosure-featureflags/+merge/103201
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.