diff --git a/packages/cli/src/ResponseHelper.ts b/packages/cli/src/ResponseHelper.ts index 0d87b77f644f1..3a3510e83d853 100644 --- a/packages/cli/src/ResponseHelper.ts +++ b/packages/cli/src/ResponseHelper.ts @@ -6,7 +6,7 @@ import type { Request, Response } from 'express'; import { parse, stringify } from 'flatted'; import picocolors from 'picocolors'; import { ErrorReporterProxy as ErrorReporter, NodeApiError } from 'n8n-workflow'; - +import { Readable } from 'node:stream'; import type { IExecutionDb, IExecutionFlatted, @@ -101,6 +101,11 @@ export function sendSuccessResponse( res.header(responseHeader); } + if (data instanceof Readable) { + data.pipe(res); + return; + } + if (raw === true) { if (typeof data === 'string') { res.send(data); diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index ddde98bc89313..f5cdd87cf65d7 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -26,13 +26,11 @@ import type { RequestOptions } from 'oauth-1.0a'; import clientOAuth1 from 'oauth-1.0a'; import { - BinaryDataService, Credentials, LoadMappingOptions, LoadNodeParameterOptions, LoadNodeListSearch, UserSettings, - FileNotFoundError, } from 'n8n-core'; import type { @@ -74,7 +72,6 @@ import { import { credentialsController } from '@/credentials/credentials.controller'; import { oauth2CredentialController } from '@/credentials/oauth2Credential.api'; import type { - BinaryDataRequest, CurlHelper, ExecutionRequest, NodeListSearchRequest, @@ -99,6 +96,7 @@ import { WorkflowStatisticsController, } from '@/controllers'; +import { BinaryDataController } from './controllers/binaryData.controller'; import { ExternalSecretsController } from '@/ExternalSecrets/ExternalSecrets.controller.ee'; import { executionsController } from '@/executions/executions.controller'; import { isApiEnabled, loadPublicApiVersions } from '@/PublicApi'; @@ -208,8 +206,6 @@ export class Server extends AbstractServer { push: Push; - binaryDataService: BinaryDataService; - constructor() { super('main'); @@ -374,7 +370,6 @@ export class Server extends AbstractServer { this.endpointPresetCredentials = config.getEnv('credentials.overwrite.endpoint'); this.push = Container.get(Push); - this.binaryDataService = Container.get(BinaryDataService); await super.start(); LoggerProxy.debug(`Server ID: ${this.uniqueInstanceId}`); @@ -581,6 +576,7 @@ export class Server extends AbstractServer { Container.get(ExternalSecretsController), Container.get(OrchestrationController), Container.get(WorkflowHistoryController), + Container.get(BinaryDataController), ]; if (isLdapEnabled()) { @@ -1442,50 +1438,6 @@ export class Server extends AbstractServer { }), ); - // ---------------------------------------- - // Binary data - // ---------------------------------------- - - // View or download binary file - this.app.get( - `/${this.restEndpoint}/data`, - async (req: BinaryDataRequest, res: express.Response): Promise => { - const { id: binaryDataId, action } = req.query; - let { fileName, mimeType } = req.query; - const [mode] = binaryDataId.split(':') as ['filesystem' | 's3', string]; - - try { - const binaryPath = this.binaryDataService.getPath(binaryDataId); - - if (!fileName || !mimeType) { - try { - const metadata = await this.binaryDataService.getMetadata(binaryDataId); - fileName = metadata.fileName; - mimeType = metadata.mimeType; - res.setHeader('Content-Length', metadata.fileSize); - } catch {} - } - - if (mimeType) res.setHeader('Content-Type', mimeType); - - if (action === 'download') { - res.setHeader('Content-Disposition', `attachment; filename="${fileName}"`); - } - - if (mode === 's3') { - const readStream = await this.binaryDataService.getAsStream(binaryDataId); - readStream.pipe(res); - return; - } else { - res.sendFile(binaryPath); - } - } catch (error) { - if (error instanceof FileNotFoundError) res.writeHead(404).end(); - else throw error; - } - }, - ); - // ---------------------------------------- // Settings // ---------------------------------------- diff --git a/packages/cli/src/controllers/binaryData.controller.ts b/packages/cli/src/controllers/binaryData.controller.ts new file mode 100644 index 0000000000000..00f6a85942cec --- /dev/null +++ b/packages/cli/src/controllers/binaryData.controller.ts @@ -0,0 +1,54 @@ +import { Service } from 'typedi'; +import express from 'express'; +import { BinaryDataService, FileNotFoundError, isValidNonDefaultMode } from 'n8n-core'; +import { Get, RestController } from '@/decorators'; +import { BinaryDataRequest } from '@/requests'; + +@RestController('/binary-data') +@Service() +export class BinaryDataController { + constructor(private readonly binaryDataService: BinaryDataService) {} + + @Get('/') + async get(req: BinaryDataRequest, res: express.Response) { + const { id: binaryDataId, action } = req.query; + + if (!binaryDataId) { + return res.status(400).end('Missing binary data ID'); + } + + if (!binaryDataId.includes(':')) { + return res.status(400).end('Missing binary data mode'); + } + + const [mode] = binaryDataId.split(':'); + + if (!isValidNonDefaultMode(mode)) { + return res.status(400).end('Invalid binary data mode'); + } + + let { fileName, mimeType } = req.query; + + try { + if (!fileName || !mimeType) { + try { + const metadata = await this.binaryDataService.getMetadata(binaryDataId); + fileName = metadata.fileName; + mimeType = metadata.mimeType; + res.setHeader('Content-Length', metadata.fileSize); + } catch {} + } + + if (mimeType) res.setHeader('Content-Type', mimeType); + + if (action === 'download') { + res.setHeader('Content-Disposition', `attachment; filename="${fileName}"`); + } + + return await this.binaryDataService.getAsStream(binaryDataId); + } catch (error) { + if (error instanceof FileNotFoundError) return res.writeHead(404).end(); + else throw error; + } + } +} diff --git a/packages/cli/test/integration/binaryData.api.test.ts b/packages/cli/test/integration/binaryData.api.test.ts new file mode 100644 index 0000000000000..fbb47f2f1299a --- /dev/null +++ b/packages/cli/test/integration/binaryData.api.test.ts @@ -0,0 +1,143 @@ +import fsp from 'node:fs/promises'; +import { Readable } from 'node:stream'; +import { BinaryDataService, FileNotFoundError } from 'n8n-core'; +import * as testDb from './shared/testDb'; +import { mockInstance, setupTestServer } from './shared/utils'; +import type { SuperAgentTest } from 'supertest'; + +jest.mock('fs/promises'); + +const throwFileNotFound = () => { + throw new FileNotFoundError('non/existing/path'); +}; + +const binaryDataService = mockInstance(BinaryDataService); +let testServer = setupTestServer({ endpointGroups: ['binaryData'] }); +let authOwnerAgent: SuperAgentTest; + +beforeAll(async () => { + const owner = await testDb.createOwner(); + authOwnerAgent = testServer.authAgentFor(owner); +}); + +afterEach(() => { + jest.restoreAllMocks(); +}); + +describe('GET /binary-data', () => { + const fileId = '599c5f84007-7d14-4b63-8f1e-d726098d0cc0'; + const fsBinaryDataId = `filesystem:${fileId}`; + const s3BinaryDataId = `s3:${fileId}`; + const binaryFilePath = `/Users/john/.n8n/binaryData/${fileId}`; + const mimeType = 'text/plain'; + const fileName = 'test.txt'; + const buffer = Buffer.from('content'); + const mockStream = new Readable(); + mockStream.push(buffer); + mockStream.push(null); + + describe('should reject on missing or invalid binary data ID', () => { + test.each([['view'], ['download']])('on request to %s', async (action) => { + binaryDataService.getPath.mockReturnValue(binaryFilePath); + fsp.readFile = jest.fn().mockResolvedValue(buffer); + + await authOwnerAgent + .get('/binary-data') + .query({ + fileName, + mimeType, + action, + }) + .expect(400); + + await authOwnerAgent + .get('/binary-data') + .query({ + id: 'invalid', + fileName, + mimeType, + action, + }) + .expect(400); + }); + }); + + describe('should return binary data [filesystem]', () => { + test.each([['view'], ['download']])('on request to %s', async (action) => { + binaryDataService.getAsStream.mockResolvedValue(mockStream); + + const res = await authOwnerAgent + .get('/binary-data') + .query({ + id: fsBinaryDataId, + fileName, + mimeType, + action, + }) + .expect(200); + + const contentDisposition = + action === 'download' ? `attachment; filename="${fileName}"` : undefined; + + expect(binaryDataService.getAsStream).toHaveBeenCalledWith(fsBinaryDataId); + expect(res.headers['content-type']).toBe(mimeType); + expect(res.headers['content-disposition']).toBe(contentDisposition); + }); + }); + + describe('should return 404 on file not found [filesystem]', () => { + test.each(['view', 'download'])('on request to %s', async (action) => { + binaryDataService.getAsStream.mockImplementation(throwFileNotFound); + + await authOwnerAgent + .get('/binary-data') + .query({ + id: fsBinaryDataId, + fileName, + mimeType, + action, + }) + .expect(404); + }); + }); + + describe('should return binary data [s3]', () => { + test.each([['view'], ['download']])('on request to %s', async (action) => { + binaryDataService.getAsStream.mockResolvedValue(mockStream); + + const res = await authOwnerAgent + .get('/binary-data') + .query({ + id: s3BinaryDataId, + fileName, + mimeType, + action, + }) + .expect(200); + + expect(binaryDataService.getAsStream).toHaveBeenCalledWith(s3BinaryDataId); + + const contentDisposition = + action === 'download' ? `attachment; filename="${fileName}"` : undefined; + + expect(res.headers['content-type']).toBe(mimeType); + expect(res.headers['content-disposition']).toBe(contentDisposition); + }); + }); + + describe('should return 404 on file not found [s3]', () => { + test.each(['view', 'download'])('on request to %s', async (action) => { + binaryDataService.getAsStream.mockImplementation(throwFileNotFound); + + await authOwnerAgent + .get('/binary-data') + .query({ + id: s3BinaryDataId, + fileName, + mimeType, + action, + }) + .expect(404); + }); + }); +}); diff --git a/packages/cli/test/integration/shared/types.ts b/packages/cli/test/integration/shared/types.ts index 233392bfd7fa8..f100daf4aeeab 100644 --- a/packages/cli/test/integration/shared/types.ts +++ b/packages/cli/test/integration/shared/types.ts @@ -34,7 +34,8 @@ export type EndpointGroup = | 'mfa' | 'metrics' | 'executions' - | 'workflowHistory'; + | 'workflowHistory' + | 'binaryData'; export interface SetupProps { applyAuth?: boolean; diff --git a/packages/cli/test/integration/shared/utils/testServer.ts b/packages/cli/test/integration/shared/utils/testServer.ts index 356861d710fac..6bcbc60bcc2da 100644 --- a/packages/cli/test/integration/shared/utils/testServer.ts +++ b/packages/cli/test/integration/shared/utils/testServer.ts @@ -66,6 +66,7 @@ import { RoleService } from '@/services/role.service'; import { UserService } from '@/services/user.service'; import { executionsController } from '@/executions/executions.controller'; import { WorkflowHistoryController } from '@/workflows/workflowHistory/workflowHistory.controller.ee'; +import { BinaryDataController } from '@/controllers/binaryData.controller'; /** * Plugin to prefix a path segment into a request URL pathname. @@ -316,6 +317,9 @@ export const setupTestServer = ({ case 'workflowHistory': registerController(app, config, Container.get(WorkflowHistoryController)); break; + case 'binaryData': + registerController(app, config, Container.get(BinaryDataController)); + break; } } } diff --git a/packages/core/src/BinaryData/utils.ts b/packages/core/src/BinaryData/utils.ts index 715456a2e5339..96eeb9fbfe5b9 100644 --- a/packages/core/src/BinaryData/utils.ts +++ b/packages/core/src/BinaryData/utils.ts @@ -15,6 +15,10 @@ export function areValidModes(modes: string[]): modes is BinaryData.Mode[] { return modes.every((m) => BINARY_DATA_MODES.includes(m as BinaryData.Mode)); } +export function isValidNonDefaultMode(mode: string): mode is BinaryData.NonDefaultMode { + return BINARY_DATA_MODES.filter((m) => m !== 'default').includes(mode as BinaryData.Mode); +} + export async function ensureDirExists(dir: string) { try { await fs.access(dir); diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 3eaecf6581d84..3b39ec9bad709 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -18,3 +18,4 @@ export { NodeExecuteFunctions, UserSettings }; export * from './errors'; export { ObjectStoreService } from './ObjectStore/ObjectStore.service.ee'; export { BinaryData } from './BinaryData/types'; +export { isValidNonDefaultMode } from './BinaryData/utils'; diff --git a/packages/editor-ui/src/stores/workflows.store.ts b/packages/editor-ui/src/stores/workflows.store.ts index e41696f9c5d7a..2ac885d7f461d 100644 --- a/packages/editor-ui/src/stores/workflows.store.ts +++ b/packages/editor-ui/src/stores/workflows.store.ts @@ -1393,7 +1393,7 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, { const rootStore = useRootStore(); let restUrl = rootStore.getRestUrl; if (restUrl.startsWith('/')) restUrl = window.location.origin + restUrl; - const url = new URL(`${restUrl}/data`); + const url = new URL(`${restUrl}/binary-data`); url.searchParams.append('id', binaryDataId); url.searchParams.append('action', action); if (fileName) url.searchParams.append('fileName', fileName);