fix: prevent JS asset truncation for files larger than 8KB #199
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
LaravelHttpServer::asset()where files >8KB could be silently truncatedfread()+ReadableResourceStreamwhich has two bugs:fread()returns at most 8,192 bytes per call inside Amp's event loop, andReadableResourceStreamcauses thecontent-lengthheader to be strippedfile_get_contents()and returns the content as a string body directly, which correctly setscontent-lengthand avoids truncationThe Bug
The
asset()method served JS files through this pipeline:Three issues:
fread()short-read — Inside Amp's non-blocking event loop,fread()returns at most 8,192 bytes per call, not the full filecontent-length—Response::setBody(ReadableStream)callsremoveHeader('content-length'), forcing chunked transfer encodingstream_socket_shutdown()onphp://temp—ReadableResourceStream::close()callsstream_socket_shutdown()on non-socket streams (undefined behavior)This caused Craft CMS Control Panel pages to fail loading because their JS bundles (~50KB+) were truncated to 8KB.
The Fix
file_get_contents()reads the entire file regardless of event loop contextReadableBuffer→ correctcontent-lengthheaderTests
Three regression tests added:
serves JS assets as string body instead of stream— Verifies response body isReadableBuffer(notReadableResourceStream) via reflection. Fails with old codesets content-length header on JS asset responses— Verifiescontent-lengthheader matches file size. Fails with old codeserves large JS files completely— End-to-end test via Playwright: creates >8KB JS file, loads it in a page, verifies a marker variable at the end of the file is accessible