Mercurial > kallithea
changeset 8069:4f03bd5ac2f2
lib: handle both HTML, unsafe strings, and exceptions passed to helpers.flash()
Before, h.flash would trust any input to contain html ... and callers would
convert exceptions to string, often with a simple str() or unicode() ... which
really didn't deserve to be trusted.
Instead, only trust messages that have a __html__ and escape anything else ...
but also apply str/unicode on the parameter so the caller doesn't have to but
*can* pass an exception directly.
| author | Mads Kiilerich <mads@kiilerich.com> |
|---|---|
| date | Tue, 24 Dec 2019 04:13:48 +0100 |
| parents | c82ef5ec8dcd |
| children | 6e5787e496a0 |
| files | kallithea/controllers/admin/repos.py kallithea/controllers/changelog.py kallithea/controllers/files.py kallithea/controllers/pullrequests.py kallithea/controllers/summary.py kallithea/lib/base.py kallithea/lib/helpers.py kallithea/templates/base/flash_msg.html kallithea/tests/functional/test_admin_users.py kallithea/tests/functional/test_files.py kallithea/tests/functional/test_pullrequests.py |
| diffstat | 11 files changed, 33 insertions(+), 33 deletions(-) [+] |
line wrap: on
line diff
--- a/kallithea/controllers/admin/repos.py Thu Dec 26 16:09:30 2019 +0100 +++ b/kallithea/controllers/admin/repos.py Tue Dec 24 04:13:48 2019 +0100 @@ -471,7 +471,7 @@ category='success') except RepositoryError as e: log.error(traceback.format_exc()) - h.flash(str(e), category='error') + h.flash(e, category='error') except Exception as e: log.error(traceback.format_exc()) h.flash(_('An error occurred during this operation'),
--- a/kallithea/controllers/changelog.py Thu Dec 26 16:09:30 2019 +0100 +++ b/kallithea/controllers/changelog.py Tue Dec 24 04:13:48 2019 +0100 @@ -67,7 +67,7 @@ h.flash(_('There are no changesets yet'), category='error') except RepositoryError as e: log.error(traceback.format_exc()) - h.flash(unicode(e), category='error') + h.flash(e, category='error') raise HTTPBadRequest() @LoginRequired(allow_default_user=True) @@ -111,7 +111,7 @@ cs = self.__get_cs(revision, repo_name) collection = cs.get_file_history(f_path) except RepositoryError as e: - h.flash(unicode(e), category='warning') + h.flash(e, category='warning') raise HTTPFound(location=h.url('changelog_home', repo_name=repo_name)) else: collection = c.db_repo_scm_instance.get_changesets(start=0, end=revision, @@ -125,11 +125,11 @@ c.cs_comments = c.db_repo.get_comments(page_revisions) c.cs_statuses = c.db_repo.statuses(page_revisions) except EmptyRepositoryError as e: - h.flash(unicode(e), category='warning') + h.flash(e, category='warning') raise HTTPFound(location=url('summary_home', repo_name=c.repo_name)) except (RepositoryError, ChangesetDoesNotExistError, Exception) as e: log.error(traceback.format_exc()) - h.flash(unicode(e), category='error') + h.flash(e, category='error') raise HTTPFound(location=url('changelog_home', repo_name=c.repo_name)) c.branch_name = branch_name
--- a/kallithea/controllers/files.py Thu Dec 26 16:09:30 2019 +0100 +++ b/kallithea/controllers/files.py Tue Dec 24 04:13:48 2019 +0100 @@ -90,7 +90,7 @@ h.flash(msg, category='error') raise HTTPNotFound() except RepositoryError as e: - h.flash(unicode(e), category='error') + h.flash(e, category='error') raise HTTPNotFound() def __get_filenode(self, cs, path): @@ -110,7 +110,7 @@ h.flash(msg, category='error') raise HTTPNotFound() except RepositoryError as e: - h.flash(unicode(e), category='error') + h.flash(e, category='error') raise HTTPNotFound() return file_node @@ -175,7 +175,7 @@ else: c.authors = c.file_history = [] except RepositoryError as e: - h.flash(unicode(e), category='error') + h.flash(e, category='error') raise HTTPNotFound() if request.environ.get('HTTP_X_PARTIAL_XHR'):
--- a/kallithea/controllers/pullrequests.py Thu Dec 26 16:09:30 2019 +0100 +++ b/kallithea/controllers/pullrequests.py Tue Dec 24 04:13:48 2019 +0100 @@ -340,7 +340,7 @@ try: cmd = CreatePullRequestAction(org_repo, other_repo, org_ref, other_ref, title, description, owner, reviewers) except CreatePullRequestAction.ValidationError as e: - h.flash(str(e), category='error', logf=log.error) + h.flash(e, category='error', logf=log.error) raise HTTPNotFound try: @@ -363,7 +363,7 @@ try: cmd = CreatePullRequestIterationAction(old_pull_request, new_org_rev, new_other_rev, title, description, owner, reviewers) except CreatePullRequestAction.ValidationError as e: - h.flash(str(e), category='error', logf=log.error) + h.flash(e, category='error', logf=log.error) raise HTTPNotFound try:
--- a/kallithea/controllers/summary.py Thu Dec 26 16:09:30 2019 +0100 +++ b/kallithea/controllers/summary.py Tue Dec 24 04:13:48 2019 +0100 @@ -108,7 +108,7 @@ try: collection = c.db_repo_scm_instance.get_changesets(reverse=True) except EmptyRepositoryError as e: - h.flash(unicode(e), category='warning') + h.flash(e, category='warning') collection = [] c.cs_pagination = Page(collection, page=p, items_per_page=size) page_revisions = [x.raw_id for x in list(c.cs_pagination)]
--- a/kallithea/lib/base.py Thu Dec 26 16:09:30 2019 +0100 +++ b/kallithea/lib/base.py Tue Dec 24 04:13:48 2019 +0100 @@ -604,7 +604,7 @@ raise webob.exc.HTTPNotFound() except RepositoryError as e: log.error(traceback.format_exc()) - h.flash(unicode(e), category='error') + h.flash(e, category='error') raise webob.exc.HTTPBadRequest()
--- a/kallithea/lib/helpers.py Thu Dec 26 16:09:30 2019 +0100 +++ b/kallithea/lib/helpers.py Tue Dec 24 04:13:48 2019 +0100 @@ -423,22 +423,14 @@ Converting the message to a string returns the message text. Instances also have the following attributes: - * ``message``: the message text. * ``category``: the category specified when the message was created. + * ``message``: the html-safe message text. """ def __init__(self, category, message): self.category = category self.message = message - def __str__(self): - return self.message - - __unicode__ = __str__ - - def __html__(self): - return escape(safe_unicode(self.message)) - def _session_flash_messages(append=None, clear=False): """Manage a message queue in tg.session: return the current message queue @@ -460,7 +452,7 @@ return flash_messages -def flash(message, category=None, logf=None): +def flash(message, category, logf=None): """ Show a message to the user _and_ log it through the specified function @@ -470,14 +462,22 @@ logf defaults to log.info, unless category equals 'success', in which case logf defaults to log.debug. """ + assert category in ('error', 'success', 'warning'), category + if hasattr(message, '__html__'): + # render to HTML for storing in cookie + safe_message = unicode(message) + else: + # Apply str - the message might be an exception with __str__ + # Escape, so we can trust the result without further escaping, without any risk of injection + safe_message = html_escape(unicode(message)) if logf is None: logf = log.info if category == 'success': logf = log.debug - logf('Flash %s: %s', category, message) + logf('Flash %s: %s', category, safe_message) - _session_flash_messages(append=(category, message)) + _session_flash_messages(append=(category, safe_message)) def pop_flash_messages(): @@ -485,7 +485,7 @@ The return value is a list of ``Message`` objects. """ - return [_Message(*m) for m in _session_flash_messages(clear=True)] + return [_Message(category, message) for category, message in _session_flash_messages(clear=True)] age = lambda x, y=False: _age(x, y)
--- a/kallithea/templates/base/flash_msg.html Thu Dec 26 16:09:30 2019 +0100 +++ b/kallithea/templates/base/flash_msg.html Tue Dec 24 04:13:48 2019 +0100 @@ -5,7 +5,7 @@ % for message in messages: <div class="alert alert-dismissable ${alert_categories[message.category]}" role="alert"> <button type="button" class="close" data-dismiss="alert" aria-hidden="true"><i class="icon-cancel-circled"></i></button> - ${message} + ${message.message|n} </div> % endfor % endif
--- a/kallithea/tests/functional/test_admin_users.py Thu Dec 26 16:09:30 2019 +0100 +++ b/kallithea/tests/functional/test_admin_users.py Tue Dec 24 04:13:48 2019 +0100 @@ -203,7 +203,7 @@ .filter(User.username == username).one() response = self.app.post(url('delete_user', id=new_user.user_id), params={'_session_csrf_secret_token': self.session_csrf_secret_token()}) - self.checkSessionFlash(response, 'User "%s" still ' + self.checkSessionFlash(response, 'User "%s" still ' 'owns 1 repositories and cannot be removed. ' 'Switch owners or remove those repositories: ' '%s' % (username, reponame)) @@ -225,7 +225,7 @@ response = self.app.post(url('delete_user', id=new_user.user_id), params={'_session_csrf_secret_token': self.session_csrf_secret_token()}) - self.checkSessionFlash(response, 'User "%s" still ' + self.checkSessionFlash(response, 'User "%s" still ' 'owns 1 repository groups and cannot be removed. ' 'Switch owners or remove those repository groups: ' '%s' % (username, groupname)) @@ -254,7 +254,7 @@ .filter(User.username == username).one() response = self.app.post(url('delete_user', id=new_user.user_id), params={'_session_csrf_secret_token': self.session_csrf_secret_token()}) - self.checkSessionFlash(response, 'User "%s" still ' + self.checkSessionFlash(response, 'User "%s" still ' 'owns 1 user groups and cannot be removed. ' 'Switch owners or remove those user groups: ' '%s' % (username, groupname))
--- a/kallithea/tests/functional/test_files.py Thu Dec 26 16:09:30 2019 +0100 +++ b/kallithea/tests/functional/test_files.py Tue Dec 24 04:13:48 2019 +0100 @@ -272,7 +272,7 @@ revision=rev, f_path=f_path), status=404) - msg = "There is no file nor directory at the given path: '%s' at revision %s" % (f_path, rev[:12]) + msg = "There is no file nor directory at the given path: '%s' at revision %s" % (f_path, rev[:12]) response.mustcontain(msg) #========================================================================== @@ -308,7 +308,7 @@ repo_name=HG_REPO, revision=rev, f_path=f_path), status=404) - msg = "There is no file nor directory at the given path: '%s' at revision %s" % (f_path, rev[:12]) + msg = "There is no file nor directory at the given path: '%s' at revision %s" % (f_path, rev[:12]) response.mustcontain(msg) def test_ajaxed_files_list(self):
--- a/kallithea/tests/functional/test_pullrequests.py Thu Dec 26 16:09:30 2019 +0100 +++ b/kallithea/tests/functional/test_pullrequests.py Tue Dec 24 04:13:48 2019 +0100 @@ -171,7 +171,7 @@ 'review_members': [str(invalid_user_id)], }, status=400) - response.mustcontain('Invalid reviewer "%s" specified' % invalid_user_id) + response.mustcontain('Invalid reviewer "%s" specified' % invalid_user_id) def test_edit_with_invalid_reviewer(self): invalid_user_id = 99999 @@ -206,7 +206,7 @@ 'review_members': [str(invalid_user_id)], }, status=400) - response.mustcontain('Invalid reviewer "%s" specified' % invalid_user_id) + response.mustcontain('Invalid reviewer "%s" specified' % invalid_user_id) def test_iteration_refs(self): # Repo graph excerpt: