Merge remote-tracking branch 'origin/3.0' into 3.1

forms/Form.php

@@ -155,10 +155,6 @@ class Form extends RequestHandler {
 		'forTemplate',
 	);
 
-	private static $casting = array(
-		'Message' => 'Text'
-	);
-
 	/**
 	 * Create a new form, with the given fields an action buttons.
 	 * 
@@ -508,10 +504,10 @@ class Form extends RequestHandler {
 	 * Add a plain text error message to a field on this form.  It will be saved into the session
 	 * and used the next time this form is displayed.
 	 */
-	public function addErrorMessage($fieldName, $message, $messageType) {
+	public 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,
 		));
 	}
@@ -1035,9 +1031,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.
 	 */
-	public function setMessage($message, $type) {
+	public function setMessage($message, $type, $escapeHtml = true) {
-		$this->message = $message;
+		$this->message = ($escapeHtml) ? Convert::raw2xml($message) : $message;
 		$this->messageType = $type;
 		return $this;
 	}
@@ -1047,14 +1046,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.
 	 */
-	public function sessionMessage($message, $type) {
+	public function sessionMessage($message, $type, $escapeHtml = true) {
-		Session::set("FormInfo.{$this->FormName()}.formError.message", $message);
+		Session::set(
+			"FormInfo.{$this->FormName()}.formError.message", 
+			$escapeHtml ? Convert::raw2xml($message) : $message
+		);
 		Session::set("FormInfo.{$this->FormName()}.formError.type", $type);
 	}
 
-	public static function messageForForm( $formName, $message, $type ) {
+	public static function messageForForm( $formName, $message, $type, $escapeHtml = true) {
-		Session::set("FormInfo.{$formName}.formError.message", $message);
+		Session::set(
+			"FormInfo.{$formName}.formError.message", 
+			$escapeHtml ? Convert::raw2xml($message) : $message
+		);
 		Session::set("FormInfo.{$formName}.formError.type", $type);
 	}
 

forms/FormField.php

@@ -93,10 +93,6 @@ class FormField extends RequestHandler {
 	 */
 	protected $attributes = array();
 
-	private static $casting = array(
-		'Message' => 'Text'
-	);
-
 	/**
 	 * Takes a fieldname and converts camelcase to spaced
 	 * words. Also resolves combined fieldnames with dot syntax
@@ -475,7 +471,10 @@ 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.
 	 */
 	public function setError($message, $messageType) {
 		$this->message = $message; 

forms/gridfield/GridFieldDetailForm.php

@@ -527,7 +527,7 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler {
 			$this->record->write();
 			$list->add($this->record, $extraData);
 		} catch(ValidationException $e) {
-			$form->sessionMessage($e->getResult()->message(), 'bad');
+			$form->sessionMessage($e->getResult()->message(), 'bad', false);
 			$responseNegotiator = new PjaxResponseNegotiator(array(
 				'CurrentForm' => function() use(&$form) {
 					return $form->forTemplate();
@@ -544,11 +544,9 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler {
 
 		// TODO Save this item into the given relationship
 
-		// TODO Allow HTML in form messages
+		$link = '<a href="' . $this->Link('edit') . '">"' 
-		// $link = '<a href="' . $this->Link('edit') . '">"' 
+			. htmlspecialchars($this->record->Title, ENT_QUOTES) 
-		// 	. htmlspecialchars($this->record->Title, ENT_QUOTES) 
+			. '"</a>';
-		// 	. '"</a>';
-		$link = '"' . $this->record->Title . '"';
 		$message = _t(
 			'GridFieldDetailForm.Saved', 
 			'Saved {name} {link}',
@@ -558,7 +556,7 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler {
 			)
 		);
 		
-		$form->sessionMessage($message, 'good');
+		$form->sessionMessage($message, 'good', false);
 
 		if($new_record) {
 			return $controller->redirect($this->Link());
@@ -585,7 +583,7 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler {
 
 			$this->record->delete();
 		} catch(ValidationException $e) {
-			$form->sessionMessage($e->getResult()->message(), 'bad');
+			$form->sessionMessage($e->getResult()->message(), 'bad', false);
 			return $this->getToplevelController()->redirectBack();
 		}
 
@@ -598,9 +596,9 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler {
 		$toplevelController = $this->getToplevelController();
 		if($toplevelController && $toplevelController instanceof LeftAndMain) {
 			$backForm = $toplevelController->getEditForm();
-			$backForm->sessionMessage($message, 'good');
+			$backForm->sessionMessage($message, 'good', false);
 		} else {
-			$form->sessionMessage($message, 'good');
+			$form->sessionMessage($message, 'good', false);
 		}
 
 		//when an item is deleted, redirect to the parent controller

security/MemberLoginForm.php

@@ -134,7 +134,6 @@ JS;
 	 * Get message from session
 	 */
 	protected function getMessageFromSession() {
-		parent::getMessageFromSession();
 		if(($member = Member::currentUser()) && !Session::get('MemberLoginForm.force_message')) {
 			$this->message = _t(
 				'Member.LOGGEDINAS',
@@ -143,6 +142,10 @@ JS;
 			);
 		}
 		Session::set('MemberLoginForm.force_message', false);
+
+		parent::getMessageFromSession();
+
+		return $this->message;
 	}
 
 

tests/forms/FormTest.php

@@ -484,6 +484,52 @@ class FormTest extends FunctionalTest {
 		$this->assertContains('three="3"', $form->getAttributesHTML('one', 'two'));
 	}
 
+	function testMessageEscapeHtml() {
+		$form = $this->getStubForm();
+		$form->Controller()->handleRequest(new SS_HTTPRequest('GET', '/'), DataModel::inst()); // stub out request
+		$form->sessionMessage('<em>Escaped HTML</em>', 'good', true);
+		$parser = new CSSContentParser($form->forTemplate());
+		$messageEls = $parser->getBySelector('.message');
+		$this->assertContains(
+			'&lt;em&gt;Escaped HTML&lt;/em&gt;',
+			$messageEls[0]->asXML()
+		);
+
+		$form = $this->getStubForm();
+		$form->Controller()->handleRequest(new SS_HTTPRequest('GET', '/'), DataModel::inst()); // stub out request
+		$form->sessionMessage('<em>Unescaped HTML</em>', 'good', false);
+		$parser = new CSSContentParser($form->forTemplate());
+		$messageEls = $parser->getBySelector('.message');
+		$this->assertContains(
+			'<em>Unescaped HTML</em>',
+			$messageEls[0]->asXML()
+		);
+	}
+
+	function testFieldMessageEscapeHtml() {
+		$form = $this->getStubForm();
+		$form->Controller()->handleRequest(new SS_HTTPRequest('GET', '/'), DataModel::inst()); // stub out request
+		$form->addErrorMessage('key1', '<em>Escaped HTML</em>', 'good', true);
+		$form->setupFormErrors();
+		$parser = new CSSContentParser($form->forTemplate());
+		$messageEls = $parser->getBySelector('#key1 .message');
+		$this->assertContains(
+			'&lt;em&gt;Escaped HTML&lt;/em&gt;',
+			$messageEls[0]->asXML()
+		);
+
+		$form = $this->getStubForm();
+		$form->Controller()->handleRequest(new SS_HTTPRequest('GET', '/'), DataModel::inst()); // stub out request
+		$form->addErrorMessage('key1', '<em>Unescaped HTML</em>', 'good', false);
+		$form->setupFormErrors();
+		$parser = new CSSContentParser($form->forTemplate());
+		$messageEls = $parser->getBySelector('#key1 .message');
+		$this->assertContains(
+			'<em>Unescaped HTML</em>',
+			$messageEls[0]->asXML()
+		);
+	}
+	
 	protected function getStubForm() {
 		return new Form(
 			new FormTest_Controller(),

tests/security/SecurityTest.php

@@ -299,7 +299,7 @@ class SecurityTest extends FunctionalTest {
 					$member->LockedOutUntil,
 					'User does not have a lockout time set if under threshold for failed attempts'
 				);
-				$this->assertContains($this->loginErrorMessage(), _t('Member.ERRORWRONGCRED'));
+				$this->assertContains($this->loginErrorMessage(), Convert::raw2xml(_t('Member.ERRORWRONGCRED')));
 			} else {
 				// Fuzzy matching for time to avoid side effects from slow running tests
 				$this->assertGreaterThan(
@@ -346,8 +346,9 @@ class SecurityTest extends FunctionalTest {
 			$this->doTestLoginForm('sam@silverstripe.com' , 'incorrectpassword');
 		}
 		$this->assertNull($this->session()->inst_get('loggedInAs'));
-		$this->assertTrue(
+		$this->assertContains(
-			false !== stripos($this->loginErrorMessage(), _t('Member.ERRORWRONGCRED')),
+			$this->loginErrorMessage(),
+			Convert::raw2xml(_t('Member.ERRORWRONGCRED')),
 			'The user can retry with a wrong password after the lockout expires'
 		);