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

This commit is contained in:
Damian Mooyman 2016-08-03 11:23:17 +12:00
parent 1c7d5de51b
commit 62a242154e
6 changed files with 125 additions and 82 deletions

View File

@ -4,7 +4,7 @@
*
* ASSUMPTION -> IF you pass your source as an array, you pass values as an array too. Likewise objects are handled
* the same.
*
*
* Example:
* <code>
* new CheckboxSetField(
@ -19,7 +19,7 @@
* $value = "1"
* );
* </code>
*
*
* <b>Saving</b>
* The checkbox set field will save its data in one of ways:
* - If the field name matches a many-many join on the object being edited, that many-many join will be updated to
@ -27,17 +27,17 @@
* the database records.
* - If the field name matches a database field, a comma-separated list of values will be saved to that field. The
* keys can be text or numbers.
*
*
* @todo Document the different source data that can be used
* with this form field - e.g ComponentSet, ArrayList,
* array. Is it also appropriate to accept so many different
* types of data when just using an array would be appropriate?
*
*
* @package forms
* @subpackage fields-basic
*/
class CheckboxSetField extends OptionsetField {
/**
* @var array
*/
@ -82,7 +82,7 @@ class CheckboxSetField extends OptionsetField {
}
}
}
// Source is not an array
if(!is_array($source) && !is_a($source, 'SQLMap')) {
if(is_array($values)) {
@ -126,19 +126,22 @@ class CheckboxSetField extends OptionsetField {
if(is_array($source)) {
unset($source['']);
}
$options = array();
if ($source == null) {
$source = array();
}
foreach($source as $value => $item) {
if($item instanceof DataObject) {
// 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);
@ -168,21 +171,21 @@ class CheckboxSetField extends OptionsetField {
* Default selections, regardless of the {@link setValue()} settings.
* Note: Items marked as disabled through {@link setDisabledItems()} can still be
* selected by default through this method.
*
*
* @param Array $items Collection of array keys, as defined in the $source array
*/
public function setDefaultItems($items) {
$this->defaultItems = $items;
return $this;
}
/**
* @return Array
*/
public function getDefaultItems() {
return $this->defaultItems;
}
/**
* Load a value into this CheckboxSetField
*/
@ -198,7 +201,7 @@ class CheckboxSetField extends OptionsetField {
return $this;
}
/**
* Save the current value of this CheckboxSetField into a DataObject.
* If the field it is saving to is a has_many or many_many relationship,
@ -228,11 +231,11 @@ class CheckboxSetField extends OptionsetField {
}
}
}
/**
* Return the CheckboxSetField value as a string
* Return the CheckboxSetField value as a string
* selected item keys.
*
*
* @return string
*/
public function dataValue() {
@ -243,30 +246,30 @@ class CheckboxSetField extends OptionsetField {
$filtered[] = str_replace(",", "{comma}", $item);
}
}
return implode(',', $filtered);
}
return '';
}
public function performDisabledTransformation() {
$clone = clone $this;
$clone->setDisabled(true);
return $clone;
}
/**
* Transforms the source data for this CheckboxSetField
* into a comma separated list of values.
*
*
* @return ReadonlyField
*/
public function performReadonlyTransformation() {
$values = '';
$data = array();
$items = $this->value;
if($this->source) {
foreach($this->source as $source) {
@ -275,7 +278,7 @@ class CheckboxSetField extends OptionsetField {
}
}
}
if($items) {
// Items is a DO Set
if($items instanceof SS_List) {
@ -283,13 +286,13 @@ class CheckboxSetField extends OptionsetField {
$data[] = $item->Title;
}
if($data) $values = implode(', ', $data);
// Items is an array or single piece of string (including comma seperated string)
} else {
if(!is_array($items)) {
$items = preg_split('/ *, */', trim($items));
}
foreach($items as $item) {
if(is_array($item)) {
$data[] = $item['Title'];
@ -301,23 +304,23 @@ class CheckboxSetField extends OptionsetField {
$data[] = $item;
}
}
$values = implode(', ', $data);
}
}
$field = $this->castedCopy('ReadonlyField');
$field->setValue($values);
return $field;
}
public function Type() {
return 'optionset checkboxset';
}
public function ExtraOptions() {
return FormField::ExtraOptions();
}
}

View File

@ -1,13 +1,13 @@
<?php
/**
* Set of radio buttons designed to emulate a dropdown.
*
* This field allows you to ensure that a form element is submitted is not optional and is part of a fixed set of
* data. This field uses the input type of radio. It's a direct subclass of {@link DropdownField},
*
* This field allows you to ensure that a form element is submitted is not optional and is part of a fixed set of
* data. This field uses the input type of radio. It's a direct subclass of {@link DropdownField},
* so the constructor and arguments are in the same format.
*
*
* <b>Usage</b>
*
*
* <code>
* new OptionsetField(
* $name = "Foobar",
@ -22,15 +22,15 @@
* $value = "1"
* );
* </code>
*
* You can use the helper functions on data object set to create the source array. eg:
*
*
* You can use the helper functions on data object set to create the source array. eg:
*
* <code>
* //Database request for the object
* $map = FooBar::get()->map();
* // returns an SS_Map object containing an array of ID => Title
*
* // Instantiate the OptionsetField
* // Instantiate the OptionsetField
* $FieldList = new FieldList(
* new OptionsetField(
* $name = "Foobar",
@ -42,16 +42,16 @@
*
* // Pass the fields to the form constructor. etc
* </code>
*
*
* @see CheckboxSetField for multiple selections through checkboxes instead.
* @see DropdownField for a simple <select> field with a single element.
* @see TreeDropdownField for a rich and customizeable UI that can visualize a tree of selectable elements
*
*
* @package forms
* @subpackage fields-basic
*/
class OptionsetField extends DropdownField {
/**
* @var Array
*/
@ -61,14 +61,19 @@ class OptionsetField extends DropdownField {
$source = $this->getSource();
$odd = 0;
$options = 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';
$extraClass .= ' val' . preg_replace('/[^a-zA-Z0-9\-\_]/', '_', $value);
$options[] = new ArrayData(array(
'ID' => $itemID,
'Class' => $extraClass,
@ -95,28 +100,28 @@ class OptionsetField extends DropdownField {
$field = $this->castedCopy('LookupField');
$field->setSource($this->getSource());
$field->setReadonly(true);
return $field;
}
/**
* Mark certain elements as disabled,
* regardless of the {@link setDisabled()} settings.
*
*
* @param array $items Collection of array keys, as defined in the $source array
*/
public function setDisabledItems($items) {
$this->disabledItems = $items;
return $this;
}
/**
* @return Array
*/
public function getDisabledItems() {
return $this->disabledItems;
}
public function ExtraOptions() {
return new ArrayList();
}

View File

@ -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>

View File

@ -1,7 +1,7 @@
<ul id="$ID" class="$extraClass">
<% 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 %>

View File

@ -5,21 +5,21 @@
* @subpackage tests
*/
class CheckboxSetFieldTest extends SapphireTest {
protected static $fixture_file = 'CheckboxSetFieldTest.yml';
protected $extraDataObjects = array(
'CheckboxSetFieldTest_Article',
'CheckboxSetFieldTest_Tag',
);
public function testSetDefaultItems() {
$f = new CheckboxSetField(
'Test',
false,
'Test',
false,
array(0 => 'Zero', 1 => 'One', 2 => 'Two', 3 => 'Three')
);
$f->setValue(array(0,1));
$f->setDefaultItems(array(2));
$p = new CSSContentParser($f->Field());
@ -48,68 +48,68 @@ class CheckboxSetFieldTest extends SapphireTest {
'Not selected by either value or default items'
);
}
public function testSaveWithNothingSelected() {
$article = $this->objFromFixture('CheckboxSetFieldTest_Article', 'articlewithouttags');
/* Create a CheckboxSetField with nothing selected */
$field = new CheckboxSetField("Tags", "Test field", DataObject::get("CheckboxSetFieldTest_Tag")->map());
/* Saving should work */
$field->saveInto($article);
$this->assertNull(
DB::query("SELECT *
FROM \"CheckboxSetFieldTest_Article_Tags\"
WHERE \"CheckboxSetFieldTest_Article_Tags\".\"CheckboxSetFieldTest_ArticleID\" = $article->ID
")->value(),
'Nothing should go into manymany join table for a saved field without any ticked boxes'
);
);
}
public function testSaveWithArrayValueSet() {
$article = $this->objFromFixture('CheckboxSetFieldTest_Article', 'articlewithouttags');
$articleWithTags = $this->objFromFixture('CheckboxSetFieldTest_Article', 'articlewithtags');
$tag1 = $this->objFromFixture('CheckboxSetFieldTest_Tag', 'tag1');
$tag2 = $this->objFromFixture('CheckboxSetFieldTest_Tag', 'tag2');
/* Create a CheckboxSetField with 2 items selected. Note that the array is in the format (key) => (selected) */
$field = new CheckboxSetField("Tags", "Test field", DataObject::get("CheckboxSetFieldTest_Tag")->map());
$field->setValue(array(
$tag1->ID => true,
$tag2->ID => true
));
/* Saving should work */
$field->saveInto($article);
$this->assertEquals(
array($tag1->ID,$tag2->ID),
array($tag1->ID,$tag2->ID),
DB::query("SELECT \"CheckboxSetFieldTest_TagID\"
FROM \"CheckboxSetFieldTest_Article_Tags\"
WHERE \"CheckboxSetFieldTest_Article_Tags\".\"CheckboxSetFieldTest_ArticleID\" = $article->ID
")->column(),
'Data shold be saved into CheckboxSetField manymany relation table on the "right end"'
);
);
$this->assertEquals(
array($articleWithTags->ID,$article->ID),
array($articleWithTags->ID,$article->ID),
DB::query("SELECT \"CheckboxSetFieldTest_ArticleID\"
FROM \"CheckboxSetFieldTest_Article_Tags\"
WHERE \"CheckboxSetFieldTest_Article_Tags\".\"CheckboxSetFieldTest_TagID\" = $tag1->ID
")->column(),
'Data shold be saved into CheckboxSetField manymany relation table on the "left end"'
);
);
}
public function testLoadDataFromObject() {
$article = $this->objFromFixture('CheckboxSetFieldTest_Article', 'articlewithouttags');
$articleWithTags = $this->objFromFixture('CheckboxSetFieldTest_Article', 'articlewithtags');
$tag1 = $this->objFromFixture('CheckboxSetFieldTest_Tag', 'tag1');
$tag2 = $this->objFromFixture('CheckboxSetFieldTest_Tag', 'tag2');
$field = new CheckboxSetField("Tags", "Test field", DataObject::get("CheckboxSetFieldTest_Tag")->map());
$form = new Form(
new Controller(),
new Controller(),
'Form',
new FieldList($field),
new FieldList()
@ -144,6 +144,27 @@ class CheckboxSetFieldTest extends SapphireTest {
$this->assertEquals('Test,Another', $dbValue);
}
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);
}
}
/**
@ -156,11 +177,11 @@ class CheckboxSetFieldTest_Article extends DataObject implements TestOnly {
private static $db = array(
"Content" => "Text",
);
private static $many_many = array(
"Tags" => "CheckboxSetFieldTest_Tag",
);
}
/**
@ -168,7 +189,7 @@ class CheckboxSetFieldTest_Article extends DataObject implements TestOnly {
* @subpackage tests
*/
class CheckboxSetFieldTest_Tag extends DataObject implements TestOnly {
private static $belongs_many_many = array(
'Articles' => 'CheckboxSetFieldTest_Article'
);

View File

@ -6,11 +6,11 @@
class OptionsetFieldTest extends SapphireTest {
public function testSetDisabledItems() {
$f = new OptionsetField(
'Test',
false,
'Test',
false,
array(0 => 'Zero', 1 => 'One')
);
$f->setDisabledItems(array(0));
$p = new CSSContentParser($f->Field());
$item0 = $p->getBySelector('#Test_0');
@ -34,4 +34,18 @@ class OptionsetFieldTest extends SapphireTest {
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);
}
}