From ac2504715baf5d866a37167d7caaf22aafd830ff Mon Sep 17 00:00:00 2001 From: Rias Date: Fri, 6 Feb 2026 10:26:16 +0100 Subject: [PATCH 1/2] fix: prevent JS asset truncation for files larger than 8KB MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The asset() method used fread() to read JS files, but fread() is limited to 8192 bytes per call when running inside Amp's event loop. This caused every JS file larger than 8KB to be silently truncated. Additionally, the rewritten content was written to a php://temp stream and served via ReadableResourceStream, which calls stream_socket_shutdown() on close — undefined behavior for non-socket streams that could also cause data loss. Replace fopen/fread with file_get_contents to read the full file, and return the string body directly in the Response. The Content-Type header is set via setHeader() after construction to avoid the constructor's setHeaders() call clearing the auto-set Content-Length. --- src/Drivers/LaravelHttpServer.php | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/Drivers/LaravelHttpServer.php b/src/Drivers/LaravelHttpServer.php index 97ae5fdb..b2d290f2 100644 --- a/src/Drivers/LaravelHttpServer.php +++ b/src/Drivers/LaravelHttpServer.php @@ -329,21 +329,23 @@ private function asset(string $filepath): Response $contentType = $contentType[0] ?? 'application/octet-stream'; if (str_ends_with($filepath, '.js')) { - $temporaryStream = fopen('php://temp', 'r+'); - assert($temporaryStream !== false, 'Failed to open temporary stream.'); + // Use file_get_contents instead of fread() because fread() is + // limited to 8192 bytes when running inside Amp's event loop. + fclose($file); + $rawContent = file_get_contents($filepath); - // @phpstan-ignore-next-line - $temporaryContent = fread($file, (int) filesize($filepath)); + assert($rawContent !== false, 'Failed to read file content.'); - assert($temporaryContent !== false, 'Failed to open temporary stream.'); + $content = $this->rewriteAssetUrl($rawContent); - $content = $this->rewriteAssetUrl($temporaryContent); + // Build the Response without passing headers to the constructor, + // because the constructor calls setBody() first (which auto-sets + // content-length for string bodies) then setHeaders() (which + // clears ALL headers and only keeps what was passed in). + $response = new Response(200, [], $content); + $response->setHeader('Content-Type', $contentType); - fwrite($temporaryStream, $content); - - rewind($temporaryStream); - - $file = $temporaryStream; + return $response; } return new Response(200, [ From 1ae34be9b066a5b7d982633dec4deeb6b1cd2a86 Mon Sep 17 00:00:00 2001 From: Rias Date: Fri, 6 Feb 2026 10:54:45 +0100 Subject: [PATCH 2/2] test: add regression tests for JS asset serving fix Two reflection-based tests that deterministically fail with the old fread()+ReadableResourceStream implementation and pass with the fix: - Verify JS assets return string body (ReadableBuffer) not stream - Verify content-length header is set correctly Plus a Playwright-based test serving a >8KB JS file end-to-end. --- src/Drivers/LaravelHttpServer.php | 6 +-- .../Drivers/Laravel/LaravelHttpServerTest.php | 54 +++++++++++++++++++ 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/src/Drivers/LaravelHttpServer.php b/src/Drivers/LaravelHttpServer.php index b2d290f2..ca7a9bfb 100644 --- a/src/Drivers/LaravelHttpServer.php +++ b/src/Drivers/LaravelHttpServer.php @@ -99,12 +99,12 @@ public function start(): void return; } - $this->socket = $server = SocketHttpServer::createForDirectAccess(new NullLogger()); + $this->socket = $server = SocketHttpServer::createForDirectAccess(new NullLogger); $server->expose("{$this->host}:{$this->port}"); $server->start( new ClosureRequestHandler($this->handleRequest(...)), - new DefaultErrorHandler(), + new DefaultErrorHandler, ); } @@ -323,7 +323,7 @@ private function asset(string $filepath): Response return new Response(404); } - $mimeTypes = new MimeTypes(); + $mimeTypes = new MimeTypes; $contentType = $mimeTypes->getMimeTypes(pathinfo($filepath, PATHINFO_EXTENSION)); $contentType = $contentType[0] ?? 'application/octet-stream'; diff --git a/tests/Unit/Drivers/Laravel/LaravelHttpServerTest.php b/tests/Unit/Drivers/Laravel/LaravelHttpServerTest.php index 7491d6a6..5622fbf0 100644 --- a/tests/Unit/Drivers/Laravel/LaravelHttpServerTest.php +++ b/tests/Unit/Drivers/Laravel/LaravelHttpServerTest.php @@ -2,7 +2,10 @@ declare(strict_types=1); +use Amp\ByteStream\ReadableBuffer; +use Amp\ByteStream\ReadableResourceStream; use Illuminate\Http\Request; +use Pest\Browser\Drivers\LaravelHttpServer; use function Pest\Laravel\withServerVariables; use function Pest\Laravel\withUnencryptedCookie; @@ -21,6 +24,57 @@ ->assertDontSee('http://localhost'); }); +it('serves JS assets as string body instead of stream', function (): void { + // Regression test: the old implementation used fread() + ReadableResourceStream + // which could truncate JS files at 8192 bytes inside Amp's event loop. + // The fix uses file_get_contents() and returns the content as a string body + // (ReadableBuffer) instead of a stream (ReadableResourceStream). + $path = public_path('string-body-test.js'); + @file_put_contents($path, "console.log('ok');"); + + $server = new LaravelHttpServer('127.0.0.1', 0); + + $method = new ReflectionMethod($server, 'asset'); + + $response = $method->invoke($server, $path); + + $body = $response->getBody(); + + expect($body)->toBeInstanceOf(ReadableBuffer::class) + ->and($body)->not->toBeInstanceOf(ReadableResourceStream::class); +}); + +it('sets content-length header on JS asset responses', function (): void { + // Regression test: the old implementation used ReadableResourceStream which + // caused Response::setBody() to remove the content-length header. + $jsContent = "console.log('content-length test');"; + $path = public_path('cl-test.js'); + @file_put_contents($path, $jsContent); + + $server = new LaravelHttpServer('127.0.0.1', 0); + + $method = new ReflectionMethod($server, 'asset'); + + $response = $method->invoke($server, $path); + + expect($response->getHeader('content-length'))->toBe((string) strlen($jsContent)) + ->and($response->getHeader('content-type'))->toBe('text/javascript'); +}); + +it('serves large JS files completely', function (): void { + // Regression test: fread() can return fewer bytes than requested inside + // Amp's event loop (limited to 8192 per read). Verify files >8KB are served intact. + $padding = str_repeat("// padding line to exceed 8192 bytes\n", 300); + $jsContent = $padding."window.__pest_large_file_marker = 'COMPLETE';"; + + @file_put_contents(public_path('large.js'), $jsContent); + + Route::get('/large-js-test', fn (): string => ''); + + visit('/large-js-test') + ->assertScript('window.__pest_large_file_marker', 'COMPLETE'); +}); + it('includes cookies set in the test', function (): void { Route::get('/cookies', fn (Request $request): array => $request->cookies->all());