Repository navigation
Conversation
A presigned PUT URL accepts a file of any size: only the UploadedToS3 rule checked the size, after the upload. A presigned POST with a content-length-range condition (bref.uploads.max_size) and an exact Content-Type condition lets S3 reject any other upload.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Draft: it needs a decision on backward compatibility first (see below).
Fixes the upload feature's main gap: a presigned PUT URL accepts a file of any size, only the
UploadedToS3rule checked the size after the upload. The route now returns a presigned POST form (Aws\S3\PostObjectV4) andbref-upload.jssends amultipart/form-dataPOST, so S3 itself rejects:bref.uploads.max_size(content-length-range, 400EntityTooLarge),Checked on a real bucket (sandbox) from Chromium with
bref-upload.js: 500 B and 1024 B uploads pass with a 1024 B limit, 1025 B and 5 MB fail withUploadErrorstatus 400 (not a network error), a tamperedContent-Typeorkeygets 403, the session token of role credentials is handled.Decisions made
bref.uploads.max_size(default 50 MB,null= no limit), now enforced by S3 too. No new config key. A per-ruleUploadedToS3(maxSize:)larger than it has no effect anymore (documented).application/octet-stream), like the agents did.AwsS3V3Adapter::getClient()), so it still needsleague/flysystem-aws-s3-v3, but so doUploadedToS3andStorage::copy(), and the docs already install it first. Building our ownS3Clientwould duplicate Laravel's disk config handling for no gain. A non-S3 upload disk now fails with a clearLogicException.fields(form fields) replacesheaders,urlis the bucket URL. The JSHttpClienttype only needspostnow.Backward compatibility: needs your call
This is a breaking change for existing 3.1 users, more than expected:
cors: <origin>shorthand (what our docs use) only allowsGET, PUT, DELETE. Every bucket set up from the docs, or by Vapor, rejects the browser's POST untilPOSTis added to its CORS rules. This hits users who import the JS fromvendor/too, i.e. uploads break oncomposer update.vendor:publish --tag=bref-uploadstill PUT tourl, which now fails.headersbreak.Options:
'uploads.method' => 'put'by default,'post'enables the S3 size limit; the JS POSTs when the response hasfields, PUTs otherwise. Docs recommendpostfor new apps, v4 makes it the default and removes PUT. More code until v4, nothing breaks.POSTto thecorsshorthand, so that the documented config works after a Lift update.The docs (
file-storage.mdx,migrating-from-vapor.mdx: CORS config, "sameurlandheadersfields as Vapor") will be updated in a bref PR once the option is chosen.