Mercurial > kallithea
changeset 5187:9a23b444a7fe
pullrequests: detect invalid reviewers and raise HTTPBadRequest
Normally, the creation of a pullrequest with invalid reviewers is not
possible because the list of reviewers is populated from a form element
that only shows valid reviewers.
However, if creating a pullrequest through an API call, invalid reviewers
can be specified but would not be detected. The reviewer would be encoded
in the database as 'NULL'/None, and opening such a pull request would cause
a server error.
Instead, detect invalid reviewers at pullrequest creation/update time and
raise HTTPBadRequest.
| author | Thomas De Schampheleire <thomas.de_schampheleire@alcatel-lucent.com> |
|---|---|
| date | Sun, 14 Jun 2015 13:48:15 +0200 |
| parents | 5fb4e6f884ce |
| children | 6db5d95c2d21 |
| files | kallithea/controllers/pullrequests.py kallithea/lib/exceptions.py kallithea/model/pull_request.py kallithea/tests/functional/test_pullrequests.py |
| diffstat | 4 files changed, 149 insertions(+), 3 deletions(-) [+] |
line wrap: on
line diff
--- a/kallithea/controllers/pullrequests.py Thu Jun 11 08:18:14 2015 +0200 +++ b/kallithea/controllers/pullrequests.py Sun Jun 14 13:48:15 2015 +0200 @@ -44,6 +44,7 @@ from kallithea.lib.helpers import Page from kallithea.lib import helpers as h from kallithea.lib import diffs +from kallithea.lib.exceptions import UserInvalidException from kallithea.lib.utils import action_logger, jsonify from kallithea.lib.vcs.utils import safe_str from kallithea.lib.vcs.exceptions import EmptyRepositoryError @@ -362,6 +363,9 @@ Session().commit() h.flash(_('Successfully opened new pull request'), category='success') + except UserInvalidException, u: + h.flash(_('Invalid reviewer "%s" specified') % u, category='error') + raise HTTPBadRequest() except Exception: h.flash(_('Error occurred while creating pull request'), category='error') @@ -446,6 +450,9 @@ old_pull_request.other_repo.repo_name, new_other_ref, revisions, reviewers_ids, title, description ) + except UserInvalidException, u: + h.flash(_('Invalid reviewer "%s" specified') % u, category='error') + raise HTTPBadRequest() except Exception: h.flash(_('Error occurred while creating pull request'), category='error') @@ -495,9 +502,12 @@ old_description = pull_request.description pull_request.title = _form['pullrequest_title'] pull_request.description = _form['pullrequest_desc'].strip() or _('No description') - PullRequestModel().mention_from_description(pull_request, old_description) - - PullRequestModel().update_reviewers(pull_request_id, reviewers_ids) + try: + PullRequestModel().mention_from_description(pull_request, old_description) + PullRequestModel().update_reviewers(pull_request_id, reviewers_ids) + except UserInvalidException, u: + h.flash(_('Invalid reviewer "%s" specified') % u, category='error') + raise HTTPBadRequest() Session().commit() h.flash(_('Pull request updated'), category='success')
--- a/kallithea/lib/exceptions.py Thu Jun 11 08:18:14 2015 +0200 +++ b/kallithea/lib/exceptions.py Sun Jun 14 13:48:15 2015 +0200 @@ -99,6 +99,10 @@ pass +class UserInvalidException(Exception): + pass + + class RepositoryCreationError(Exception): pass
--- a/kallithea/model/pull_request.py Thu Jun 11 08:18:14 2015 +0200 +++ b/kallithea/model/pull_request.py Sun Jun 14 13:48:15 2015 +0200 @@ -32,6 +32,7 @@ from kallithea.model.meta import Session from kallithea.lib import helpers as h +from kallithea.lib.exceptions import UserInvalidException from kallithea.model import BaseModel from kallithea.model.db import PullRequest, PullRequestReviewers, Notification,\ ChangesetStatus, User @@ -117,6 +118,8 @@ #members for member in set(reviewers): _usr = self._get_user(member) + if _usr is None: + raise UserInvalidException(member) reviewer = PullRequestReviewers(_usr, pr) Session().add(reviewer)
--- a/kallithea/tests/functional/test_pullrequests.py Thu Jun 11 08:18:14 2015 +0200 +++ b/kallithea/tests/functional/test_pullrequests.py Sun Jun 14 13:48:15 2015 +0200 @@ -1,8 +1,11 @@ +import re + from kallithea.tests import * from kallithea.tests.fixture import Fixture from kallithea.model.meta import Session from kallithea.controllers.pullrequests import PullrequestsController +from kallithea.lib.exceptions import UserInvalidException fixture = Fixture() @@ -13,6 +16,132 @@ response = self.app.get(url(controller='pullrequests', action='index', repo_name=HG_REPO)) + def test_create_trivial(self): + self.log_user() + response = self.app.post(url(controller='pullrequests', action='create', + repo_name=HG_REPO), + {'org_repo': HG_REPO, + 'org_ref': 'branch:default:default', + 'other_repo': HG_REPO, + 'other_ref': 'branch:default:default', + 'pullrequest_title': 'title', + 'pullrequest_desc': 'description', + '_authentication_token': self.authentication_token(), + } + ) + self.assertEqual(response.status, '302 Found') + response = response.follow() + self.assertEqual(response.status, '200 OK') + response.mustcontain('This pull request has already been merged to default.') + + def test_create_with_existing_reviewer(self): + self.log_user() + response = self.app.post(url(controller='pullrequests', action='create', + repo_name=HG_REPO), + {'org_repo': HG_REPO, + 'org_ref': 'branch:default:default', + 'other_repo': HG_REPO, + 'other_ref': 'branch:default:default', + 'pullrequest_title': 'title', + 'pullrequest_desc': 'description', + '_authentication_token': self.authentication_token(), + 'review_members': TEST_USER_ADMIN_LOGIN, + } + ) + self.assertEqual(response.status, '302 Found') + response = response.follow() + self.assertEqual(response.status, '200 OK') + response.mustcontain('This pull request has already been merged to default.') + + def test_create_with_invalid_reviewer(self): + invalid_user_name = 'invalid_user' + self.log_user() + response = self.app.post(url(controller='pullrequests', action='create', + repo_name=HG_REPO), + { + 'org_repo': HG_REPO, + 'org_ref': 'branch:default:default', + 'other_repo': HG_REPO, + 'other_ref': 'branch:default:default', + 'pullrequest_title': 'title', + 'pullrequest_desc': 'description', + '_authentication_token': self.authentication_token(), + 'review_members': invalid_user_name, + }, + status=400) + response.mustcontain('Invalid reviewer "%s" specified' % invalid_user_name) + + def test_update_with_invalid_reviewer(self): + invalid_user_id = 99999 + self.log_user() + # create a valid pull request + response = self.app.post(url(controller='pullrequests', action='create', + repo_name=HG_REPO), + { + 'org_repo': HG_REPO, + 'org_ref': 'branch:default:default', + 'other_repo': HG_REPO, + 'other_ref': 'branch:default:default', + 'pullrequest_title': 'title', + 'pullrequest_desc': 'description', + '_authentication_token': self.authentication_token(), + } + ) + self.assertEqual(response.status, '302 Found') + # location is of the form: + # http://localhost/vcs_test_hg/pull-request/54/_/title + m = re.search('/pull-request/(\d+)/', response.location) + self.assertNotEqual(m, None) + pull_request_id = m.group(1) + + # update it + response = self.app.post(url(controller='pullrequests', action='post', + repo_name=HG_REPO, pull_request_id=pull_request_id), + { + 'updaterev': 'default', + 'pullrequest_title': 'title', + 'pullrequest_desc': 'description', + '_authentication_token': self.authentication_token(), + 'review_members': invalid_user_id, + }, + status=400) + response.mustcontain('Invalid reviewer "%s" specified' % invalid_user_id) + + def test_edit_with_invalid_reviewer(self): + invalid_user_id = 99999 + self.log_user() + # create a valid pull request + response = self.app.post(url(controller='pullrequests', action='create', + repo_name=HG_REPO), + { + 'org_repo': HG_REPO, + 'org_ref': 'branch:default:default', + 'other_repo': HG_REPO, + 'other_ref': 'branch:default:default', + 'pullrequest_title': 'title', + 'pullrequest_desc': 'description', + '_authentication_token': self.authentication_token(), + } + ) + self.assertEqual(response.status, '302 Found') + # location is of the form: + # http://localhost/vcs_test_hg/pull-request/54/_/title + m = re.search('/pull-request/(\d+)/', response.location) + self.assertNotEqual(m, None) + pull_request_id = m.group(1) + + # edit it + response = self.app.post(url(controller='pullrequests', action='post', + repo_name=HG_REPO, pull_request_id=pull_request_id), + { + 'pullrequest_title': 'title', + 'pullrequest_desc': 'description', + '_authentication_token': self.authentication_token(), + 'review_members': invalid_user_id, + }, + status=400) + response.mustcontain('Invalid reviewer "%s" specified' % invalid_user_id) + class TestPullrequestsGetRepoRefs(TestController): def setUp(self):