Skip to content
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

[11.x] Correctly parse multi dimensional arrays in multipart http requests #53028

Closed
wants to merge 7 commits into from

Conversation

RomainMazB
Copy link

@RomainMazB RomainMazB commented Oct 4, 2024

When sending multi dimensional arrays inside an http requests, the framework currently doesn't handle the conversion and results as a failure A 'contents' key is required.

This failure is described here in #37174 and results from this Guzzle issue guzzle/guzzle#1679.

With this PR, the framework will now handles multi-dimensional arrays and convert it into properly formed one level multipart form data array.

Meaning this array:

[
  'users' => [
    ['name' => 'Taylor', 'car' => 'Lambo'],
    ['name' => 'Romain', 'car' => 'Not Lambo']
  ]
];

Will be converted into this array, which will be correctly handled by Guzzle:

[
  ['name' => 'users[0][name]', 'contents' => 'Taylor']
  ['name' => 'users[0][car]', 'contents' => 'Lambo']
  ['name' => 'users[1][name]', 'contents' => 'Romain']
  ['name' => 'users[1][car]', 'contents' => 'Not Lambo']
]

The trick used for the conversion is inspired by a comment in the Guzzle issue.

The existing testCanSendMultipartDataWithBothSimplifiedAndExtendedParameters test was modified only because the conversion re-order the multipart fields, the already expanded fields are passed before the simple ones.

This fields reorganization doesn't affect the success of the request.

I also added a test to mix file upload with nested array extra data.
As an example, this kind of Http call is needed to use this SignNow API endpoint.

@RomainMazB RomainMazB changed the title Correctly parse multi dimensional arrays in multipart http requests [11.x] Correctly parse multi dimensional arrays in multipart http requests Oct 4, 2024
@taylorotwell
Copy link
Member

Seems like something that should be fixed in Guzzle if it needs fixing?

@RomainMazB
Copy link
Author

RomainMazB commented Oct 4, 2024

Seems like something that should be fixed in Guzzle if it needs fixing?

I agree and that's what I wanted to do at the beginning of my investigations.

The problem @taylorotwell is that we started managing complex multipart form inside Laravel with this modification c72697d .

If I now solve this from the Guzzle part which I am ok to do, it will conflict with this changes.

This "already expanded" array, which is accepted "as-is" by both the Laravel Client and Guzzle for now:

[
  ['name' => 'first_name', 'contents' => 'Romain']
]

will results into this after the Guzzle transformation:

[
  ['name' => 'name', 'contents' => 'first_name'],
  ['name' => 'contents', 'contents' => 'Romain'],
]

If I do the PR in Guzzle, would you mind rollback these change and leave the whole multipart formatting part to Guzzle?

Also as a sidenote, Symfony also handles this outside of Guzzle here: symfony/symfony@6cc2c29.

@RomainMazB
Copy link
Author

RomainMazB commented Oct 4, 2024

Bonus information.

According to the Guzzle documentation, the multipart option, described here, the contents value is not meant to accept an array.

I am really unsure that this would be accepted as a PR in Guzzle. It looks like it remains as a low level interface and leaves to the users the conversion work

Should Laravel continue to live without being able to pass data along with a file?

@mahlenko
Copy link

Maybe my decision will suit you too. I pre-encode all arrays in json. In my case, it worked.

@RomainMazB
Copy link
Author

RomainMazB commented Oct 19, 2024

Maybe my decision will suit you too. I pre-encode all arrays in json. In my case, it worked.

It can suit if you are the one in charge to handle this data because you can the decode the json.

This feature is needed in many service API like the SignNow API.
https://docs.signnow.com/docs/signnow/document/operations/create-a-document-fieldextract

In such case, I just can't do what I want and have to respect the API needs: a file and a formdata-structured nested array

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants