Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 3.8k
improvement(buffer): explicit mediaType override instead of guessing on ambiguous media#5642
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
a1ac9a9c5ecd7aa80fbd24e20bc38770cd5File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -56,6 +56,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => { | ||
| dueAt: body.dueAt, | ||
| saveToDraft: body.saveToDraft, | ||
| media: body.media, | ||
| mediaType: body.mediaType, | ||
| mediaAltText: body.mediaAltText, | ||
waleedlatif1 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| userId: authResult.userId, | ||
| requestId, | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -56,6 +56,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => { | ||
| dueAt: body.dueAt, | ||
| saveToDraft: body.saveToDraft, | ||
| media: body.media, | ||
| mediaType: body.mediaType, | ||
| mediaAltText: body.mediaAltText, | ||
waleedlatif1 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. waleedlatif1 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| userId: authResult.userId, | ||
| requestId, | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -18,6 +18,17 @@ import { | ||
| const VIDEO_EXTENSIONS = ['.mp4', '.mov', '.m4v', '.webm', '.avi'] | ||
| const IMAGE_EXTENSIONS = ['.png', '.jpg', '.jpeg', '.gif', '.webp'] | ||
| const MEDIA_PROBE_TIMEOUT_MS = 5000 | ||
| /** | ||
| * Buffer fetches asset URLs at publish time, which for queued or scheduled | ||
| * posts can be days after createPost. Presign stored files for the S3 maximum | ||
| * of 7 days so scheduled posts within that window can still be published. | ||
| * The effective lifetime is additionally bounded by the signing credentials: | ||
| * Sim's storage clients sign with static keys (AWS_ACCESS_KEY_ID / | ||
| * AWS_SECRET_ACCESS_KEY), which support the full 7 days; deployments signing | ||
| * with temporary session credentials cap every presigned URL at the session | ||
| * lifetime platform-wide. | ||
| */ | ||
| const MEDIA_PRESIGN_EXPIRY_SECONDS = 7 * 24 * 60 * 60 | ||
waleedlatif1 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| const CREATE_POST_MUTATION = ` | ||
| mutation CreatePost($input: CreatePostInput!) { | ||
| @@ -53,6 +64,7 @@ const EDIT_POST_MUTATION = ` | ||
| interface ResolveMediaAssetOptions { | ||
| media: RawFileInput | string | ||
| mediaType?: 'auto' | 'image' | 'video' | null | ||
| mediaAltText?: string | null | ||
| userId: string | ||
| requestId: string | ||
| @@ -79,15 +91,16 @@ function mediaKindFromExtension(pathOrName: string): 'image' | 'video' | null { | ||
| * Determines whether media should be attached as a video or image asset. | ||
| * Prefers the file's MIME type, then the path/URL extension, and for | ||
| * extensionless URLs falls back to a DNS-pinned HEAD probe of the resolved | ||
| * URL's Content-Type. Defaults to image when nothing is conclusive. | ||
| * URL's Content-Type. Returns null when nothing is conclusive so the caller | ||
| * can ask for an explicit media type instead of guessing. | ||
| */ | ||
| async function resolveMediaKind( | ||
| mimeType: string | undefined, | ||
| pathOrName: string, | ||
| fileUrl: string, | ||
| requestId: string, | ||
| logger: Logger | ||
| ): Promise<'image' | 'video'> { | ||
| ): Promise<'image' | 'video' | null> { | ||
| if (mimeType?.startsWith('video/')) return 'video' | ||
| if (mimeType?.startsWith('image/')) return 'image' | ||
| @@ -106,11 +119,11 @@ async function resolveMediaKind( | ||
| if (contentType.startsWith('image/')) return 'image' | ||
| } | ||
| } catch (error) { | ||
| logger.warn(`[${requestId}] Media content-type probe failed, defaulting to image`, { | ||
| logger.warn(`[${requestId}] Media content-type probe was inconclusive`, { | ||
| error: getErrorMessage(error, 'probe failed'), | ||
| }) | ||
| } | ||
| return 'image' | ||
| return null | ||
| } | ||
| /** | ||
| @@ -122,7 +135,7 @@ async function resolveMediaKind( | ||
| export async function resolveMediaAsset( | ||
| options: ResolveMediaAssetOptions | ||
| ): Promise<ResolvedMediaAsset> { | ||
| const { media, mediaAltText, userId, requestId, logger } = options | ||
| const { media, mediaType, mediaAltText, userId, requestId, logger } = options | ||
| const isFileInput = typeof media === 'object' | ||
| const resolution = await resolveFileInputToUrl({ | ||
| @@ -131,6 +144,7 @@ export async function resolveMediaAsset( | ||
| userId, | ||
| requestId, | ||
| logger, | ||
| presignExpirySeconds: MEDIA_PRESIGN_EXPIRY_SECONDS, | ||
| }) | ||
| if (resolution.error || !resolution.fileUrl) { | ||
| return { | ||
| @@ -143,7 +157,22 @@ export async function resolveMediaAsset( | ||
| const mimeType = isFileInput ? media.type : undefined | ||
| const pathOrName = isFileInput ? media.name || '' : media | ||
| const kind = await resolveMediaKind(mimeType, pathOrName, resolution.fileUrl, requestId, logger) | ||
| const kind = | ||
| mediaType === 'image' || mediaType === 'video' | ||
| ? mediaType | ||
| : await resolveMediaKind(mimeType, pathOrName, resolution.fileUrl, requestId, logger) | ||
| if (!kind) { | ||
| return { | ||
| errorResponse: NextResponse.json( | ||
| { | ||
| success: false, | ||
| error: | ||
| 'Could not determine whether the media is an image or a video. Set mediaType to "image" or "video".', | ||
| }, | ||
| { status: 400 } | ||
| ), | ||
| } | ||
| } | ||
| if (kind === 'video') { | ||
| return { asset: { video: { url: resolution.fileUrl } } } | ||
| } | ||
| @@ -210,6 +239,7 @@ interface ForwardPostMutationOptions { | ||
| dueAt?: string | null | ||
| saveToDraft?: boolean | null | ||
| media?: RawFileInput | string | null | ||
| mediaType?: 'auto' | 'image' | 'video' | null | ||
| mediaAltText?: string | null | ||
| userId: string | ||
| requestId: string | ||
| @@ -224,7 +254,8 @@ interface ForwardPostMutationOptions { | ||
| export async function forwardPostMutation( | ||
| options: ForwardPostMutationOptions | ||
| ): Promise<NextResponse> { | ||
| const { apiKey, postId, channelId, media, mediaAltText, userId, requestId, logger } = options | ||
| const { apiKey, postId, channelId, media, mediaType, mediaAltText, userId, requestId, logger } = | ||
| options | ||
| const input: Record<string, unknown> = { | ||
| mode: options.mode, | ||
| @@ -243,6 +274,7 @@ export async function forwardPostMutation( | ||
| if (media) { | ||
| const { asset, errorResponse } = await resolveMediaAsset({ | ||
| media, | ||
| mediaType, | ||
| mediaAltText, | ||
| userId, | ||
| requestId, | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -49,6 +49,12 @@ export interface ResolveFileInputOptions { | ||
| userId: string | ||
| requestId: string | ||
| logger: Logger | ||
| /** | ||
| * Expiry for presigned URLs minted for stored files, in seconds. | ||
| * Defaults to 5 minutes; raise it only when the external service fetches | ||
| * the URL later than the current request (e.g. scheduled publishing). | ||
| */ | ||
| presignExpirySeconds?: number | ||
| } | ||
| /** | ||
| @@ -62,7 +68,7 @@ export interface ResolveFileInputOptions { | ||
| export async function resolveFileInputToUrl( | ||
| options: ResolveFileInputOptions | ||
| ): Promise<FileResolutionResult> { | ||
| const { file, filePath, userId, requestId, logger } = options | ||
| const { file, filePath, userId, requestId, logger, presignExpirySeconds = 5 * 60 } = options | ||
| if (file) { | ||
| let userFile: UserFile | ||
| @@ -77,19 +83,11 @@ export async function resolveFileInputToUrl( | ||
| } | ||
| } | ||
| let fileUrl = userFile.url || '' | ||
| // Handle internal URLs | ||
| if (fileUrl && isInternalFileUrl(fileUrl)) { | ||
| const resolution = await resolveInternalFileUrl(fileUrl, userId, requestId, logger) | ||
| if (resolution.error) { | ||
| return { error: resolution.error } | ||
| } | ||
| fileUrl = resolution.fileUrl || '' | ||
| } | ||
| // Generate presigned URL if we have a key but no URL | ||
| if (!fileUrl && userFile.key) { | ||
| // A stored file always gets a freshly minted presigned URL scoped to the | ||
| // requested expiry — an embedded url (internal serve path or a previously | ||
| // minted presigned link) may be stale, shorter-lived than required, or | ||
| // point at a different object than the verified key. | ||
| if (userFile.key) { | ||
| const context = resolveTrustedFileContext(userFile.key, userFile.context) | ||
| const hasAccess = await verifyFileAccess(userFile.key, userId, undefined, context, false) | ||
| @@ -102,7 +100,30 @@ export async function resolveFileInputToUrl( | ||
| return { error: { status: 404, message: 'File not found' } } | ||
| } | ||
| fileUrl = await StorageService.generatePresignedDownloadUrl(userFile.key, context, 5 * 60) | ||
| const fileUrl = await StorageService.generatePresignedDownloadUrl( | ||
| userFile.key, | ||
| context, | ||
| presignExpirySeconds | ||
| ) | ||
waleedlatif1 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| return { fileUrl } | ||
| } | ||
| let fileUrl = userFile.url || '' | ||
| // Without a key, the schema guarantees the url references an uploaded | ||
| // file, so resolve the internal serve path to a presigned URL. | ||
| if (fileUrl && isInternalFileUrl(fileUrl)) { | ||
| const resolution = await resolveInternalFileUrl( | ||
| fileUrl, | ||
| userId, | ||
| requestId, | ||
| logger, | ||
| presignExpirySeconds | ||
| ) | ||
| if (resolution.error) { | ||
| return { error: resolution.error } | ||
| } | ||
| fileUrl = resolution.fileUrl || '' | ||
| } | ||
| return { fileUrl } | ||
| @@ -112,7 +133,13 @@ export async function resolveFileInputToUrl( | ||
| let fileUrl = filePath | ||
| if (isInternalFileUrl(filePath)) { | ||
| const resolution = await resolveInternalFileUrl(filePath, userId, requestId, logger) | ||
| const resolution = await resolveInternalFileUrl( | ||
| filePath, | ||
| userId, | ||
| requestId, | ||
| logger, | ||
| presignExpirySeconds | ||
| ) | ||
| if (resolution.error) { | ||
| return { error: resolution.error } | ||
| } | ||
| @@ -227,7 +254,8 @@ export async function resolveInternalFileUrl( | ||
| filePath: string, | ||
| userId: string, | ||
| requestId: string, | ||
| logger: Logger | ||
| logger: Logger, | ||
| presignExpirySeconds = 5 * 60 | ||
| ): Promise<{ fileUrl?: string; error?: { status: number; message: string } }> { | ||
| if (!isInternalFileUrl(filePath)) { | ||
| return { fileUrl: filePath } | ||
| @@ -247,7 +275,11 @@ export async function resolveInternalFileUrl( | ||
| return { error: { status: 404, message: 'File not found' } } | ||
| } | ||
| const fileUrl = await StorageService.generatePresignedDownloadUrl(storageKey, context, 5 * 60) | ||
| const fileUrl = await StorageService.generatePresignedDownloadUrl( | ||
| storageKey, | ||
| context, | ||
| presignExpirySeconds | ||
| ) | ||
| logger.info(`[${requestId}] Generated presigned URL for ${context} file`) | ||
| return { fileUrl } | ||
| } catch (error) { | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.