feat: allow region override based on request#1187
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
WalkthroughThis change introduces dynamic region selection for file uploads, allowing the upload region to be chosen based on user location. It adds a new documentation section, updates middleware and internal types to support region metadata, and adjusts internal logic to propagate and utilize a preferred region throughout the upload process. Several exports are updated to expose the experimental region selection feature. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Middleware
participant Handler
participant IngestUrl
participant StorageRegion
User->>Middleware: Initiate upload request (with headers)
Middleware->>Middleware: Extract continent from headers
Middleware->>Middleware: Map continent to region
Middleware->>Handler: Pass preferredRegion in metadata
Handler->>IngestUrl: Request URL with preferredRegion
IngestUrl->>StorageRegion: Select region based on preferredRegion
StorageRegion-->>IngestUrl: Return region URL
IngestUrl-->>Handler: Return ingest URL
Handler-->>User: Provide upload URL
Suggested labels
Suggested reviewers
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
⏰ Context from checks skipped due to timeout of 90000ms (9)
🔇 Additional comments (3)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
juliusmarminge
added
the
❗ infra changes required
More templates
commit: |
juliusmarminge
added
the
✅ infra changes implemented
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
packages/uploadthing/src/_internal/handler.ts (2)
328-335: 🛠️ Refactor suggestion
originfield should be validated / normalised before use
originarrives from the signed payload, which gives integrity but not semantic correctness:
- an empty string or something that is not a valid absolute URL will throw later when
prependUrlconcatenates it- an unexpected protocol (e.g.
http,ftp) could bypass TLS expectations- a value like
https://evil.com@ingest.uploadthing.comis syntactically valid and passes a naïve “startsWith” testConsider defensive parsing right after decoding:
+ // Validate that `origin` is a well-formed https URL before we rely on it + let safeOrigin: string | null = null; + try { + const url = new URL(requestInput.origin); + if (url.protocol !== "https:") { + throw new Error("origin must use https"); + } + safeOrigin = url.origin; // normalised form (drops path / query) + } catch (err) { + yield* Effect.logError("Invalid origin in callback") + .pipe(Effect.annotateLogs("origin", requestInput.origin)); + return yield* new UploadThingError({ + code: "BAD_REQUEST", + message: "Invalid origin", + }); + }…and then pass
safeOrigindownstream.
[security]
387-395: 🛠️ Refactor suggestionUse the validated
originwhen building the callback URLFollowing the previous comment, the call site should reference the normalised/validated value instead of
requestInput.origindirectly:- HttpClientRequest.prependUrl(requestInput.origin), + HttpClientRequest.prependUrl(safeOrigin),This prevents a malicious value from leaking into an outbound request if the validation ever returns early.
[security]
🧹 Nitpick comments (8)
packages/uploadthing/src/fastify.ts (1)
12-19: Consistent experimental export of UTRegion
Theexperimental_UTRegionsymbol is correctly re-exported with a clear JSDoc warning, aligning with other platform adapters and enabling the region override feature.Consider also exporting the
UTRegionAliastype to provide developers with compile-time validation of region codes. For example:export { UTFiles, /** * This is an experimental feature. * You need to be feature flagged on our backend to use this */ UTRegion as experimental_UTRegion, } from "./_internal/types"; +export type { UTRegionAlias } from "./_internal/types";This addition will make the set of valid region string literals discoverable in IDEs.
packages/uploadthing/src/effect-platform.ts (1)
13-20: Add experimental UTRegion export
ExportingUTRegionasexperimental_UTRegionwith a JSDoc notice is correct and consistent with other adapters.Optionally export the
UTRegionAliastype to help users leverage TypeScript’s type safety:export { UTFiles, /** * This is an experimental feature. * You need to be feature flagged on our backend to use this */ UTRegion as experimental_UTRegion, } from "./_internal/types"; +export type { UTRegionAlias } from "./_internal/types";This will make region code types discoverable and assist with IDE autocompletion.
packages/uploadthing/src/express.ts (1)
17-24: Expose experimental region key for Express adapter
You've correctly re-exportedUTRegionasexperimental_UTRegionwith appropriate JSDoc, aligning with the new dynamic region feature.To aid TypeScript users, consider also exporting the associated region code type:
export { UTFiles, /** * This is an experimental feature. * You need to be feature flagged on our backend to use this */ UTRegion as experimental_UTRegion, } from "./_internal/types"; +export type { UTRegionAlias } from "./_internal/types";This provides compile-time validation of allowed region strings.
packages/uploadthing/src/remix.ts (1)
12-19: Add experimental UTRegion export in Remix adapter
Theexperimental_UTRegionexport is correctly added with a JSDoc warning, consistent with other adapters for enabling dynamic region override.Optionally include the
UTRegionAliastype in exports to expose valid region string literals:export { UTFiles, /** * This is an experimental feature. * You need to be feature flagged on our backend to use this */ UTRegion as experimental_UTRegion, } from "./_internal/types"; +export type { UTRegionAlias } from "./_internal/types";This will improve developer ergonomics when specifying regions.
packages/uploadthing/src/h3.ts (1)
12-19: Experimental region support for H3 adapter
Correctly re-exportedUTRegionasexperimental_UTRegionwith JSDoc, maintaining consistency and enabling the region override feature.Consider also exporting the region code type
UTRegionAliasfor better type discovery and validation:export { UTFiles, /** * This is an experimental feature. * You need to be feature flagged on our backend to use this */ UTRegion as experimental_UTRegion, } from "./_internal/types"; +export type { UTRegionAlias } from "./_internal/types";This ensures developers have access to the allowed region codes in their code editor.
playground/app/api/uploadthing/route.ts (1)
30-45: Consider improving the fallback logic for region selectionThe current implementation is good but has a subtle issue with the fallback logic when a header is missing. When the
x-vercel-ip-continentheader is missing, it relies on uppercasingundefinedand then looks up "US" which isn't directly in the mapping. This works because it falls back to "sea1", but it's not clear from the code.- const region = ( - { - AF: "fra1", - AN: "sea1", - AS: "bom1", - EU: "fra1", - NA: "sea1", - OC: "sea1", - SA: "sea1", - } as const - )[opts.req.headers.get("x-vercel-ip-continent")?.toUpperCase() ?? "US"]!; + const continentMap = { + AF: "fra1", + AN: "sea1", + AS: "bom1", + EU: "fra1", + NA: "sea1", + OC: "sea1", + SA: "sea1", + } as const; + + const continent = opts.req.headers.get("x-vercel-ip-continent")?.toUpperCase(); + const region = continent ? (continentMap[continent] ?? "sea1") : "sea1";packages/uploadthing/src/_internal/handler.ts (2)
469-474:preferredRegionshould be type-checked and whitelisted
metadata[UTRegion]is currently typed asunknown(symbolindex signature).
Passing it unchecked toIngestUrlrelies on that function falling back safely, but adding an explicit guard here improves debuggability and prevents weird states:- preferredRegion: metadata[UTRegion], + preferredRegion: + typeof metadata[UTRegion] === "string" && metadata[UTRegion].length > 0 + ? metadata[UTRegion] + : undefined,Optionally, verify it exists in
UTToken.regionsto fail fast instead of silently falling back.
507-512: Destructuring is fine – consider naming consistencyThe variable is called
preferredRegion, while the exported public API symbol isexperimental_UTRegionelsewhere. Aligning the internal naming (preferredRegion) with the external (utRegion,experimentalRegion…) could save a quick mental mapping for newcomers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
docs/src/app/(docs)/concepts/regions-acl/multiregion.pngis excluded by!**/*.png
📒 Files selected for processing (14)
docs/src/app/(docs)/concepts/regions-acl/page.mdx(1 hunks)packages/uploadthing/src/_internal/config.ts(1 hunks)packages/uploadthing/src/_internal/handler.ts(6 hunks)packages/uploadthing/src/_internal/types.ts(2 hunks)packages/uploadthing/src/effect-platform.ts(1 hunks)packages/uploadthing/src/express.ts(1 hunks)packages/uploadthing/src/fastify.ts(1 hunks)packages/uploadthing/src/h3.ts(1 hunks)packages/uploadthing/src/next-legacy.ts(1 hunks)packages/uploadthing/src/next.ts(1 hunks)packages/uploadthing/src/remix.ts(1 hunks)packages/uploadthing/src/sdk/utils.ts(1 hunks)packages/uploadthing/src/server.ts(1 hunks)playground/app/api/uploadthing/route.ts(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/uploadthing/src/sdk/utils.ts (1)
packages/uploadthing/src/_internal/config.ts (1)
IngestUrl(85-99)
packages/uploadthing/src/_internal/handler.ts (2)
packages/uploadthing/src/_internal/types.ts (1)
UTRegion(37-37)packages/uploadthing/src/_internal/config.ts (1)
IngestUrl(85-99)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: analyze-bundle (current-pr)
- GitHub Check: lint
- GitHub Check: e2e-node (backend-adapters)
- GitHub Check: build
- GitHub Check: e2e-node (minimal-pagedir)
- GitHub Check: e2e-node (minimal-appdir)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (14)
packages/uploadthing/src/sdk/utils.ts (1)
87-87: The explicit undefined parameter adapts to the updated IngestUrl signature.This change passes
undefinedas thepreferredRegionparameter to the updatedIngestUrlfunction, maintaining backward compatibility while supporting the new region override feature.packages/uploadthing/src/server.ts (1)
12-19: Good documentation for the experimental feature export.The export of
UTRegionasexperimental_UTRegionis properly marked with JSDoc comments indicating its experimental status and backend feature flag requirement.packages/uploadthing/src/next.ts (1)
12-19: Consistent export pattern for the experimental region feature.The export implementation matches the pattern used in server.ts, properly exposing the feature for Next.js App Router applications with clear documentation about its experimental status.
packages/uploadthing/src/next-legacy.ts (1)
12-19: Feature consistently exported for legacy Next.js applications.The experimental region feature is properly exposed for Next.js Pages Router applications, maintaining API consistency across all supported Next.js versions.
packages/uploadthing/src/_internal/config.ts (1)
85-93: LGTM! Well-implemented dynamic region selectionThe implementation correctly handles the new region selection logic, attempting to use the provided
preferredRegionif it exists in the available regions, with a proper fallback to the first region when needed.playground/app/api/uploadthing/route.ts (2)
7-7: LGTM! Correct import of the experimental featureThe import of
experimental_UTRegionensures access to the new region selection functionality.
19-19: Verify intent of ACL change from private to public-readThe ACL for blob files was changed from what was likely "private" to "public-read", which is unrelated to the region selection feature. Make sure this change is intentional.
packages/uploadthing/src/_internal/types.ts (3)
22-33: LGTM! Comprehensive region alias typeThe
UTRegionAliastype properly defines all the supported regions, matching those documented in the regions table.
34-38: LGTM! Well-documented region symbolThe
UTRegionsymbol is properly defined with clear documentation explaining its purpose for dynamic region selection.
60-60: LGTM! Correct type extension for middlewareThe
ValidMiddlewareObjectinterface is correctly extended to include the optional region property.docs/src/app/(docs)/concepts/regions-acl/page.mdx (2)
50-103: LGTM! Well-documented experimental featureThe documentation clearly explains the dynamic region selection feature, including:
- Its private beta status and how to request access
- The purpose and benefits of the feature
- A practical example that demonstrates implementation
- Clear code comments explaining the region selection logic
The example provided correctly demonstrates how to implement the feature in middleware, with proper error handling and fallback logic.
63-63:Details
✅ Verification successful
Verify image path and accessibility
The documentation references an image
./multiregion.pngwhich appears to be a relative path. Make sure this image exists and is accessible when the documentation is published.
🏁 Script executed:
#!/bin/bash # Check if the multiregion.png image exists cd docs/src/app/\(docs\)/concepts/regions-acl/ if [ -f "multiregion.png" ]; then echo "Image exists" else echo "Image not found at expected path" # Look for it elsewhere cd ../../../../../ fd -t f "multiregion.png" fiLength of output: 112
Image path and accessibility verified
The image
multiregion.pngexists indocs/src/app/(docs)/concepts/regions-acl/and includes the alt text “Dynamic region selection,” ensuring it will be displayed and accessible when published. No further action needed.packages/uploadthing/src/_internal/handler.ts (2)
49-49: Import looks correct – keep the unused-import linter in mind
UTRegionis now required downstream, so the addition is correct. If the linter is configured to flag unused imports, double-check that every build target touches code paths that referenceUTRegion; otherwise tree–shaking could strip it and the linter may complain in the future.
571-571: Good integration ofpreferredRegionwithIngestUrl
IngestUrlalready validates the provided region against the server-supplied list and falls back gracefully—nice touch. No further action required here.
📦 Bundle size comparison
|
|
A new canary is available for testing. You can install this latest build in your project with: pnpm add @uploadthing/expo@7.2.3-canary.07fd596
pnpm add @uploadthing/mime-types@0.3.5-canary.07fd596
pnpm add @uploadthing/nuxt@7.1.8-canary.07fd596
pnpm add @uploadthing/react@7.3.1-canary.07fd596
pnpm add @uploadthing/shared@7.1.8-canary.07fd596
pnpm add @uploadthing/solid@7.3.1-canary.07fd596
pnpm add @uploadthing/svelte@7.3.1-canary.07fd596
pnpm add uploadthing@7.6.1-canary.07fd596
pnpm add @uploadthing/vue@7.3.1-canary.07fd596 |
This reverts commit 23f91dd.
1 participant
experimental
Summary by CodeRabbit