[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:
Bernhard Schussek 2012-01-17 00:02:58 +01:00
parent 87b16e7015
commit e1fc5a5c8c
10 changed files with 361 additions and 327 deletions

View File

@ -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(),
);

View File

@ -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();

View File

@ -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
));
}
}
}

View File

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

View File

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

View File

@ -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());
}
}

View File

@ -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');

View File

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

View File

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