From 3c993d59c4cc5384b29a253753fcdbeaaf67d3e7 Mon Sep 17 00:00:00 2001 From: Charles Zhao Date: Wed, 25 Sep 2024 14:36:50 +0800 Subject: [PATCH] fix(core,tunnel,phrases): support range requests when hosting custom ui assets (#6630) --- .changeset/tasty-kings-fetch.md | 9 ++++ .changeset/tricky-mice-pay.md | 5 ++ .../koa-serve-custom-ui-assets.test.ts | 40 +++++++++++++- .../middleware/koa-serve-custom-ui-assets.ts | 46 +++++++++++++--- .../core/src/utils/storage/azure-storage.ts | 31 +++++++++-- .../phrases/src/locales/en/errors/request.ts | 1 + .../toolkit/core-kit/src/utils/url.test.ts | 29 ++++++++++- packages/toolkit/core-kit/src/utils/url.ts | 43 +++++++++++++++ .../tunnel/src/commands/tunnel/utils.test.ts | 21 +++----- packages/tunnel/src/commands/tunnel/utils.ts | 52 ++++++++++++++----- 10 files changed, 238 insertions(+), 39 deletions(-) create mode 100644 .changeset/tasty-kings-fetch.md create mode 100644 .changeset/tricky-mice-pay.md diff --git a/.changeset/tasty-kings-fetch.md b/.changeset/tasty-kings-fetch.md new file mode 100644 index 000000000..68a5196e4 --- /dev/null +++ b/.changeset/tasty-kings-fetch.md @@ -0,0 +1,9 @@ +--- +"@logto/phrases": patch +"@logto/tunnel": patch +"@logto/core": patch +--- + +fix an issue that prevent mp4 video from playing on Safari + +Safari browser uses range request to fetch video data, but it was not supported by the `@logto/tunnel` CLI tool and `koa-serve-custom-ui-assets` middleware in core. This prevents our users who want to build custom sign-in pages with video background. In order to fix this, we need to partially read the video file stream based on the `range` request header, and set proper response headers and status code (206). diff --git a/.changeset/tricky-mice-pay.md b/.changeset/tricky-mice-pay.md new file mode 100644 index 000000000..fe7caf577 --- /dev/null +++ b/.changeset/tricky-mice-pay.md @@ -0,0 +1,5 @@ +--- +"@logto/core-kit": patch +--- + +add range request handling to url utilities diff --git a/packages/core/src/middleware/koa-serve-custom-ui-assets.test.ts b/packages/core/src/middleware/koa-serve-custom-ui-assets.test.ts index 41621a06c..c01fa90f5 100644 --- a/packages/core/src/middleware/koa-serve-custom-ui-assets.test.ts +++ b/packages/core/src/middleware/koa-serve-custom-ui-assets.test.ts @@ -25,12 +25,14 @@ SystemContext.shared.experienceBlobsProviderConfig = experienceBlobsProviderConf const mockedIsFileExisted = jest.fn(async (filename: string) => true); const mockedDownloadFile = jest.fn(); +const mockedGetFileProperties = jest.fn(async () => ({ contentLength: 100 })); await mockEsmWithActual('#src/utils/storage/azure-storage.js', () => ({ buildAzureStorage: jest.fn(() => ({ uploadFile: jest.fn(async () => 'https://fake.url'), downloadFile: mockedDownloadFile, isFileExisted: mockedIsFileExisted, + getFileProperties: mockedGetFileProperties, })), })); @@ -81,11 +83,47 @@ describe('koaServeCustomUiAssets middleware', () => { }); it('should return 404 if the file does not exist', async () => { - mockedIsFileExisted.mockResolvedValue(false); + mockedIsFileExisted.mockResolvedValueOnce(false); const ctx = createMockContext({ url: '/fake.txt' }); await expect(koaServeCustomUiAssets('custom-ui-asset-id')(ctx, next)).rejects.toMatchError( new RequestError({ code: 'entity.not_found', status: 404 }) ); }); + + it('should be able to handle range request', async () => { + const mockBodyStream = Readable.from('video data'); + mockedDownloadFile.mockImplementationOnce( + async (objectKey: string, offset?: number, count?: number) => { + if (objectKey.endsWith('/video.mp4')) { + return { + contentType: 'video/mp4', + readableStreamBody: mockBodyStream, + contentLength: count ?? 0, + }; + } + throw new Error('File not found'); + } + ); + + const ctx = createMockContext({ url: '/video.mp4', headers: { range: 'bytes=0-10' } }); + + await koaServeCustomUiAssets('custom-ui-asset-id')(ctx, next); + + expect(mockedDownloadFile).toHaveBeenCalledWith('default/custom-ui-asset-id/video.mp4', 0, 11); + expect(ctx.type).toEqual('video/mp4'); + expect(ctx.body).toEqual(mockBodyStream); + expect(ctx.status).toEqual(206); + expect(ctx.response.headers['accept-ranges']).toEqual('bytes'); + expect(ctx.response.headers['content-range']).toEqual('bytes 0-10/100'); + expect(ctx.response.headers['content-length']).toEqual('11'); + }); + + it('should throw if range is not satisfiable', async () => { + const ctx = createMockContext({ url: '/video.mp4', headers: { range: 'invalid-range' } }); + + await expect(koaServeCustomUiAssets('custom-ui-asset-id')(ctx, next)).rejects.toMatchError( + new RequestError({ code: 'request.range_not_satisfiable', status: 416 }) + ); + }); }); diff --git a/packages/core/src/middleware/koa-serve-custom-ui-assets.ts b/packages/core/src/middleware/koa-serve-custom-ui-assets.ts index 0232fedf9..63552026b 100644 --- a/packages/core/src/middleware/koa-serve-custom-ui-assets.ts +++ b/packages/core/src/middleware/koa-serve-custom-ui-assets.ts @@ -1,3 +1,5 @@ +import { isFileAssetPath, parseRange } from '@logto/core-kit'; +import { tryThat } from '@silverhand/essentials'; import type { MiddlewareType } from 'koa'; import SystemContext from '#src/tenants/SystemContext.js'; @@ -5,6 +7,11 @@ import assertThat from '#src/utils/assert-that.js'; import { buildAzureStorage } from '#src/utils/storage/azure-storage.js'; import { getTenantId } from '#src/utils/tenant.js'; +import RequestError from '../errors/RequestError/index.js'; + +const noCache = 'no-cache, no-store, must-revalidate'; +const maxAgeSevenDays = 'max-age=604_800_000'; + /** * Middleware that serves custom UI assets user uploaded previously through sign-in experience settings. * If the request path contains a dot, consider it as a file and will try to serve the file directly. @@ -19,19 +26,46 @@ export default function koaServeCustomUiAssets(customUiAssetId: string) { assertThat(tenantId, 'session.not_found', 404); const { container, connectionString } = experienceBlobsProviderConfig; - const { downloadFile, isFileExisted } = buildAzureStorage(connectionString, container); + const { downloadFile, isFileExisted, getFileProperties } = buildAzureStorage( + connectionString, + container + ); const contextPath = `${tenantId}/${customUiAssetId}`; const requestPath = ctx.request.path; - const isFileRequest = requestPath.includes('.'); + const isFileAssetRequest = isFileAssetPath(requestPath); - const fileObjectKey = `${contextPath}${isFileRequest ? requestPath : '/index.html'}`; + const fileObjectKey = `${contextPath}${isFileAssetRequest ? requestPath : '/index.html'}`; const isExisted = await isFileExisted(fileObjectKey); assertThat(isExisted, 'entity.not_found', 404); - const downloadResponse = await downloadFile(fileObjectKey); - ctx.type = downloadResponse.contentType ?? 'application/octet-stream'; - ctx.body = downloadResponse.readableStreamBody; + const range = ctx.get('range'); + const { start, end, count } = tryThat( + () => parseRange(range), + new RequestError({ code: 'request.range_not_satisfiable', status: 416 }) + ); + + const [ + { contentLength = 0, readableStreamBody, contentType }, + { contentLength: totalFileSize = 0 }, + ] = await Promise.all([ + downloadFile(fileObjectKey, start, count), + getFileProperties(fileObjectKey), + ]); + + ctx.body = readableStreamBody; + ctx.type = contentType ?? 'application/octet-stream'; + ctx.status = range ? 206 : 200; + + ctx.set('Cache-Control', isFileAssetRequest ? maxAgeSevenDays : noCache); + ctx.set('Content-Length', contentLength.toString()); + if (range) { + ctx.set('Accept-Ranges', 'bytes'); + ctx.set( + 'Content-Range', + `bytes ${start ?? 0}-${end ?? Math.max(totalFileSize - 1, 0)}/${totalFileSize}` + ); + } return next(); }; diff --git a/packages/core/src/utils/storage/azure-storage.ts b/packages/core/src/utils/storage/azure-storage.ts index ed03f26ef..ea9ca7ed7 100644 --- a/packages/core/src/utils/storage/azure-storage.ts +++ b/packages/core/src/utils/storage/azure-storage.ts @@ -1,4 +1,9 @@ -import { type BlobDownloadResponseParsed, BlobServiceClient } from '@azure/storage-blob'; +import { + type BlobDownloadOptions, + type BlobDownloadResponseParsed, + type BlobGetPropertiesResponse, + BlobServiceClient, +} from '@azure/storage-blob'; import type { UploadFile } from './types.js'; @@ -9,8 +14,14 @@ export const buildAzureStorage = ( container: string ): { uploadFile: UploadFile; - downloadFile: (objectKey: string) => Promise; + downloadFile: ( + objectKey: string, + offset?: number, + count?: number, + options?: BlobDownloadOptions + ) => Promise; isFileExisted: (objectKey: string) => Promise; + getFileProperties: (objectKey: string) => Promise; } => { const blobServiceClient = BlobServiceClient.fromConnectionString(connectionString); const containerClient = blobServiceClient.getContainerClient(container); @@ -31,9 +42,14 @@ export const buildAzureStorage = ( }; }; - const downloadFile = async (objectKey: string) => { + const downloadFile = async ( + objectKey: string, + offset?: number, + count?: number, + options?: BlobDownloadOptions + ) => { const blockBlobClient = containerClient.getBlockBlobClient(objectKey); - return blockBlobClient.download(); + return blockBlobClient.download(offset, count, options); }; const isFileExisted = async (objectKey: string) => { @@ -41,5 +57,10 @@ export const buildAzureStorage = ( return blockBlobClient.exists(); }; - return { uploadFile, downloadFile, isFileExisted }; + const getFileProperties = async (objectKey: string) => { + const blockBlobClient = containerClient.getBlockBlobClient(objectKey); + return blockBlobClient.getProperties(); + }; + + return { uploadFile, downloadFile, isFileExisted, getFileProperties }; }; diff --git a/packages/phrases/src/locales/en/errors/request.ts b/packages/phrases/src/locales/en/errors/request.ts index db7883092..29678f11e 100644 --- a/packages/phrases/src/locales/en/errors/request.ts +++ b/packages/phrases/src/locales/en/errors/request.ts @@ -1,6 +1,7 @@ const request = { invalid_input: 'Input is invalid. {{details}}', general: 'Request error occurred.', + range_not_satisfiable: 'Range not satisfiable.', }; export default Object.freeze(request); diff --git a/packages/toolkit/core-kit/src/utils/url.test.ts b/packages/toolkit/core-kit/src/utils/url.test.ts index 75ab7588c..67ceed20e 100644 --- a/packages/toolkit/core-kit/src/utils/url.test.ts +++ b/packages/toolkit/core-kit/src/utils/url.test.ts @@ -1,6 +1,12 @@ import { describe, expect, it } from 'vitest'; -import { isLocalhost, isValidUrl, validateRedirectUrl } from './url.js'; +import { + isFileAssetPath, + isLocalhost, + isValidUrl, + parseRange, + validateRedirectUrl, +} from './url.js'; describe('url utilities', () => { it('should allow valid redirect URIs', () => { @@ -42,6 +48,27 @@ describe('url utilities', () => { expect(isValidUrl('abc.com/callback?test=123')).toBeFalsy(); expect(isValidUrl('abc.com/callback#test=123')).toBeFalsy(); }); + + it('should be able to parse value from request URL with range header', () => { + expect(parseRange('bytes=0-499')).toEqual({ start: 0, end: 499, count: 500 }); + expect(parseRange('bytes=0-')).toEqual({ start: 0, end: undefined, count: undefined }); + expect(() => parseRange('invalid')).toThrowError('Range not satisfiable.'); + }); + + it('should be able to check if a request path is file asset', () => { + expect(isFileAssetPath('/file.js')).toBe(true); + expect(isFileAssetPath('/file.css')).toBe(true); + expect(isFileAssetPath('/file.png')).toBe(true); + expect(isFileAssetPath('/oidc/.well-known/openid-configuration')).toBe(false); + expect(isFileAssetPath('/oidc/auth')).toBe(false); + expect(isFileAssetPath('/api/interaction/submit')).toBe(false); + expect(isFileAssetPath('/consent')).toBe(false); + expect( + isFileAssetPath( + '/callback/45doq0d004awrjyvdbp92?state=PxsR_Iqtkxw&code=4/0AcvDMrCOMTFXWlKzTcUO24xDify5tQbIMYvaYDS0sj82NzzYlrG4BWXJB4-OxjBI1RPL8g&scope=email%20profile%20openid%20https:/www.googleapis.com/auth/userinfo.profile%20https:/www.googleapis.com/auth/userinfo.email&authuser=0&hd=silverhand.io&prompt=consent' + ) + ).toBe(false); + }); }); describe('isLocalhost()', () => { diff --git a/packages/toolkit/core-kit/src/utils/url.ts b/packages/toolkit/core-kit/src/utils/url.ts index 420899a3e..4b7b602d0 100644 --- a/packages/toolkit/core-kit/src/utils/url.ts +++ b/packages/toolkit/core-kit/src/utils/url.ts @@ -36,3 +36,46 @@ export const isLocalhost = (url: string) => { return ['localhost', '127.0.0.1', '::1'].includes(parsedUrl.hostname); }; + +/** + * Check if the request URL is a file asset path. + * The check is based on the last segment of the URL path containing a dot, ignoring query params. + * Example: + * - `path/scripts.js` -> true + * - `path/index.html?query=param` -> true + * - `path` -> false + * - `path?email=abc@test.com` -> false + * @param url Request URL + * @returns Boolean value indicating if the request URL is a file asset path + */ +export const isFileAssetPath = (url: string): boolean => { + const pathWithoutQuery = url.split('?')[0]; + return Boolean(pathWithoutQuery?.split('/').at(-1)?.includes('.')); +}; + +/** + * Parse the "range" request header value to get the start, end, and count values. + * Example: + * - `range: bytes=0-499` -> { start: 0, end: 499, count: 500 } + * - `range: bytes=0-` -> { start: 0, end: undefined, count: undefined } + * - `range: invalid` -> Error: Range not satisfiable + * - Without range header -> { start: undefined, end: undefined, count: undefined } + * @param range Range request header value + * @returns Object containing start, end, and count values + */ +export const parseRange = (range: string) => { + const rangeMatch = /bytes=(\d+)-(\d+)?/.exec(range); + if (range && !rangeMatch) { + throw new Error('Range not satisfiable.'); + } + + const start = rangeMatch?.[1] === undefined ? undefined : Number.parseInt(rangeMatch[1], 10); + const end = rangeMatch?.[2] === undefined ? undefined : Number.parseInt(rangeMatch[2], 10); + const count = end === undefined ? undefined : end - (start ?? 0) + 1; + + return { + start, + end, + count, + }; +}; diff --git a/packages/tunnel/src/commands/tunnel/utils.test.ts b/packages/tunnel/src/commands/tunnel/utils.test.ts index 271f0ac4e..5ff5acf52 100644 --- a/packages/tunnel/src/commands/tunnel/utils.test.ts +++ b/packages/tunnel/src/commands/tunnel/utils.test.ts @@ -1,20 +1,13 @@ import { expect, describe, it } from 'vitest'; -import { isFileAssetPath } from './utils.js'; +import { getMimeType } from './utils.js'; describe('Tunnel utils', () => { - it('should be able to check if a request path is file asset', () => { - expect(isFileAssetPath('/file.js')).toBe(true); - expect(isFileAssetPath('/file.css')).toBe(true); - expect(isFileAssetPath('/file.png')).toBe(true); - expect(isFileAssetPath('/oidc/.well-known/openid-configuration')).toBe(false); - expect(isFileAssetPath('/oidc/auth')).toBe(false); - expect(isFileAssetPath('/api/interaction/submit')).toBe(false); - expect(isFileAssetPath('/consent')).toBe(false); - expect( - isFileAssetPath( - '/callback/45doq0d004awrjyvdbp92?state=PxsR_Iqtkxw&code=4/0AcvDMrCOMTFXWlKzTcUO24xDify5tQbIMYvaYDS0sj82NzzYlrG4BWXJB4-OxjBI1RPL8g&scope=email%20profile%20openid%20https:/www.googleapis.com/auth/userinfo.profile%20https:/www.googleapis.com/auth/userinfo.email&authuser=0&hd=silverhand.io&prompt=consent' - ) - ).toBe(false); + it('should be able to get mime type according to request path', () => { + expect(getMimeType('/scripts.js')).toEqual('text/javascript'); + expect(getMimeType('/image.png')).toEqual('image/png'); + expect(getMimeType('/style.css')).toEqual('text/css'); + expect(getMimeType('/index.html')).toEqual('text/html'); + expect(getMimeType('/')).toEqual('text/html; charset=utf-8'); }); }); diff --git a/packages/tunnel/src/commands/tunnel/utils.ts b/packages/tunnel/src/commands/tunnel/utils.ts index 035e201cc..2b8824eee 100644 --- a/packages/tunnel/src/commands/tunnel/utils.ts +++ b/packages/tunnel/src/commands/tunnel/utils.ts @@ -3,7 +3,7 @@ import fs from 'node:fs/promises'; import type http from 'node:http'; import path from 'node:path'; -import { isValidUrl } from '@logto/core-kit'; +import { isFileAssetPath, isValidUrl, parseRange } from '@logto/core-kit'; import { conditional, trySafe } from '@silverhand/essentials'; import chalk from 'chalk'; import { createProxyMiddleware, responseInterceptor } from 'http-proxy-middleware'; @@ -48,17 +48,51 @@ export const createStaticFileProxy = if (request.method === 'HEAD' || request.method === 'GET') { const fallBackToIndex = !isFileAssetPath(request.url); const requestPath = path.join(staticPath, fallBackToIndex ? index : request.url); + const { range = '' } = request.headers; + + const readFile = async (requestPath: string, start?: number, end?: number) => { + const fileHandle = await fs.open(requestPath, 'r'); + const { size } = await fileHandle.stat(); + const readStart = start ?? 0; + const readEnd = end ?? Math.max(size - 1, 0); + const buffer = Buffer.alloc(readEnd - readStart + 1); + await fileHandle.read(buffer, 0, buffer.length, readStart); + await fileHandle.close(); + return { buffer, totalFileSize: size }; + }; + + const setRangeHeaders = ( + response: http.ServerResponse, + range: string, + totalFileSize: number + ) => { + if (range) { + const { start, end } = parseRange(range); + const readStart = start ?? 0; + const readEnd = end ?? totalFileSize - 1; + response.setHeader('Accept-Ranges', 'bytes'); + response.setHeader('Content-Range', `bytes ${readStart}-${readEnd}/${totalFileSize}`); + } + }; + try { - const content = await fs.readFile(requestPath, 'utf8'); + const { start, end } = parseRange(range); + const { buffer, totalFileSize } = await readFile(requestPath, start, end); response.setHeader('cache-control', fallBackToIndex ? noCache : maxAgeSevenDays); response.setHeader('content-type', getMimeType(request.url)); - response.writeHead(200); - response.end(content); + setRangeHeaders(response, range, totalFileSize); + + response.setHeader('content-length', String(buffer.length)); + response.writeHead(range ? 206 : 200); + response.end(buffer); } catch (error) { const errorMessage = error instanceof Error ? error.message : String(error); consoleLog.error(chalk.red(errorMessage)); + response.setHeader('content-type', getMimeType(request.url)); - response.writeHead(existsSync(request.url) ? 500 : 404); + const statusCode = + errorMessage === 'Range not satisfiable.' ? 416 : existsSync(request.url) ? 500 : 404; + response.writeHead(statusCode); response.end(); } } @@ -152,13 +186,7 @@ Specify --help for available options`); export const isLogtoRequestPath = (requestPath?: string): boolean => ['/oidc/', '/api/'].some((path) => requestPath?.startsWith(path)) || requestPath === '/consent'; -export const isFileAssetPath = (url: string): boolean => { - // Check if the request URL contains query params. If yes, ignore the params and check the request path - const pathWithoutQuery = url.split('?')[0]; - return Boolean(pathWithoutQuery?.split('/').at(-1)?.includes('.')); -}; - -const getMimeType = (requestPath: string) => { +export const getMimeType = (requestPath: string) => { const fallBackToIndex = !isFileAssetPath(requestPath); if (fallBackToIndex) { return indexContentType;