Skip to content

[Form] broken submit behavior with zero indexed collection items. #52318

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.

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

Closed
jan-pintr opened this issue Oct 27, 2023 · 3 comments
Closed

[Form] broken submit behavior with zero indexed collection items. #52318

jan-pintr opened this issue Oct 27, 2023 · 3 comments

Comments

@jan-pintr
Copy link

Symfony version(s) affected

6.3.6

Description

#52021 introduced an issue when form collection contains zero indexed item.

When submitting form with zero indexed collection item its values are reindexed.

There was an attempt to fix this in b769b77 but it was not enough for this case. As the original issue is closed. There is new one.

How to reproduce

See reproducer with test case and proposed change:
https://github.com/symfony/symfony/compare/6.4...jan-pintr:symfony-form-zero-indexed-collection-reporoducer:form-zero-indexed-col-reproducer?expand=1

Possible Solution

Reproducer contains proposed change to FormUtil::mergeParamsAndFiles() function.

Additional Context

No response

@xabbuh
Copy link
Member

xabbuh commented Oct 27, 2023

Can you please check if this is the same as (or similar to) #52248 and was also fixed by #52255?

@jan-pintr
Copy link
Author

It is pretty much same as #52248 but this case was not fixed by #52248 or #52255.

@nicolas-grekas
Copy link
Member

Can you please send a PR so that we can discuss on the patch?

@fabpot fabpot closed this as completed Oct 28, 2023
fabpot added a commit that referenced this issue Oct 28, 2023
This PR was merged into the 5.4 branch.

Discussion
----------

[Form] Fix merging form data and files (ter)

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix #52318
| License       | MIT

Adapted from a patch provided by `@jan`-pintr

The original was like below but this one looks more appropriate to me.

<details>

```diff
--- a/src/Symfony/Component/Form/Util/FormUtil.php
+++ b/src/Symfony/Component/Form/Util/FormUtil.php
@@ -47,8 +47,12 @@ public static function isEmpty(mixed $data): bool
     public static function mergeParamsAndFiles(array $params, array $files): array
     {
         if (array_is_list($files)) {
-            foreach ($files as $value) {
-                $params[] = $value;
+            foreach ($files as $key => $value) {
+                if (is_array($params[$key])) {
+                    $params[$key] = array_merge($params[$key], $value);
+                } else {
+                    $params[] = $value;
+                }
             }

             return $params;
@@ -61,6 +65,6 @@ public static function mergeParamsAndFiles(array $params, array $files): array
             }
         }

-        return array_replace($params, $files);
+        return array_merge($params, $files);
     }```

</details>

Commits
-------

9facb2f [Form] Fix merging form data and files (ter)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants