diff --git a/forms/ComplexTableField.php b/forms/ComplexTableField.php
index 8c7aa8fb3..64a63975d 100755
--- a/forms/ComplexTableField.php
+++ b/forms/ComplexTableField.php
@@ -642,7 +642,7 @@ JS;
try {
$childData->write();
} catch(ValidationException $e) {
- $form->sessionMessage($e->getResult()->message(), 'bad');
+ $form->sessionMessage($e->getResult()->message(), 'bad', false);
return Director::redirectBack();
}
@@ -670,16 +670,16 @@ JS;
_t('ComplexTableField.CLOSEPOPUP', 'Close Popup')
);
- $editLink = Controller::join_links($this->Link(), 'item/' . $childData->ID . '/edit');
+ $editLink = Controller::join_links($this->Link(), 'item/' . (int)$childData->ID . '/edit');
$message = sprintf(
_t('ComplexTableField.SUCCESSADD', 'Added %s %s %s'),
$childData->singular_name(),
- '' . $childData->Title . '',
+ '' . Convert::raw2xml($childData->Title) . '',
$closeLink
);
- $form->sessionMessage($message, 'good');
+ $form->sessionMessage($message, 'good', false);
Director::redirectBack();
}
@@ -851,7 +851,7 @@ class ComplexTableField_ItemRequest extends TableListField_ItemRequest {
$closeLink
);
- $form->sessionMessage($message, 'good');
+ $form->sessionMessage($message, 'good', false);
Director::redirectBack();
}
diff --git a/forms/Form.php b/forms/Form.php
index f457352ad..86a8cd3b8 100755
--- a/forms/Form.php
+++ b/forms/Form.php
@@ -137,10 +137,6 @@ class Form extends RequestHandler {
*/
protected $extraClasses = array();
- public static $casting = array(
- 'Message' => 'Text'
- );
-
/**
* Create a new form, with the given fields an action buttons.
*
@@ -393,10 +389,10 @@ class Form extends RequestHandler {
* Add an error message to a field on this form. It will be saved into the session
* and used the next time this form is displayed.
*/
- function addErrorMessage($fieldName, $message, $messageType) {
+ function addErrorMessage($fieldName, $message, $messageType, $escapeHtml = true) {
Session::add_to_array("FormInfo.{$this->FormName()}.errors", array(
'fieldName' => $fieldName,
- 'message' => $message,
+ 'message' => $escapeHtml ? Convert::raw2xml($message) : $message,
'messageType' => $messageType,
));
}
@@ -799,6 +795,7 @@ class Form extends RequestHandler {
$this->getMessageFromSession();
$message = $this->message;
$this->clearMessage();
+
return $message;
}
@@ -816,7 +813,6 @@ class Form extends RequestHandler {
}else{
$this->message = Session::get("FormInfo.{$this->FormName()}.formError.message");
$this->messageType = Session::get("FormInfo.{$this->FormName()}.formError.type");
-
Session::clear("FormInfo.{$this->FormName()}");
}
}
@@ -826,9 +822,12 @@ class Form extends RequestHandler {
*
* @param message the text of the message
* @param type Should be set to good, bad, or warning.
+ * @param boolean $escapeHtml Automatically sanitize the message. Set to FALSE if the message contains HTML.
+ * In that case, you might want to use {@link Convert::raw2xml()} to escape any
+ * user supplied data in the message.
*/
- function setMessage($message, $type) {
- $this->message = $message;
+ function setMessage($message, $type, $escapeHtml = true) {
+ $this->message = ($escapeHtml) ? Convert::raw2xml($message) : $message;
$this->messageType = $type;
}
@@ -837,14 +836,23 @@ class Form extends RequestHandler {
*
* @param message the text of the message
* @param type Should be set to good, bad, or warning.
+ * @param boolean $escapeHtml Automatically sanitize the message. Set to FALSE if the message contains HTML.
+ * In that case, you might want to use {@link Convert::raw2xml()} to escape any
+ * user supplied data in the message.
*/
- function sessionMessage($message, $type) {
- Session::set("FormInfo.{$this->FormName()}.formError.message", $message);
+ function sessionMessage($message, $type, $escapeHtml = true) {
+ Session::set(
+ "FormInfo.{$this->FormName()}.formError.message",
+ $escapeHtml ? Convert::raw2xml($message) : $message
+ );
Session::set("FormInfo.{$this->FormName()}.formError.type", $type);
}
- static function messageForForm( $formName, $message, $type ) {
- Session::set("FormInfo.{$formName}.formError.message", $message);
+ static function messageForForm( $formName, $message, $type, $escapeHtml = true) {
+ Session::set(
+ "FormInfo.{$formName}.formError.message",
+ $escapeHtml ? Convert::raw2xml($message) : $message
+ );
Session::set("FormInfo.{$formName}.formError.type", $type);
}
diff --git a/forms/FormField.php b/forms/FormField.php
index cc62c3748..7d41c162f 100755
--- a/forms/FormField.php
+++ b/forms/FormField.php
@@ -72,10 +72,6 @@ class FormField extends RequestHandler {
* @var Custom Validation Message for the Field
*/
protected $customValidationMessage = "";
-
- public static $casting = array(
- 'Message' => 'Text'
- );
/**
* Create a new field.
@@ -330,9 +326,12 @@ class FormField extends RequestHandler {
/**
* Sets the error message to be displayed on the form field
- * Set by php validation of the form
+ * Set by php validation of the form.
+ *
+ * @param string $message Message to show to the user. Allows HTML content,
+ * which means you need to use Convert::raw2xml() for any user supplied data.
*/
- function setError($message,$messageType){
+ function setError($message, $messageType){
$this->message = $message;
$this->messageType = $messageType;
}
diff --git a/security/MemberLoginForm.php b/security/MemberLoginForm.php
index 6e67e1dd4..b0bb75f2c 100644
--- a/security/MemberLoginForm.php
+++ b/security/MemberLoginForm.php
@@ -106,11 +106,17 @@ JS
* Get message from session
*/
protected function getMessageFromSession() {
- parent::getMessageFromSession();
if(($member = Member::currentUser()) && !Session::get('MemberLoginForm.force_message')) {
- $this->message = sprintf(_t('Member.LOGGEDINAS', "You're logged in as %s."), $member->{$this->loggedInAsField});
+ $this->sessionMessage(
+ sprintf(_t('Member.LOGGEDINAS', "You're logged in as %s."), $member->{$this->loggedInAsField}),
+ 'notice'
+ );
}
Session::set('MemberLoginForm.force_message', false);
+
+ parent::getMessageFromSession();
+
+ return $this->message;
}
diff --git a/tests/forms/FormTest.php b/tests/forms/FormTest.php
index e45aba6e4..118b8649a 100755
--- a/tests/forms/FormTest.php
+++ b/tests/forms/FormTest.php
@@ -324,10 +324,56 @@ class FormTest extends FunctionalTest {
SecurityToken::disable(); // restore original
}
+
+ function testMessageEscapeHtml() {
+ $form = $this->getStubForm();
+ $form->Controller()->handleRequest(new SS_HTTPRequest('GET', '/')); // stub out request
+ $form->sessionMessage('Escaped HTML', 'good', true);
+ $parser = new CSSContentParser($form->forTemplate());
+ $messageEls = $parser->getBySelector('.message');
+ $this->assertContains(
+ '<em>Escaped HTML</em>',
+ $messageEls[0]->asXML()
+ );
+
+ $form = $this->getStubForm();
+ $form->Controller()->handleRequest(new SS_HTTPRequest('GET', '/')); // stub out request
+ $form->sessionMessage('Unescaped HTML', 'good', false);
+ $parser = new CSSContentParser($form->forTemplate());
+ $messageEls = $parser->getBySelector('.message');
+ $this->assertContains(
+ 'Unescaped HTML',
+ $messageEls[0]->asXML()
+ );
+ }
+
+ function testFieldMessageEscapeHtml() {
+ $form = $this->getStubForm();
+ $form->Controller()->handleRequest(new SS_HTTPRequest('GET', '/')); // stub out request
+ $form->addErrorMessage('key1', 'Escaped HTML', 'good', true);
+ $form->setupFormErrors();
+ $parser = new CSSContentParser($form->forTemplate());
+ $messageEls = $parser->getBySelector('#key1 .message');
+ $this->assertContains(
+ '<em>Escaped HTML</em>',
+ $messageEls[0]->asXML()
+ );
+
+ $form = $this->getStubForm();
+ $form->Controller()->handleRequest(new SS_HTTPRequest('GET', '/')); // stub out request
+ $form->addErrorMessage('key1', 'Unescaped HTML', 'good', false);
+ $form->setupFormErrors();
+ $parser = new CSSContentParser($form->forTemplate());
+ $messageEls = $parser->getBySelector('#key1 .message');
+ $this->assertContains(
+ 'Unescaped HTML',
+ $messageEls[0]->asXML()
+ );
+ }
protected function getStubForm() {
return new Form(
- new Controller(),
+ new FormTest_Controller(),
'Form',
new FieldSet(new TextField('key1')),
new FieldSet()
diff --git a/tests/security/SecurityTest.php b/tests/security/SecurityTest.php
index c8bdbe221..055fd26b4 100644
--- a/tests/security/SecurityTest.php
+++ b/tests/security/SecurityTest.php
@@ -231,7 +231,7 @@ class SecurityTest extends FunctionalTest {
/* THE FIRST 4 TIMES, THE MEMBER SHOULDN'T BE LOCKED OUT */
if($i < 5) {
$this->assertNull($member->LockedOutUntil);
- $this->assertContains($this->loginErrorMessage(), _t('Member.ERRORWRONGCRED'));
+ $this->assertContains($this->loginErrorMessage(), Convert::raw2xml(_t('Member.ERRORWRONGCRED')));
}
/* AFTER THAT THE USER IS LOCKED OUT FOR 15 MINUTES */
@@ -272,7 +272,7 @@ class SecurityTest extends FunctionalTest {
$this->doTestLoginForm('sam@silverstripe.com' , 'incorrectpassword');
$this->doTestLoginForm('sam@silverstripe.com' , 'incorrectpassword');
$this->assertNull($this->session()->inst_get('loggedInAs'));
- $this->assertTrue(false !== stripos($this->loginErrorMessage(), _t('Member.ERRORWRONGCRED')));
+ $this->assertContains($this->loginErrorMessage(), Convert::raw2xml(_t('Member.ERRORWRONGCRED')));
$this->doTestLoginForm('sam@silverstripe.com' , '1nitialPassword');
$this->assertEquals($this->session()->inst_get('loggedInAs'), $member->ID);