Form rendering throws notice if the form is submitted with array instead of string #23995

mhujer · 2017-08-26T12:02:24Z

Q	A
Bug report?	yes
Feature request?	no
BC Break report?	no
RFC?	no
Symfony version	3.3.6

When the user uses Chrome Inspect feature and changes the <input type="text" name to include an extra array (e.g. from article_form[title] to article_form[title][foobar], the form fails to render. The reason is that the template is trying to render the "invalid" array as an <input> value.

Twig_Error_Runtime:
An exception has been thrown during the rendering of a template ("Notice: Array to string conversion").

  at vendor\symfony\symfony\src\Symfony\Bridge\Twig\Resources\views\Form\form_div_layout.html.twig:13
  at Twig_Template->displayBlock('form_widget_simple', array('value' => array('foo' => 'aa'), 'attr' => array(), 'form' => object(FormView), 'id' => 'article_form_title', 'name' => 'title', 'full_name' => 'article_form[title]', 'disabled' => false, 'label' => 'Article title', 'label_format' => null, 'multipart' => false, 'block_prefixes' => array('form', 'text', '_article_form_title'), 'unique_block_prefix' => '_article_form_title', 'translation_domain' => null, 'cache_key' => '_article_form_title_text', 'errors' => object(FormErrorIterator), 'valid' => false, 'data' => array('foo' => 'aa'), 'required' => true, 'size' => null, 'label_attr' => array(), 'compound' => false, 'method' => 'POST', 'action' => '', 'submitted' => true, 'app' => object(AppVariable)), array('form_widget' => array(object(__TwigTemplate_74fc98e978cb4218b308d1fa4d08196af014f2b1069a5f65da3ed6ecc38bdf83), 'block_form_widget'),
...

The problematic line in the template is this:

symfony/src/Symfony/Bridge/Twig/Resources/views/Form/form_div_layout.html.twig

Line 13 in 3a9653d

    
           <input type="{{ type }}" {{ block('widget_attributes') }} {% if value is not empty %}value="{{ value }}" {% endif %}/>

The simplest repro-case I managed to create is this one:

    public function createAction()
    {

        $form = $this->createFormBuilder(
            [
                'title' => [
                    'foo' => 'bar',
                ],
            ])
            ->add('title', TextType::class)
            ->getForm();

        return $this->render('article/add-article.html.twig', [
            'form' => $form->createView(),
        ]);
    }

# article/add-article.html.twig
{% block body %}
	{{ form(form) }}
{% endblock %}

The text was updated successfully, but these errors were encountered:

linaori · 2017-08-26T12:22:52Z

So that means the page would result in a 500 error because the user is messing with the HTML?

mhujer · 2017-08-26T12:35:26Z

No, I jumped to conclusion too fast. It only reports a notice. Which is converted to exception in dev mode.

In prod mode it converts the data to string Array:

It happens also on the Symfony.com: http://symfony.com/search?q[foo]=form.

But if the developer forgets to add @Assert\Type(type="string"), the form may validate just fine and pass the array further in the app. Which may actually be happening - the Symfony web searches for term Array.

(I checked several webapps and most of them are prone to this issue)

sstok · 2017-08-26T13:12:07Z

I'm not entirely sure how we can fix this this properly, we could add a safe guard in the themes (which will not fix this issue for custom themes).

Or we can add a transformer to ensure if a none scalar value is provided it's transformed to an empty string. But this could be considered a BC break as some may have added additional transformer(s) that expect this kind of behavior 🤔

mhujer · 2017-08-26T13:30:14Z

@iltar I'm afraid I found a case when it fails with 500.

Can someone please check that I haven't overlooked something?

When there is another assert - such as @Assert\Email() or @Assert\Length(min="10", max="100") which expects string-ish value, they do this check:

        if (!is_scalar($value) && !(is_object($value) && method_exists($value, '__toString'))) {
            throw new UnexpectedTypeException($value, 'string');
        }

So if the $value is of type array, exception is throw and the error page is rendered.

The Type validator is called (if it is before other validators - such as in the example bellow), but it does not prevent the other validators from being called.

Symfony\Component\Validator\Exception\UnexpectedTypeException:
Expected argument of type "string", "array" given

  at vendor\symfony\symfony\src\Symfony\Component\Validator\Constraints\LengthValidator.php:37
  at Symfony\Component\Validator\Constraints\LengthValidator->validate(array('foo' => 'aaaa'), object(Length))
     (vendor\symfony\symfony\src\Symfony\Component\Validator\Validator\RecursiveContextualValidator.php:850)
  at Symfony\Component\Validator\Validator\RecursiveContextualValidator->validateInGroup(array('foo' => 'aaaa'), null, object(GenericMetadata), 'Default', object(ExecutionContext))
     (vendor\symfony\symfony\src\Symfony\Component\Validator\Validator\RecursiveContextualValidator.php:696)
  at Symfony\Component\Validator\Validator\RecursiveContextualValidator->validateGenericNode(array('foo' => 
...

Case for reproduction (you need to change input name from form[title] to form[title][foo]:

    public function createyAction(Request $request)
    {
        $form = $this->createFormBuilder()
            ->add('title', TextType::class, [
                'constraints' => [
                    new Type('string'),
                    new Length(['min' => 10]),
                ],
            ])
            ->getForm();

        $form->handleRequest($request);
        if ($form->isSubmitted() && $form->isValid()) {
            //foo
        }

        return $this->render('article/add-article.html.twig', [
            'form' => $form->createView(),
        ]);
    }

# article/add-article.html.twig
{% block body %}
	{{ form(form) }}
{% endblock %}

mhujer · 2017-08-26T14:52:09Z

Related: #14943 and #20935

linaori · 2017-08-26T19:04:14Z

Honestly, if a user messes with the input fields, a 500 is justified. Won't have to expect valid responses if you send invalid requests

mhujer · 2017-08-27T13:37:00Z

For me it seems unexpected from the DX point-of-view: If the proper @Assert\Type() annotations are added, it should work like any other validation.

And I think it may be possible to flood the server logs with this error.

techi602 · 2017-08-28T17:39:09Z

@iltar you can not justify error 500 for unhandled user input validation (error 400 bad request would be more suitable in this case - yet still embarrassing for simple form validation)
Imagine hackbots which will spam your application error logs with random input field names.

ianmclaughlin · 2017-09-15T07:53:41Z

HTTP 400 (Bad Request) is for client errors, so it's inappropriate to return 500

Our logs are being filled with exceptions which don't need to be thrown

Tobion · 2017-10-11T01:34:13Z

Related #4102

xabbuh · 2018-09-23T11:56:53Z

closing in favour of #4102

mhujer changed the title ~~Form rendering fails if the form is submitted with array instead of string~~ Form rendering throws notice if the form is submitted with array instead of string Aug 26, 2017

ogizanagi added the Form label Aug 26, 2017

xabbuh mentioned this issue Sep 14, 2017

[Validator] Wrong error handling when type differs #24196

Closed

ro0NL mentioned this issue Sep 15, 2017

String validators (email, regex etc) throw exceptions when validating an array #24214

Closed

xabbuh closed this as completed Sep 23, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Form rendering throws notice if the form is submitted with array instead of string #23995

Form rendering throws notice if the form is submitted with array instead of string #23995

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Form rendering throws notice if the form is submitted with array instead of string #23995

Form rendering throws notice if the form is submitted with array instead of string #23995

Comments

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!