[Form] Restricted form names to specific characters to (1) fix generation of HTML IDs and to (2) avoid problems with property paths.
ad (1): HTML4 "id" attributes are limited to strings starting with a letter and containing only letters, digits, underscores, hyphens, periods and colons. ad (2): Property paths contain three special characters needed for correct parsing: left/right bracket and period. The rules for form naming are: * Names may start with a letter, a digit or an underscore. Leading digits or underscores will be stripped from the "id" attributes. * Names must only contain letters, digits, underscores, hyphens and colons. * Root forms may have an empty name. Solves #1919 and #3021 on a wider scope.
This commit is contained in:
parent
87b16e7015
commit
e1fc5a5c8c
@ -25,7 +25,7 @@ class CollectionType extends AbstractType
|
||||
public function buildForm(FormBuilder $builder, array $options)
|
||||
{
|
||||
if ($options['allow_add'] && $options['prototype']) {
|
||||
$prototype = $builder->create('$$' . $options['prototype_name'] . '$$', $options['type'], $options['options']);
|
||||
$prototype = $builder->create($options['prototype_name'], $options['type'], $options['options']);
|
||||
$builder->setAttribute('prototype', $prototype->getForm());
|
||||
}
|
||||
|
||||
@ -78,7 +78,7 @@ class CollectionType extends AbstractType
|
||||
'allow_add' => false,
|
||||
'allow_delete' => false,
|
||||
'prototype' => true,
|
||||
'prototype_name' => 'name',
|
||||
'prototype_name' => '__name__',
|
||||
'type' => 'text',
|
||||
'options' => array(),
|
||||
);
|
||||
|
@ -88,6 +88,11 @@ class FieldType extends AbstractType
|
||||
} else {
|
||||
$id = $name;
|
||||
$fullName = $name;
|
||||
|
||||
// Strip leading underscores and digits. These are allowed in
|
||||
// form names, but not in HTML4 ID attributes.
|
||||
// http://www.w3.org/TR/html401/struct/global.html#adef-id
|
||||
$id = ltrim($id, '_0123456789');
|
||||
}
|
||||
|
||||
$types = array();
|
||||
|
@ -193,6 +193,10 @@ class Form implements \IteratorAggregate, FormInterface
|
||||
$required = false, $readOnly = false, $errorBubbling = false,
|
||||
$emptyData = null, array $attributes = array())
|
||||
{
|
||||
$name = (string) $name;
|
||||
|
||||
self::validateName($name);
|
||||
|
||||
foreach ($clientTransformers as $transformer) {
|
||||
if (!$transformer instanceof DataTransformerInterface) {
|
||||
throw new UnexpectedTypeException($transformer, 'Symfony\Component\Form\DataTransformerInterface');
|
||||
@ -211,7 +215,7 @@ class Form implements \IteratorAggregate, FormInterface
|
||||
}
|
||||
}
|
||||
|
||||
$this->name = (string) $name;
|
||||
$this->name = $name;
|
||||
$this->dispatcher = $dispatcher;
|
||||
$this->types = $types;
|
||||
$this->clientTransformers = $clientTransformers;
|
||||
@ -1055,4 +1059,32 @@ class Form implements \IteratorAggregate, FormInterface
|
||||
|
||||
return $value;
|
||||
}
|
||||
|
||||
/**
|
||||
* Validates whether the given variable is a valid form name.
|
||||
*
|
||||
* A name is accepted if it
|
||||
*
|
||||
* * is empty
|
||||
* * starts with a letter, digit or underscore
|
||||
* * contains only letters, digits, numbers, underscores ("_"),
|
||||
* hyphens ("-") and colons (":")
|
||||
*
|
||||
* @param string $name The tested form name.
|
||||
* @throws UnexpectedTypeException If the name is not a string.
|
||||
* @throws \InvalidArgumentException If the name contains invalid characters.
|
||||
*/
|
||||
static public function validateName($name)
|
||||
{
|
||||
if (!is_string($name)) {
|
||||
throw new UnexpectedTypeException($name, 'string');
|
||||
}
|
||||
|
||||
if ($name !== '' && !preg_match('/^[a-zA-Z0-9_][a-zA-Z0-9_\-:]*$/D', $name)) {
|
||||
throw new \InvalidArgumentException(sprintf(
|
||||
'The name "%s" contains illegal characters. Names should start with a letter, digit or underscore and only contains letters, digits, numbers, underscores ("_"), hyphens ("-") and colons (":").',
|
||||
$name
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -123,6 +123,10 @@ class FormBuilder
|
||||
*/
|
||||
public function __construct($name, FormFactoryInterface $factory, EventDispatcherInterface $dispatcher, $dataClass = null)
|
||||
{
|
||||
$name = (string) $name;
|
||||
|
||||
Form::validateName($name);
|
||||
|
||||
$this->name = $name;
|
||||
$this->factory = $factory;
|
||||
$this->dispatcher = $dispatcher;
|
||||
|
@ -11,6 +11,8 @@
|
||||
|
||||
namespace Symfony\Component\Form\Util;
|
||||
|
||||
use Symfony\Component\Form\Exception\UnexpectedTypeException;
|
||||
|
||||
abstract class FormUtil
|
||||
{
|
||||
/**
|
||||
|
File diff suppressed because it is too large
Load Diff
@ -125,7 +125,7 @@ class CollectionFormTest extends TypeTestCase
|
||||
'prototype' => false,
|
||||
));
|
||||
|
||||
$this->assertFalse($form->has('$$name$$'));
|
||||
$this->assertFalse($form->has('__name__'));
|
||||
}
|
||||
|
||||
public function testPrototypeMultipartPropagation()
|
||||
@ -150,7 +150,7 @@ class CollectionFormTest extends TypeTestCase
|
||||
));
|
||||
|
||||
$data = $form->getData();
|
||||
$this->assertFalse(isset($data['$$name$$']));
|
||||
$this->assertFalse(isset($data['__name__']));
|
||||
}
|
||||
|
||||
public function testGetDataDoesNotContainsPrototypeNameAfterDataAreSet()
|
||||
@ -163,7 +163,7 @@ class CollectionFormTest extends TypeTestCase
|
||||
|
||||
$form->setData(array('foobar.png'));
|
||||
$data = $form->getData();
|
||||
$this->assertFalse(isset($data['$$name$$']));
|
||||
$this->assertFalse(isset($data['__name__']));
|
||||
}
|
||||
|
||||
public function testPrototypeNameOption()
|
||||
@ -174,15 +174,15 @@ class CollectionFormTest extends TypeTestCase
|
||||
'allow_add' => true,
|
||||
));
|
||||
|
||||
$this->assertSame('$$name$$', $form->getAttribute('prototype')->getName(), '$$name$$ is the default');
|
||||
$this->assertSame('__name__', $form->getAttribute('prototype')->getName(), '__name__ is the default');
|
||||
|
||||
$form = $this->factory->create('collection', null, array(
|
||||
'type' => 'field',
|
||||
'prototype' => true,
|
||||
'allow_add' => true,
|
||||
'prototype_name' => 'test',
|
||||
'prototype_name' => '__test__',
|
||||
));
|
||||
|
||||
$this->assertSame('$$test$$', $form->getAttribute('prototype')->getName());
|
||||
$this->assertSame('__test__', $form->getAttribute('prototype')->getName());
|
||||
}
|
||||
}
|
||||
|
@ -117,6 +117,16 @@ class FieldTypeTest extends TypeTestCase
|
||||
$this->assertEquals('name', $view->get('full_name'));
|
||||
}
|
||||
|
||||
public function testStripLeadingUnderscoresAndDigitsFromId()
|
||||
{
|
||||
$form = $this->factory->createNamed('field', '_09name');
|
||||
$view = $form->createView();
|
||||
|
||||
$this->assertEquals('name', $view->get('id'));
|
||||
$this->assertEquals('_09name', $view->get('name'));
|
||||
$this->assertEquals('_09name', $view->get('full_name'));
|
||||
}
|
||||
|
||||
public function testPassIdAndNameToViewWithParent()
|
||||
{
|
||||
$parent = $this->factory->createNamed('field', 'parent');
|
||||
|
@ -36,6 +36,37 @@ class FormBuilderTest extends \PHPUnit_Framework_TestCase
|
||||
$this->builder = null;
|
||||
}
|
||||
|
||||
public function getHtml4Ids()
|
||||
{
|
||||
// The full list is tested in FormTest, since both Form and FormBuilder
|
||||
// use the same implementation internally
|
||||
return array(
|
||||
array('#', false),
|
||||
array('a ', false),
|
||||
array("a\t", false),
|
||||
array("a\n", false),
|
||||
array('a.', false),
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider getHtml4Ids
|
||||
*/
|
||||
public function testConstructAcceptsOnlyNamesValidAsIdsInHtml4($name, $accepted)
|
||||
{
|
||||
try {
|
||||
new FormBuilder($name, $this->factory, $this->dispatcher);
|
||||
if (!$accepted) {
|
||||
$this->fail(sprintf('The value "%s" should not be accepted', $name));
|
||||
}
|
||||
} catch (\InvalidArgumentException $e) {
|
||||
// if the value was not accepted, but should be, rethrow exception
|
||||
if ($accepted) {
|
||||
throw $e;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Changing the name is not allowed, otherwise the name and property path
|
||||
* are not synchronized anymore
|
||||
|
@ -60,6 +60,62 @@ class FormTest extends \PHPUnit_Framework_TestCase
|
||||
new Form('name', $this->dispatcher, array(), array(), array(), null, $validators);
|
||||
}
|
||||
|
||||
public function getHtml4Ids()
|
||||
{
|
||||
return array(
|
||||
array('a0', true),
|
||||
array('a9', true),
|
||||
array('z0', true),
|
||||
array('A0', true),
|
||||
array('A9', true),
|
||||
array('Z0', true),
|
||||
array('#', false),
|
||||
array('a#', false),
|
||||
array('a$', false),
|
||||
array('a%', false),
|
||||
array('a ', false),
|
||||
array("a\t", false),
|
||||
array("a\n", false),
|
||||
array('a-', true),
|
||||
array('a_', true),
|
||||
array('a:', true),
|
||||
// Periods are allowed by the HTML4 spec, but disallowed by us
|
||||
// because they break the generated property paths
|
||||
array('a.', false),
|
||||
// Contrary to the HTML4 spec, we allow names starting with a
|
||||
// number, otherwise naming fields by collection indices is not
|
||||
// possible.
|
||||
// For root forms, leading digits will be stripped from the
|
||||
// "id" attribute to produce valid HTML4.
|
||||
array('0', true),
|
||||
array('9', true),
|
||||
// Contrary to the HTML4 spec, we allow names starting with an
|
||||
// underscore, since this is already a widely used practice in
|
||||
// Symfony2.
|
||||
// For root forms, leading underscores will be stripped from the
|
||||
// "id" attribute to produce valid HTML4.
|
||||
array('_', true),
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider getHtml4Ids
|
||||
*/
|
||||
public function testConstructAcceptsOnlyNamesValidAsIdsInHtml4($name, $accepted)
|
||||
{
|
||||
try {
|
||||
new Form($name, $this->dispatcher);
|
||||
if (!$accepted) {
|
||||
$this->fail(sprintf('The value "%s" should not be accepted', $name));
|
||||
}
|
||||
} catch (\InvalidArgumentException $e) {
|
||||
// if the value was not accepted, but should be, rethrow exception
|
||||
if ($accepted) {
|
||||
throw $e;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
public function testDataIsInitializedEmpty()
|
||||
{
|
||||
$norm = new FixedDataTransformer(array(
|
||||
|
Reference in New Issue
Block a user