Skip to content

fix: Add multipart form-data support for Laravel HTTP server#200

Open
homiedopie wants to merge 6 commits intopestphp:4.xfrom
homiedopie:fix/multipart-form-parser
Open

fix: Add multipart form-data support for Laravel HTTP server#200
homiedopie wants to merge 6 commits intopestphp:4.xfrom
homiedopie:fix/multipart-form-parser

Conversation

@homiedopie
Copy link

@homiedopie homiedopie commented Feb 16, 2026 •
edited
Loading

Problem

  • Multipart form-data requests were not fully parsed in the Laravel HTTP server driver.
  • As a result, uploaded files and nested multipart fields were not reliably available in the Laravel Request object.

Fix

  • Added multipart parsing support using Amp form parser and mapped payloads to Symfony/Laravel-compatible request structures.
  • Implemented normalization for nested fields and uploaded files, and converted uploaded parts into Laravel UploadedFile instances.

Test

  • URL-encoded form body
  • Multipart form with file uploads (from: @gtg-bantonio)
  • Multipart form with nested fields (from: @gtg-bantonio)

Verification

  • Verified my SPA test cases using form-data

Note

  • With the help of Github Copilot (GPT 5.3 Codex)

Related

Related #177 - Thanks to @gtg-bantonio
Fixes #1495

@homiedopie homiedopie mentioned this pull request Feb 16, 2026
Open
@homiedopie homiedopie marked this pull request as draft February 16, 2026 04:59
@homiedopie homiedopie marked this pull request as ready for review February 16, 2026 06:04
@gtg-bantonio
Copy link

gtg-bantonio commented Feb 23, 2026

Good day, I detect an error when you don't select a file: The browser send a file with the header filename empty, and in the parser it should return a file with the UPLOAD_ERR_NO_FILE flag set, not return a default value.

https://github.com/pestphp/pest-plugin-browser/pull/200/changes#diff-b56f173c966197dc61e8e52e50e147d3ed935d600b16d78bc2507808a9a441f7R538-R542

Additionally, I notice that other corner cases like when the file is greater that the max file size defined in the INI file (UPLOAD_ERR_INI_SIZE and UPLOAD_ERR_FORM_SIZE)

@homiedopie
Copy link
Author

homiedopie commented Feb 25, 2026

@gtg-bantonio Thanks for the feedback. Ill check it out and will add some test as well!

@homiedopie
Copy link
Author

homiedopie commented Feb 26, 2026

@gtg-bantonio I addressed it on the new fixes I have added. Can you please check on your end? Thank you!

@homiedopie homiedopie force-pushed the fix/multipart-form-parser branch from 7527459 to 52ad5dd Compare February 26, 2026 00:09
@homiedopie homiedopie force-pushed the fix/multipart-form-parser branch from 52ad5dd to 6e72b2e Compare February 26, 2026 00:35
@gtg-bantonio
Copy link

gtg-bantonio commented Feb 26, 2026

I found another bug, with nested indexed fields in a linear sequence:
Let's assume the browser send the following multipart data (in URL-encoded notation to don't make it too long)

extra = 'Test'
data[0][field1] = 1
data[0][field2] = 1
data[1][field1] = 2
data[1][field2] = 0

The decoded data should be (in PHP array notation)

[
  'extra' => 'Test',
  'data' => [
    0 => [
      'field1' => '1',
      'field2' => '1',
    ],
    1 => [
      'field1' => '2',
      'field2' => '0',
    ],
  ],
]

But instead the parser returns:

[
  'extra' => 'Test',
  'data' => [
    0 => [
      'field1' => '1',
      'field2' => ['0', '1'],
    ],
    1 => [
      'field1' => '2',
      'field2' => '0',
    ],
  ],
]

@akulmehta
Copy link

akulmehta commented Feb 26, 2026 •
edited
Loading

Can I ask, how is this PR different from the other PR #177 ? Confused on which one to follow.

@gtg-bantonio
Copy link

gtg-bantonio commented Feb 26, 2026

@akulmehta in this PR, the library that use is the amphp/http-server-form-parser with some manipulation to respect the PHP $_POST structure, an in my PR #177 I extract the ReactPHP MultipartParser that parse directly to the correct structure, but with some maintenance burden

@homiedopie
Copy link
Author

homiedopie commented Feb 26, 2026

@gtg-bantonio - thanks for explaining it! I addressed the issue already and separated the test (dedicated to multipart upload/parameters)

@gtg-bantonio
Copy link

gtg-bantonio commented Feb 26, 2026

I can't found another error with our full script, and the tests and code are better that mine, so I will close my PR in favor of yours

@homiedopie homiedopie force-pushed the fix/multipart-form-parser branch from b5a29af to f0400c0 Compare February 26, 2026 21:47
@gtg-bantonio gtg-bantonio mentioned this pull request Feb 26, 2026
Closed
@homiedopie
Copy link
Author

homiedopie commented Feb 26, 2026

@gtg-bantonio - Thanks again for your help! wouldnt be possible without your guidance and inputs!

@bluesheep100 bluesheep100 mentioned this pull request Mar 4, 2026
Closed
bluesheep100
bluesheep100 reviewed Mar 5, 2026
View reviewed changes
src/Drivers/LaravelHttpServer.php
$cookies = array_map(fn (RequestCookie $cookie): string => urldecode($cookie->getValue()), $request->getCookies());
$cookies = array_merge($cookies, test()->prepareCookiesForRequest()); // @phpstan-ignore-line
/** @var array<string, string> $serverVariables */
$serverVariables = test()->serverVariables(); // @phpstan-ignore-line
Copy link

@bluesheep100 bluesheep100 Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$_SERVER['CONTENT_TYPE'] is still inconsistent with $request->header('content-type'), when checked in the Laravel end.
You can fix this by adding $serverVariables['CONTENT_TYPE'] = $contentType; right here, and verify with this test:

it('matches content-type in server variables when uploading files', function (): void {
    Route::get('/form', fn (): string => '
        <html>
        <head></head>
        <body>
            <form method="post" action="/content-type" enctype="multipart/form-data">
                <label for="file1">A file</label>
                <input id="file1" type="file" name="file1">

                <button type="submit">Send</button>
            </form>
        </body>
        </html>
    ');
    Route::post(
        '/content-type',
        fn (Request $request): string => $request->server('CONTENT_TYPE') === $request->header('content-type') ? 'true' : 'false',
    );

    visit('/form')
        ->attach('A file', fixture('lorem-ipsum.txt'))
        ->screenshot(true, 'OOF')
        ->click('Send')
        ->assertSee('true');
});

Copy link
Author

@homiedopie homiedopie Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bluesheep100 - I added one for content length as well. Tests are added. Can you please check on your end? Thank you for your feedback!

Copy link

@bluesheep100 bluesheep100 Mar 5, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Content type is still missing from $_SERVER; apparently I had it wrong how it works.
However looking into Symfony\Component\HttpFoundation\Request, I can see it has an overrideGlobals() method.

It seems most correct to call that after setting the request headers, and it will update $_SERVER appropriately, instead of handling it manually on a case-by-case basis.

Here's the relevant snippet:

foreach ($this->headers->all() as $key => $value) {
    $key = strtoupper(str_replace('-', '_', $key));
    if (\in_array($key, ['CONTENT_TYPE', 'CONTENT_LENGTH', 'CONTENT_MD5'], true)) {
        $_SERVER[$key] = implode(', ', $value);
    } else {
        $_SERVER['HTTP_'.$key] = implode(', ', $value);
    }
}

EDIT: Can confirm that $symfonyRequest->overrideGlobals() also makes my file uploading browser tests pass.

Copy link
Author

@homiedopie homiedopie Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bluesheep100 - This has been addressed, I added a case for content_md5 as well to cover all bases. Please check thanks!

@homiedopie homiedopie requested a review from bluesheep100 March 5, 2026 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bluesheep100 bluesheep100 Awaiting requested review from bluesheep100

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: Request payload is empty when form enctype is multipart/form-data

4 participants

@homiedopie @gtg-bantonio @akulmehta @bluesheep100