Maciej Szulik added the comment:
roundup/pull_request.py:
> key = os.environ['SECRET_KEY']
If this env is missing the code throws KeyError, which is not nice. It would be
better
to check if it exists and fail nicely, informing about the problem.
> def testPullRequestEventForTitle(self):
> # When the title of a PR has string "fixes bpo123"
> dummy_client = self._make_client("pullrequestevent.txt")
But both pullrequestevent.txt and pullrequestevent1.txt have 'fixes bpo1' in
body, not in title.
> return self.db.issue.create(title=title)
I haven't seen the issue being created, when testing this locally. I mean,
the code goes this path, but there's no new issue in the bugtracker, am I
missing something here? OK I got it, here's again the problem with per
issue URL, iow. I'm failing this condition:
> issue_exists = len(self.db.issue.filter(None, {'id': issue_id})) == 1
> url_exists = len(self.db.pull_request.filter(None, {'url': url})) == 1
> if issue_exists and not url_exists:
The issue exists, but unfortunately I'm again using the same PR URL.
You should check just this issue's urls.
> title_match = self.issue_re.search(title)
> body_match = self.issue_re.search(body)
This matching code only matches the last one, so if I specify multiple
fixes bpoX, bpoY, only Y will be updated.
> self._validate_webhook_secret()
> self._verify_request()
> self._extract()
_verify_request should come before _validate_webhook_secret, otherwise if
somebody
sends you malformed data (wrong event, method, content type) you first try to
interpret
it in secret validation, instead of rejecting the request.
Generally, try more defensive approach when accessing dictionaries, since this
endpoint will be available to outside world, it may be a potential place to
attacks, this means you should be prepared to KeyErrors at any place you're
accessing
dictionary. A except KeyError at one of the higher levels (eg. _extract) will
be perfectly
fine and that should immediately throw Reject exception.
_______________________________________________________
PSF Meta Tracker <metatrac...@psf.upfronthosting.co.za>
<http://psf.upfronthosting.co.za/roundup/meta/issue589>
_______________________________________________________
_______________________________________________
Tracker-discuss mailing list
Tracker-discuss@python.org
https://mail.python.org/mailman/listinfo/tracker-discuss
Code of Conduct: https://www.python.org/psf/codeofconduct/
