Skip to content

Instantly share code, notes, and snippets.

@guilhermeblanco
Last active August 29, 2015 14:14
Show Gist options
  • Save guilhermeblanco/287e67fff9d1dcbc2f13 to your computer and use it in GitHub Desktop.
Save guilhermeblanco/287e67fff9d1dcbc2f13 to your computer and use it in GitHub Desktop.
Form deficiencies when dealing with GET requests

IntegerType does not properly validate in GET.

When creating a request like this: /page?page=1

And considering I have this in my FormType:

$builder->add('page', 'integer', array(
    'empty_data' => 1,
));

Every time I submit request information, it never validates and gives you constraint violations.

Create unnamed forms. While dealing with GET requests, I expect my query string to be like:

/page?page=1&results_per_page=50

This means I need to define a form like this one:

<?php

namespace Acme\DemoBundle\Form\Type;

use Symfony\Component\Form\AbstractType;
use Symfony\Component\Form\FormBuilderInterface;
use Symfony\Component\OptionsResolver\OptionsResolverInterface;

class PaginationType extends AbstractType
{
    public function buildForm(FormBuilderInterface $builder, array $options)
    {
        $builder
            ->add('page', 'text', array(
                'empty_data' => 1,
                'required' => false,
                'property_path' => 'currentPage',
            ))
            ->add('results_per_page', 'text', array(
                'empty_data' => 50,
                'required' => false,
                'property_path' => 'resultsPerPage',
            ));
    }

    public function getName()
    {
        return '';
    }

    public function setDefaultOptions(OptionsResolverInterface $resolver)
    {
        $resolver->setDefaults(array(
            'data_class' => 'Acme\DemoBundle\Form\Model\Pagination',
            'required' => true,
        ));
    }
}

The only allowed way to create an unamed form is by doing:

$form = $this->createForm(new PaginationType(), new Pagination());

Any attempt to define it as part of DIC such as:

services:
    acme_demo.form.type.pagination:
        class: 'Acme\DemoBundle\Form\Type\PaginationType'
        tags:
            - { name: 'form.type', alias: 'acme_demo_pagination' }

And when I try to consume it as a string by using its alias such as:

$form = $this->createForm('acme_demo_pagination', new Pagination());

Or even:

$form = $this->formFactory->createNamedBuilder('', 'acme_demo_pagination', new Pagination())->getForm();

You'll always get this error:

The type name specified for the service "acme_demo.form.type.pagination" does not match the actual name. Expected "acme_demo_pagination", given ""

Empty value not being considered when form key submission does not exist.

Consider the following GET request submission: /page?page=1

And also the FormType written in previous issue. When I inspect the element of Pagination, I'll always get:

class Pagination#1 (2) {
  public $currentPage =>
  int(1)
  public $resultsPerPage =>
  NULL
}

Validation of children not being considered during isValid().

Consider issue #3 and that I wrote a default value to Pagination#resultsPerPage as 'NaN' and that I write my validator to assert integer type.

Ideally, submitting only the page=1 querystring, I'd get the following Pagination instance:

class Pagination#1 (2) {
  public $currentPage =>
  int(1)
  public $resultsPerPage =>
  string(3) "NaN"
}

Now once I ask $form->isValid(), it returns true, even though Pagination#resultsPerPage added a violation to the Form. This happens because of this skip: https://github.com/symfony/symfony/blob/2.7/src/Symfony/Component/Form/Form.php#L814

Attributes are not being considered as part of GET request.

Consider the following route: /group/{group}/users

And the following request: /group/1/users?page=2

Problem happens that GET requests only consider $request->query->all() (or $_GET) parameters, and does not merge with $request->attributes->all()

Form to Model hydration should happen independently of form being submitted or not, but it must be $form->isSubmitted() === false if $request->getMethod() !== 'GET'

Let's consider the same FormType detailed in #2 and request: /page

It doesn't matter if I have set empty_data on all fields, Form never hydrates the Model. In the case of a GET request, you're ALWAYS submitting the form, it doesn't matter if you have 0, 1 or +Inf fields defined.

@stof
Copy link

stof commented Feb 4, 2015

For your issue 2, you are confusing the name of the form and the name of the form type. Form types must have unique names, and using an empty one will cause issues (and so is rejected). The form name can be empty:

$this->get('form.factory')->createNamed('', 'pagination', new Pagination());

@stof
Copy link

stof commented Feb 4, 2015

For your issue 4, you are confusing the form and the validator component. The form component only calls the validator on the root data and then let the validator decide whether it should cascade or no (which is controlled by the @Assert\Valid constraint)

@guilhermeblanco
Copy link
Author

@stof Sorry, but you may not have understood what I wrote there.
When $form->isValid() is called, it called for getErrors(), but it skips any constraints violation that were added to the child Form because the submitted data does not contain the key for that child submission. This turns the $form->isSubmitted() to return false and then error checking gets skipped, leading $form->isValid() to return true.
I highlighted the exact place where this happens.

@guilhermeblanco
Copy link
Author

Form not being submitted: symfony/symfony#13596

@guilhermeblanco
Copy link
Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment