[PATCH] pullrequests: detect invalid reviewers and raise HTTPBadRequest
Thomas De Schampheleire
patrickdepinguin at gmail.com
Tue Jun 16 15:48:47 EDT 2015
# HG changeset patch
# User Thomas De Schampheleire <thomas.de_schampheleire at alcatel-lucent.com>
# Date 1434282495 -7200
# Sun Jun 14 13:48:15 2015 +0200
# Node ID ad0534276417535724301a0c84ced272940456c1
# Parent 53d68f201e4602d3f2ccfcd27107d2ebea2deea2
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.
diff --git a/kallithea/controllers/pullrequests.py b/kallithea/controllers/pullrequests.py
--- a/kallithea/controllers/pullrequests.py
+++ b/kallithea/controllers/pullrequests.py
@@ -44,6 +44,7 @@ from kallithea.lib.auth import LoginRequ
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 @@ class PullrequestsController(BaseRepoCon
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 @@ class PullrequestsController(BaseRepoCon
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 @@ class PullrequestsController(BaseRepoCon
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')
diff --git a/kallithea/lib/exceptions.py b/kallithea/lib/exceptions.py
--- a/kallithea/lib/exceptions.py
+++ b/kallithea/lib/exceptions.py
@@ -99,6 +99,10 @@ class UserCreationError(Exception):
pass
+class UserInvalidException(Exception):
+ pass
+
+
class RepositoryCreationError(Exception):
pass
diff --git a/kallithea/model/pull_request.py b/kallithea/model/pull_request.py
--- a/kallithea/model/pull_request.py
+++ b/kallithea/model/pull_request.py
@@ -32,6 +32,7 @@ from pylons.i18n.translation import _
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 @@ class PullRequestModel(BaseModel):
#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)
diff --git a/kallithea/tests/functional/test_pullrequests.py b/kallithea/tests/functional/test_pullrequests.py
--- a/kallithea/tests/functional/test_pullrequests.py
+++ b/kallithea/tests/functional/test_pullrequests.py
@@ -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 @@ class TestPullrequestsController(TestCon
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):
More information about the kallithea-general
mailing list