fix(storage): include metadata in uploadToSignedUrl request#2155
fix(storage): include metadata in uploadToSignedUrl request#2155MaxwellCalkin wants to merge 3 commits intosupabase:masterfrom
Conversation
The uploadToSignedUrl method accepted metadata in fileOptions but never used it. This copies the metadata handling from uploadOrUpdate into uploadToSignedUrl for all three body types (Blob, FormData, raw body). Also forwards custom fileOptions.headers, matching uploadOrUpdate behavior. Fixes supabase/storage#823
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR updates StorageFileApi to merge DEFAULT_FILE_OPTIONS before per-call options, capture fileOptions.metadata, and propagate metadata across upload paths. For Blob/FormData uploads metadata is added to FormData; for raw bodies metadata is base64-encoded and sent as an x-metadata header. Per-call headers are merged into base headers and duplex is forwarded/inferred for streaming bodies. Tests were added for uploadToSignedUrl to verify metadata handling for Blob, FormData, and raw body uploads. Sequence Diagram(s)sequenceDiagram
participant Client as Client (storage-js)
participant SignedURL as Signed URL endpoint
participant Storage as Storage Server
Client->>Client: Merge DEFAULT_FILE_OPTIONS + fileOptions\ncapture metadata
alt Blob or File
Client->>Client: create FormData\nappend file, cacheControl, metadata (if present)
Client->>SignedURL: PUT FormData (with metadata field)
else FormData passed in
Client->>Client: ensure cacheControl present\nappend metadata if missing
Client->>SignedURL: PUT FormData (with metadata field)
else Raw body/stream
Client->>Client: preserve headers\nadd x-metadata: base64(metadata) if present\nset duplex for streams
Client->>SignedURL: PUT body with headers (including x-metadata)
end
SignedURL->>Storage: forward upload
Storage->>SignedURL: store object with metadata
SignedURL-->>Client: response
Assessment against linked issues
Out-of-scope changes
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/storage-js/src/packages/StorageFileApi.ts`:
- Around line 282-296: The uploadToSignedUrl path fails to set fetch duplex for
stream bodies; update the uploadToSignedUrl function to detect Node readable
streams the same way uploadOrUpdate does and, when the body is a stream, pass
duplex: 'half' through to the put call (i.e., include { headers, duplex: 'half'
} or merge into the options argument you pass into put(this.fetch,
url.toString(), body, options)), ensuring the same stream detection/handling
logic and propagation used by uploadOrUpdate is reused for consistency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 20624c43-b34f-4b37-a09e-fbb8d7b7a7a0
📒 Files selected for processing (2)
packages/core/storage-js/src/packages/StorageFileApi.tspackages/core/storage-js/test/storageFileApi.test.ts
The raw-body path in uploadToSignedUrl was missing the stream detection and duplex: 'half' propagation that uploadOrUpdate already has. Without this, passing a ReadableStream or Node.js stream to uploadToSignedUrl fails on Node 20+ fetch which requires duplex: 'half' for stream bodies. Mirrors the exact pattern from uploadOrUpdate (lines 121-142) into uploadToSignedUrl for consistency. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@supabase/auth-js
@supabase/functions-js
@supabase/postgrest-js
@supabase/realtime-js
@supabase/storage-js
@supabase/supabase-js
commit: |
mandarini
left a comment
There was a problem hiding this comment.
Overall it's a solid PR. Let's remove this redundant line, and I will approve, after verifying with storage team. Thank you for the contribution!
| const headers: Record<string, string> = { | ||
| const options = { | ||
| ...DEFAULT_FILE_OPTIONS, | ||
| upsert: DEFAULT_FILE_OPTIONS.upsert, |
There was a problem hiding this comment.
I think we can remove this line as it's redundant.
Summary
Fixes supabase/storage#823.
uploadToSignedUrlacceptsmetadatainfileOptionsbut the implementation never reads or appends it to the request. Files uploaded via signed URLs silently drop custom metadata, unlike.upload()which works correctly.Root cause: The
uploadToSignedUrlmethod constructs the request body without checkingoptions.metadata, while theuploadOrUpdatehelper (used by.upload()and.update()) includes metadata handling for all three body types.Fix: Copy the metadata handling pattern from
uploadOrUpdateintouploadToSignedUrl:metadatafield to FormDatametadatafield (with!body.has()guard, matchinguploadOrUpdate)x-metadatabase64 headerAlso forwards
fileOptions.headersto the request, matchinguploadOrUpdatebehavior.Test plan
nx build storage-js)