[ss-2016-015] Fix value / title escaping in CheckboxSetField and Opti…

…onsetField
silverstripe · Aug 15, 2016 · 049cdef · 049cdef
1 parent fa7f5af
commit 049cdef
Show file tree

Hide file tree

Showing 6 changed files with 48 additions and 5 deletions.
diff --git a/forms/CheckboxSetField.php b/forms/CheckboxSetField.php
@@ -132,11 +132,14 @@ public function getOptions() {
 		}
 
 		foreach($source as $value => $item) {
+			// Ensure $title is cast for template
 			if($item instanceof DataObject) {
 				$value = $item->ID;
-				$title = $item->Title;
-			} else {
+				$title = $item->obj('Title');
+			} elseif ($item instanceof DBField) {
 				$title = $item;
+			} else {
+				$title = DBField::create_field('Text', $item);
 			}
 
 			$itemID = $this->ID() . '_' . preg_replace('/[^a-zA-Z0-9]/', '', $value);

diff --git a/forms/OptionsetField.php b/forms/OptionsetField.php
@@ -62,6 +62,11 @@ public function Field($properties = array()) {
 
 		if($source) {
 			foreach($source as $value => $title) {
+				// Ensure $title is safely cast
+				if ( !($title instanceof DBField) ) {
+					$title = DBField::create_field('Text', $title);
+				}
+
 				$itemID = $this->ID() . '_' . preg_replace('/[^a-zA-Z0-9]/', '', $value);
 				$odd = ($odd + 1) % 2;
 				$extraClass = $odd ? 'odd' : 'even';

diff --git a/templates/forms/CheckboxSetField.ss b/templates/forms/CheckboxSetField.ss
@@ -2,9 +2,9 @@
 	<% if $Options.Count %>
 		<% loop $Options %>
 			<li class="$Class">
-				<input id="$ID" class="checkbox" name="$Name" type="checkbox" value="$Value"<% if $isChecked %> checked="checked"<% end_if %><% if $isDisabled %> disabled="disabled"<% end_if %> />
+				<input id="$ID" class="checkbox" name="$Name" type="checkbox" value="$Value.ATT"<% if $isChecked %> checked="checked"<% end_if %><% if $isDisabled %> disabled="disabled"<% end_if %> />
 				<label for="$ID">$Title</label>
-			</li> 
+			</li>
 		<% end_loop %>
 	<% else %>
 		<li>No options available</li>

diff --git a/templates/forms/OptionsetField.ss b/templates/forms/OptionsetField.ss
@@ -1,7 +1,7 @@
 <ul $AttributesHTML>
 	<% loop $Options %>
 		<li class="$Class">
-			<input id="$ID" class="radio" name="$Name" type="radio" value="$Value"<% if $isChecked %> checked<% end_if %><% if $isDisabled %> disabled<% end_if %> />
+			<input id="$ID" class="radio" name="$Name" type="radio" value="$Value.ATT"<% if $isChecked %> checked<% end_if %><% if $isDisabled %> disabled<% end_if %> />
 			<label for="$ID">$Title</label>
 		</li>
 	<% end_loop %>

diff --git a/tests/forms/CheckboxSetFieldTest.php b/tests/forms/CheckboxSetFieldTest.php
@@ -206,6 +206,27 @@ public function testValidationWithDataList() {
 		);
 	}
 
+	public function testSafelyCast() {
+		$member = new Member();
+		$member->FirstName = '<firstname>';
+		$member->Surname = '<surname>';
+		$member->write();
+		$field1 = new CheckboxSetField('Options', 'Options', array(
+			'one' => 'One',
+			'two' => 'Two & Three',
+			'three' => DBField::create_field('HTMLText', 'Four &amp; Five &amp; Six'),
+			$member
+		));
+		$fieldHTML = (string)$field1->Field();
+		$this->assertContains('One', $fieldHTML);
+		$this->assertContains('Two &amp; Three', $fieldHTML);
+		$this->assertNotContains('Two & Three', $fieldHTML);
+		$this->assertContains('Four &amp; Five &amp; Six', $fieldHTML);
+		$this->assertNotContains('Four & Five & Six', $fieldHTML);
+		$this->assertContains('&lt;firstname&gt;', $fieldHTML);
+		$this->assertNotContains('<firstname>', $fieldHTML);
+	}
+
 }
 
 /**

diff --git a/tests/forms/OptionsetFieldTest.php b/tests/forms/OptionsetFieldTest.php
@@ -63,4 +63,18 @@ public function testReadonlyField() {
 		preg_match('/Yes/', $field->Field(), $matches);
 		$this->assertEquals($matches[0], 'Yes');
 	}
+
+	public function testSafelyCast() {
+		$field1 = new OptionsetField('Options', 'Options', array(
+			1 => 'One',
+			2 => 'Two & Three',
+			3 => DBField::create_field('HTMLText', 'Four &amp; Five &amp; Six')
+		));
+		$fieldHTML = (string)$field1->Field();
+		$this->assertContains('One', $fieldHTML);
+		$this->assertContains('Two &amp; Three', $fieldHTML);
+		$this->assertNotContains('Two & Three', $fieldHTML);
+		$this->assertContains('Four &amp; Five &amp; Six', $fieldHTML);
+		$this->assertNotContains('Four & Five & Six', $fieldHTML);
+	}
 }