Skip to content

Can't transform array data in PreSubmit event or DataTransformer #47093

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
ghost opened this issue Jul 28, 2022 · 5 comments
Closed

Can't transform array data in PreSubmit event or DataTransformer #47093

ghost opened this issue Jul 28, 2022 · 5 comments
Labels
Bug Form Stalled Status: Needs Review

Comments

@ghost
Copy link

ghost commented Jul 28, 2022 •
edited by ghost
Loading

Symfony version(s) affected

5.4.9 and above

Description

In Form.php there is a check whether the submitted data is an array and if that is the case it is mandated that the form is compound or has the multiple attribute set to true. This check is done before handling the PRE_SUBMIT event, which prevents transforming the submitted array to a single value in an event handler (or other data transformers).

How to reproduce

<?php

use Symfony\Component\Form\AbstractType;
use Symfony\Component\Form\Extension\Core\Type\TextType;
use Symfony\Component\Form\FormBuilderInterface;
use Symfony\Component\Form\FormEvent;
use Symfony\Component\Form\FormEvents;

class F extends AbstractType {
    public function buildForm(FormBuilderInterface $builder, array $options)
    {
        $builder->add('foo', TextType::class);
        $builder->get('foo')->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $e) {
            $e->setData($e->getData()['bar']);
        });
    }
}
// …
$f = $this->createForm(F::class);
$f->submit(['foo' => ['bar' => 'buz']]);

Possible Solution

Move the check for compound or multiple, after handling PRE_SUBMIT and other data transformation has been handled.

Additional Context

In my case data is submitted as json via XMLHttpRequest rather than Html Forms.

symfony/form < 5.4.9 would unintendedly allow arrays if the multiple attribute was present but false.
@ghost ghost added the Bug label Jul 28, 2022
@ghost ghost changed the title Can't transform array data in PreSubmit event or Data Can't transform array data in PreSubmit event or DataTransformer Jul 28, 2022
@xabbuh xabbuh added the Form label Jul 29, 2022
@yceruto
Copy link
Member

yceruto commented Dec 27, 2022 •
edited
Loading

Compound form types expect an array or null on submission, while scalar form types expect a string or null. However, I think you have a valid point. The PRE_SUBMIT event is meant to change the content of the submitted data and we currently allow this for compound forms before checking whether it's an array or not. Therefore, for consistency, we should allow the same for non-compound forms.

It seems like a bug to me and after #29307, some PRE_SUBMIT events stopped being called. For example, the ChoiceType required a test update here #46424 creating inconsistency with the error message.

As for the solution, I agree to move the submitted data check just after the PRE_SUBMIT block. At least for these two:

if ($this->config->getRequestHandler()->isFileUpload($submittedData)) {
    if (!$this->config->getOption('allow_file_upload')) {
        $submittedData = null;
        throw new TransformationFailedException('Submitted data was expected to be text or number, file upload given.');
    }
} elseif (\is_array($submittedData) && !$this->config->getCompound() && !$this->config->getOption('multiple', false)) {
    $submittedData = null;
    throw new TransformationFailedException('Submitted data was expected to be text or number, array given.');
}

Handling this should be easy and it should also resolve the mentioned inconsistency, but I might miss something, ping @nicolas-grekas, and @xabbuh as you worked on these two PRs, what do you think? Do you consider this a bug?

@carsonbot
Copy link

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

@carsonbot
Copy link

Could I get a reply or should I close this?

@carsonbot
Copy link

Hey,

I didn't hear anything so I'm going to close it. Feel free to comment if this is still relevant, I can always reopen!

@ghost
Copy link
Author

ghost commented Aug 16, 2023

I don't think this issue is resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Form Stalled Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@xabbuh @yceruto @carsonbot