From 062c8ee574f812f1705a222695f6bf786a063eed Mon Sep 17 00:00:00 2001 From: John Kristensen Date: Mon, 10 Aug 2015 23:35:03 +1000 Subject: [PATCH] Display errors containing HTML with RejectRaw (issue2550847) In general outputting un-escaped HTML in a message to the user is an unsafe operation, which is why error message are escaped by default. In some cases though it is desirable for a detector to include HTML within an error message. For these cases where HTML is required the RejectRaw exception can be used within the detector. --- CHANGES.txt | 3 +++ doc/customizing.txt | 8 +++++++- roundup/cgi/actions.py | 35 ++++++++++++++++++++++------------- roundup/cgi/client.py | 6 ++++-- roundup/exceptions.py | 9 +++++++++ 5 files changed, 45 insertions(+), 16 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index fcfc6e0b5d..fc2a2d6051 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -57,6 +57,9 @@ Features: (similar to RFC 2822), e.g. +0200 for CEST or -0500 for EST. This also works in the XMLRPC interface. For examples see roundup.date.Date. (Ralf Schlatterbeck) +- Add RejectRaw exception to allow unescaped HTML error messages to be + displayed to the user (thanks Ezio Melotti for the initial patch) + (John Kristensen) Fixed: diff --git a/doc/customizing.txt b/doc/customizing.txt index 83711aa053..0fd79e27b8 100644 --- a/doc/customizing.txt +++ b/doc/customizing.txt @@ -928,7 +928,13 @@ To use, simply add at the top of your auditor:: And then when your rejection criteria have been detected, simply:: - raise Reject + raise Reject('Description of error') + +Error messages raised with ``Reject`` automatically have any HTML content +escaped before being displayed to the user. To display an error message to the +user without performing any HTML escaping the ``RejectRaw`` should be used. All +security implications should be carefully considering before using +``RejectRaw``. Generating email from Roundup diff --git a/roundup/cgi/actions.py b/roundup/cgi/actions.py index e16e913358..4d6c3d30d4 100644 --- a/roundup/cgi/actions.py +++ b/roundup/cgi/actions.py @@ -3,9 +3,9 @@ import re, cgi, time, random, csv, codecs from roundup import hyperdb, token, date, password from roundup.actions import Action as BaseAction from roundup.i18n import _ -import roundup.exceptions from roundup.cgi import exceptions, templating from roundup.mailgw import uidFromAddress +from roundup.exceptions import Reject, RejectRaw from roundup.anypy import io_, urllib_ __all__ = ['Action', 'ShowAction', 'RetireAction', 'SearchAction', @@ -106,7 +106,7 @@ class RetireAction(Action): """Retire the context item.""" # ensure modification comes via POST if self.client.env['REQUEST_METHOD'] != 'POST': - raise roundup.exceptions.Reject(self._('Invalid request')) + raise Reject(self._('Invalid request')) # if we want to view the index template now, then unset the itemid # context info (a special-case for retire actions on the index page) @@ -285,7 +285,7 @@ class EditCSVAction(Action): """ # ensure modification comes via POST if self.client.env['REQUEST_METHOD'] != 'POST': - raise roundup.exceptions.Reject(self._('Invalid request')) + raise Reject(self._('Invalid request')) # figure the properties list for the class cl = self.db.classes[self.classname] @@ -606,7 +606,7 @@ class EditItemAction(EditCommon): """ # ensure modification comes via POST if self.client.env['REQUEST_METHOD'] != 'POST': - raise roundup.exceptions.Reject(self._('Invalid request')) + raise Reject(self._('Invalid request')) user_activity = self.lastUserActivity() if user_activity: @@ -620,8 +620,11 @@ class EditItemAction(EditCommon): # handle the props try: message = self._editnodes(props, links) - except (ValueError, KeyError, IndexError, - roundup.exceptions.Reject), message: + except RejectRaw as message: + self.client.add_error_message( + self._('Edit Error: %s') % str(message), escape=False) + return + except (ValueError, KeyError, IndexError, Reject) as message: self.client.add_error_message( self._('Edit Error: %s') % str(message)) return @@ -652,7 +655,7 @@ class NewItemAction(EditCommon): ''' # ensure modification comes via POST if self.client.env['REQUEST_METHOD'] != 'POST': - raise roundup.exceptions.Reject(self._('Invalid request')) + raise Reject(self._('Invalid request')) # parse the props from the form try: @@ -666,8 +669,11 @@ class NewItemAction(EditCommon): try: # when it hits the None element, it'll set self.nodeid messages = self._editnodes(props, links) - except (ValueError, KeyError, IndexError, - roundup.exceptions.Reject), message: + except RejectRaw as message: + self.client.add_error_message( + _('Error: %s') % str(message), escape=False) + return + except (ValueError, KeyError, IndexError, Reject) as message: # these errors might just be indicative of user dumbness self.client.add_error_message(_('Error: %s') % str(message)) return @@ -833,7 +839,7 @@ class RegisterAction(RegoCommon, EditCommon): """ # ensure modification comes via POST if self.client.env['REQUEST_METHOD'] != 'POST': - raise roundup.exceptions.Reject(self._('Invalid request')) + raise Reject(self._('Invalid request')) # parse the props from the form try: @@ -849,8 +855,11 @@ class RegisterAction(RegoCommon, EditCommon): try: # when it hits the None element, it'll set self.nodeid messages = self._editnodes(props, links) - except (ValueError, KeyError, IndexError, - roundup.exceptions.Reject), message: + except RejectRaw as message: + self.client.add_error_message( + _('Error: %s') % str(message), escape=False) + return + except (ValueError, KeyError, IndexError, Reject) as message: # these errors might just be indicative of user dumbness self.client.add_error_message(_('Error: %s') % str(message)) return @@ -957,7 +966,7 @@ class LoginAction(Action): """ # ensure modification comes via POST if self.client.env['REQUEST_METHOD'] != 'POST': - raise roundup.exceptions.Reject(self._('Invalid request')) + raise Reject(self._('Invalid request')) # we need the username at a minimum if '__login_name' not in self.form: diff --git a/roundup/cgi/client.py b/roundup/cgi/client.py index 3b7f7d40ac..f28db9cad8 100644 --- a/roundup/cgi/client.py +++ b/roundup/cgi/client.py @@ -16,7 +16,7 @@ except ImportError: from roundup import roundupdb, date, hyperdb, password from roundup.cgi import templating, cgitb, TranslationService from roundup.cgi.actions import * -from roundup.exceptions import * +from roundup.exceptions import LoginError, Reject, RejectRaw, Unauthorised from roundup.cgi.exceptions import * from roundup.cgi.form_parser import FormParser from roundup.mailer import Mailer, MessageSendError, encode_quopri @@ -1275,7 +1275,9 @@ class Client: else: return action_klass(self).execute() - except (ValueError, Reject), err: + except RejectRaw as err: + self.add_error_message(str(err), escape=False) + except (ValueError, Reject) as err: self.add_error_message(str(err)) def get_action_class(self, action_name): diff --git a/roundup/exceptions.py b/roundup/exceptions.py index 629516d3a0..78c45a6f58 100644 --- a/roundup/exceptions.py +++ b/roundup/exceptions.py @@ -21,6 +21,15 @@ class Reject(Exception): """ pass + +class RejectRaw(Reject): + """ + Performs the same function as Reject, except HTML in the message is not + escaped when displayed to the user. + """ + pass + + class UsageError(ValueError): pass -- 2.5.1