[Validator] GroupSequence stopping at first group, never using the second

[damienalexandre]

Symfony version(s) affected: v3.4.40

Description

I'm using Symfony v3.4.37 and tried to upgrade to v3.4.40, but it looks like the GroupSequence validation_groups is not working as expected anymore.

How to reproduce

Here is a simple test to reproduce:

<?php

namespace Tests\AppBundle\Controller;

use Symfony\Component\Form\Extension\Core\Type\EmailType;
use Symfony\Component\Form\Extension\Core\Type\FormType;
use Symfony\Component\Form\FormInterface;
use Symfony\Component\Validator\Constraints\Email;
use Symfony\Component\Validator\Constraints\GroupSequence;
use Symfony\Component\Validator\Constraints\Length;

class GroupSequenceTest extends WebTestCase
{
    public function testWithoutSequence()
    {
        parent::createClient();

        $formBuilder = self::$kernel->getContainer()->get('form.factory')->createBuilder(FormType::class, [], [
            'csrf_protection' => false,
            'validation_groups' => ['First', 'Second']
        ]);

        /** @var FormInterface $form */
        $form = $formBuilder
            ->add('foobar', EmailType::class, [
                'constraints' => [
                    new Email([
                        'strict' => true,
                        'groups' => ['First'],
                    ]),
                    new Length([
                        'groups' => ['Second'],
                        'max' => 3
                    ]),
                ],
            ])
            ->getForm();

        $form->submit(['foobar' => 'test@example.org']);

        $this->assertFalse($form->isValid());
    }

    public function testWithGroupSequence()
    {
        parent::createClient();

        $formBuilder = self::$kernel->getContainer()->get('form.factory')->createBuilder(FormType::class, [], [
            'csrf_protection' => false,
            'validation_groups' => new GroupSequence(['First', 'Second'])
        ]);

        /** @var FormInterface $form */
        $form = $formBuilder
            ->add('foobar', EmailType::class, [
                'constraints' => [
                    new Email([
                        'strict' => true,
                        'groups' => ['First'],
                    ]),
                    new Length([
                        'groups' => ['Second'],
                        'max' => 3
                    ]),
                ],
            ])
            ->getForm();

        $form->submit(['foobar' => 'test@example.org']);

        $this->assertFalse($form->isValid());
    }
}

The first one use two groups, the form is not valid which is what we expect.

The second test use the sames groups in a GroupSequence, this is where I see a difference between Symfony version:

• on Symfony v3.4.37 the test is green, the two validators are checked
• on Symfony v3.4.40 the test is red, only the first validator is checked

Possible Solution

None.

Additional context

Tested on (yes, I know 😬):

PHP 7.0.33 (cli) (built: Dec  7 2018 16:18:01) ( NTS )
Copyright (c) 1997-2017 The PHP Group
Zend Engine v3.0.0, Copyright (c) 1998-2017 Zend Technologies
    with Zend OPcache v7.0.33, Copyright (c) 1999-2017, by Zend Technologies
    with blackfire v1.28.4~linux-musl-x64-non_zts70, https://blackfire.io, by Blackfire

[xabbuh]

Thank you for the great example. I can reproduce it and I am working on a fix for it.

[xabbuh]

WIP PR is here: #36865

@damienalexandre Your use case should already be fixed by the current state of the PR, but I need to do some more adaptions.

[xabbuh]

@damienalexandre Can you confirm that #36865 fixes your issue (it should be ready now)?

[lyrixx]

Hello @xabbuh

I applied your patch on our project, and unfortunately, this is worst.
Here is the log line when the test failed:

[2020-05-27 14:39:25] request.CRITICAL: Uncaught PHP Exception PHPUnit\Framework\Error\Warning: "Illegal offset type" at /home/ouibus/ouibus/vendor/symfony/symfony/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php line 133 {"exception":"[object] (PHPUnit\Framework\Error\Warning(code: 2): Illegal offset type at /home/ouibus/ouibus/vendor/symfony/symfony/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php:133)"} {"token":null,"url":"/recherche?origin=BRZ&destination=PAR&outboundDate=2020-05-30&passengers%5B0%5D%5Btype%5D=A","ip":"127.0.0.1","http_method":"GET","server":"fr.ouibus.test","referrer":null,"uid":"0493a2a"}

The faulty line:

foreach ($form->all() as $field) {
    if ($field->isSubmitted()) {
        $this->resolvedGroups[$field] = $groups; // line 133
        $fieldFormConstraint = new Form();
        $validator->atPath(sprintf($fieldPropertyPath, $field->getPropertyPath()))->validate($field, $fieldFormConstraint);
    }
}

[xabbuh]

@lyrixx Can you extract a failing test case?

[lyrixx]

Hello @xabbuh

I took time to dig it.
The issue is that at some point $this->resolvedGroups is null. If I remove the null assignation, it's fine (but we now have a memory leak)

I also managed to create a reproducer. Actually, it's super easy : 2 fields... I guess there is an issue with "deepness" 'I guess because of if $form->isRoot().

Anyway, here is the reproducer :
You need to take my branch, composer install, and use you fork :)

Thanks for taking time to dig in this issue, I don't know so much about this component :/

[xabbuh]

@lyrixx Thanks, I am going to look into it tomorrow.