From 273d0913fe5f45c0fe074e6a788e475d5a1d50bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Wed, 6 Sep 2023 12:38:37 +0200 Subject: [PATCH] feat(HTTP Request Node): Determine binary file name from content-disposition headers (#7032) Fixes: https://community.n8n.io/t/http-request-node-read-filename-from-content-disposition-header-when-downloading-files/13453 https://community.n8n.io/t/read-filename-from-content-disposition-header-when-downloading-files/22192 --- packages/cli/package.json | 4 - packages/cli/src/middlewares/bodyParser.ts | 24 +- packages/core/package.json | 4 + packages/core/src/NodeExecuteFunctions.ts | 86 ++++--- .../HttpRequest/V1/HttpRequestV1.node.ts | 23 +- .../HttpRequest/V2/HttpRequestV2.node.ts | 23 +- .../HttpRequest/V3/HttpRequestV3.node.ts | 18 +- .../test/binaryData/HttpRequest.test.ts | 45 ++++ .../test/binaryData/binaryData.test.json | 242 ++++++++++++++++++ packages/workflow/src/index.ts | 6 + pnpm-lock.yaml | 24 +- 11 files changed, 388 insertions(+), 111 deletions(-) create mode 100644 packages/nodes-base/nodes/HttpRequest/test/binaryData/HttpRequest.test.ts create mode 100644 packages/nodes-base/nodes/HttpRequest/test/binaryData/binaryData.test.json diff --git a/packages/cli/package.json b/packages/cli/package.json index 7207a938b9..8a82e4f2d1 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -71,8 +71,6 @@ "@types/bcryptjs": "^2.4.2", "@types/compression": "1.0.1", "@types/connect-history-api-fallback": "^1.3.1", - "@types/content-disposition": "^0.5.5", - "@types/content-type": "^1.1.5", "@types/convict": "^6.1.1", "@types/cookie-parser": "^1.4.2", "@types/express": "^4.17.6", @@ -121,8 +119,6 @@ "class-validator": "^0.14.0", "compression": "^1.7.4", "connect-history-api-fallback": "^1.6.0", - "content-disposition": "^0.5.4", - "content-type": "^1.0.4", "convict": "^6.2.4", "cookie-parser": "^1.4.6", "crypto-js": "~4.1.1", diff --git a/packages/cli/src/middlewares/bodyParser.ts b/packages/cli/src/middlewares/bodyParser.ts index 676c371258..538302c44f 100644 --- a/packages/cli/src/middlewares/bodyParser.ts +++ b/packages/cli/src/middlewares/bodyParser.ts @@ -1,9 +1,8 @@ -import { parse as parseContentDisposition } from 'content-disposition'; -import { parse as parseContentType } from 'content-type'; import getRawBody from 'raw-body'; import type { Request, RequestHandler } from 'express'; import { parse as parseQueryString } from 'querystring'; import { Parser as XmlParser } from 'xml2js'; +import { parseIncomingMessage } from 'n8n-core'; import { jsonParse } from 'n8n-workflow'; import config from '@/config'; import { UnprocessableRequestError } from '@/ResponseHelper'; @@ -17,26 +16,7 @@ const xmlParser = new XmlParser({ const payloadSizeMax = config.getEnv('endpoints.payloadSizeMax'); export const rawBodyReader: RequestHandler = async (req, res, next) => { - if ('content-type' in req.headers) { - const { type: contentType, parameters } = (() => { - try { - return parseContentType(req); - } catch { - return { type: undefined, parameters: undefined }; - } - })(); - req.contentType = contentType; - req.encoding = (parameters?.charset ?? 'utf-8').toLowerCase() as BufferEncoding; - - const contentDispositionHeader = req.headers['content-disposition']; - if (contentDispositionHeader?.length) { - const { - type, - parameters: { filename }, - } = parseContentDisposition(contentDispositionHeader); - req.contentDisposition = { type, filename }; - } - } + parseIncomingMessage(req); req.readRawBody = async () => { if (!req.rawBody) { diff --git a/packages/core/package.json b/packages/core/package.json index bf5664ea8a..50b6f41913 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -35,6 +35,8 @@ ], "devDependencies": { "@types/concat-stream": "^2.0.0", + "@types/content-disposition": "^0.5.5", + "@types/content-type": "^1.1.5", "@types/cron": "~1.7.1", "@types/crypto-js": "^4.0.1", "@types/express": "^4.17.6", @@ -50,6 +52,8 @@ "axios": "^0.21.1", "@n8n/client-oauth2": "workspace:*", "concat-stream": "^2.0.0", + "content-disposition": "^0.5.4", + "content-type": "^1.0.4", "cron": "~1.7.2", "crypto-js": "~4.1.1", "fast-glob": "^3.2.5", diff --git a/packages/core/src/NodeExecuteFunctions.ts b/packages/core/src/NodeExecuteFunctions.ts index b6757384bb..8fb3f6daeb 100644 --- a/packages/core/src/NodeExecuteFunctions.ts +++ b/packages/core/src/NodeExecuteFunctions.ts @@ -78,10 +78,11 @@ import { validateFieldType, NodeSSLError, } from 'n8n-workflow'; - +import { parse as parseContentDisposition } from 'content-disposition'; +import { parse as parseContentType } from 'content-type'; import pick from 'lodash/pick'; import { Agent } from 'https'; -import { IncomingMessage } from 'http'; +import { IncomingMessage, type IncomingHttpHeaders } from 'http'; import { stringify } from 'qs'; import type { Token } from 'oauth-1.0a'; import clientOAuth1 from 'oauth-1.0a'; @@ -100,7 +101,6 @@ import type { OptionsWithUri, OptionsWithUrl } from 'request'; import type { RequestPromiseOptions } from 'request-promise-native'; import FileType from 'file-type'; import { lookup, extension } from 'mime-types'; -import type { IncomingHttpHeaders } from 'http'; import type { AxiosError, AxiosPromise, @@ -600,6 +600,29 @@ type ConfigObject = { simple?: boolean; }; +export function parseIncomingMessage(message: IncomingMessage) { + if ('content-type' in message.headers) { + const { type: contentType, parameters } = (() => { + try { + return parseContentType(message); + } catch { + return { type: undefined, parameters: undefined }; + } + })(); + message.contentType = contentType; + message.encoding = (parameters?.charset ?? 'utf-8').toLowerCase() as BufferEncoding; + } + + const contentDispositionHeader = message.headers['content-disposition']; + if (contentDispositionHeader?.length) { + const { + type, + parameters: { filename }, + } = parseContentDisposition(contentDispositionHeader); + message.contentDisposition = { type, filename }; + } +} + export async function proxyRequestToAxios( workflow: Workflow | undefined, additionalData: IWorkflowExecuteAdditionalData | undefined, @@ -654,35 +677,22 @@ export async function proxyRequestToAxios( try { const response = await requestFn(); - if (configObject.resolveWithFullResponse === true) { - let body = response.data; - if (response.data === '') { - if (axiosConfig.responseType === 'arraybuffer') { - body = Buffer.alloc(0); - } else { - body = undefined; - } - } - await additionalData?.hooks?.executeHookFunctions('nodeFetchedData', [workflow?.id, node]); - return { - body, - headers: response.headers, - statusCode: response.status, - statusMessage: response.statusText, - request: response.request, - }; - } else { - let body = response.data; - if (response.data === '') { - if (axiosConfig.responseType === 'arraybuffer') { - body = Buffer.alloc(0); - } else { - body = undefined; - } - } - await additionalData?.hooks?.executeHookFunctions('nodeFetchedData', [workflow?.id, node]); - return body; + let body = response.data; + if (body instanceof IncomingMessage && axiosConfig.responseType === 'stream') { + parseIncomingMessage(body); + } else if (body === '') { + body = axiosConfig.responseType === 'arraybuffer' ? Buffer.alloc(0) : undefined; } + await additionalData?.hooks?.executeHookFunctions('nodeFetchedData', [workflow?.id, node]); + return configObject.resolveWithFullResponse + ? { + body, + headers: response.headers, + statusCode: response.status, + statusMessage: response.statusText, + request: response.request, + } + : body; } catch (error) { const { config, response } = error; @@ -998,6 +1008,20 @@ async function prepareBinaryData( mimeType?: string, ): Promise { let fileExtension: string | undefined; + if (binaryData instanceof IncomingMessage) { + if (!filePath) { + try { + const { responseUrl } = binaryData; + filePath = + binaryData.contentDisposition?.filename ?? + ((responseUrl && new URL(responseUrl).pathname) ?? binaryData.req?.path)?.slice(1); + } catch {} + } + if (!mimeType) { + mimeType = binaryData.contentType; + } + } + if (!mimeType) { // If no mime type is given figure it out diff --git a/packages/nodes-base/nodes/HttpRequest/V1/HttpRequestV1.node.ts b/packages/nodes-base/nodes/HttpRequest/V1/HttpRequestV1.node.ts index 587247d230..f1db2e33ae 100644 --- a/packages/nodes-base/nodes/HttpRequest/V1/HttpRequestV1.node.ts +++ b/packages/nodes-base/nodes/HttpRequest/V1/HttpRequestV1.node.ts @@ -1,3 +1,5 @@ +import type { Readable } from 'stream'; + import type { IExecuteFunctions, IDataObject, @@ -632,7 +634,7 @@ export class HttpRequestV1 implements INodeType { oAuth2Api = await this.getCredentials('oAuth2Api'); } catch {} - let requestOptions: OptionsWithUri; + let requestOptions: OptionsWithUri & { useStream?: boolean }; let setUiParameter: IDataObject; const uiParameters: IDataObject = { @@ -873,6 +875,7 @@ export class HttpRequestV1 implements INodeType { if (responseFormat === 'file') { requestOptions.encoding = null; + requestOptions.useStream = true; if (options.bodyContentType !== 'raw') { requestOptions.body = JSON.stringify(requestOptions.body); @@ -885,6 +888,7 @@ export class HttpRequestV1 implements INodeType { } } else if (options.bodyContentType === 'raw') { requestOptions.json = false; + requestOptions.useStream = true; } else { requestOptions.json = true; } @@ -991,7 +995,6 @@ export class HttpRequestV1 implements INodeType { response = response.value; const options = this.getNodeParameter('options', itemIndex, {}); - const url = this.getNodeParameter('url', itemIndex) as string; const fullResponse = !!options.fullResponse; @@ -1014,8 +1017,7 @@ export class HttpRequestV1 implements INodeType { Object.assign(newItem.binary, items[itemIndex].binary); } - const fileName = url.split('/').pop(); - + let binaryData: Buffer | Readable; if (fullResponse) { const returnItem: IDataObject = {}; for (const property of fullResponseProperties) { @@ -1026,20 +1028,13 @@ export class HttpRequestV1 implements INodeType { } newItem.json = returnItem; - - newItem.binary![dataPropertyName] = await this.helpers.prepareBinaryData( - response!.body as Buffer, - fileName, - ); + binaryData = response!.body; } else { newItem.json = items[itemIndex].json; - - newItem.binary![dataPropertyName] = await this.helpers.prepareBinaryData( - response! as Buffer, - fileName, - ); + binaryData = response; } + newItem.binary![dataPropertyName] = await this.helpers.prepareBinaryData(binaryData); returnItems.push(newItem); } else if (responseFormat === 'string') { const dataPropertyName = this.getNodeParameter('dataPropertyName', 0); diff --git a/packages/nodes-base/nodes/HttpRequest/V2/HttpRequestV2.node.ts b/packages/nodes-base/nodes/HttpRequest/V2/HttpRequestV2.node.ts index 7ae3efb839..c2f5a7553c 100644 --- a/packages/nodes-base/nodes/HttpRequest/V2/HttpRequestV2.node.ts +++ b/packages/nodes-base/nodes/HttpRequest/V2/HttpRequestV2.node.ts @@ -1,3 +1,5 @@ +import type { Readable } from 'stream'; + import type { IDataObject, IExecuteFunctions, @@ -672,7 +674,7 @@ export class HttpRequestV2 implements INodeType { } catch {} } - let requestOptions: OptionsWithUri; + let requestOptions: OptionsWithUri & { useStream?: boolean }; let setUiParameter: IDataObject; const uiParameters: IDataObject = { @@ -913,6 +915,7 @@ export class HttpRequestV2 implements INodeType { if (responseFormat === 'file') { requestOptions.encoding = null; + requestOptions.useStream = true; if (options.bodyContentType !== 'raw') { requestOptions.body = JSON.stringify(requestOptions.body); @@ -925,6 +928,7 @@ export class HttpRequestV2 implements INodeType { } } else if (options.bodyContentType === 'raw') { requestOptions.json = false; + requestOptions.useStream = true; } else { requestOptions.json = true; } @@ -1044,7 +1048,6 @@ export class HttpRequestV2 implements INodeType { response = response.value; const options = this.getNodeParameter('options', itemIndex, {}); - const url = this.getNodeParameter('url', itemIndex) as string; const fullResponse = !!options.fullResponse; @@ -1067,8 +1070,7 @@ export class HttpRequestV2 implements INodeType { Object.assign(newItem.binary, items[itemIndex].binary); } - const fileName = url.split('/').pop(); - + let binaryData: Buffer | Readable; if (fullResponse) { const returnItem: IDataObject = {}; for (const property of fullResponseProperties) { @@ -1079,20 +1081,13 @@ export class HttpRequestV2 implements INodeType { } newItem.json = returnItem; - - newItem.binary![dataPropertyName] = await this.helpers.prepareBinaryData( - response!.body as Buffer, - fileName, - ); + binaryData = response!.body; } else { newItem.json = items[itemIndex].json; - - newItem.binary![dataPropertyName] = await this.helpers.prepareBinaryData( - response! as Buffer, - fileName, - ); + binaryData = response; } + newItem.binary![dataPropertyName] = await this.helpers.prepareBinaryData(binaryData); returnItems.push(newItem); } else if (responseFormat === 'string') { const dataPropertyName = this.getNodeParameter('dataPropertyName', 0); diff --git a/packages/nodes-base/nodes/HttpRequest/V3/HttpRequestV3.node.ts b/packages/nodes-base/nodes/HttpRequest/V3/HttpRequestV3.node.ts index 2711f65c36..c6a3806262 100644 --- a/packages/nodes-base/nodes/HttpRequest/V3/HttpRequestV3.node.ts +++ b/packages/nodes-base/nodes/HttpRequest/V3/HttpRequestV3.node.ts @@ -1452,8 +1452,6 @@ export class HttpRequestV3 implements INodeType { response = response.value; - const url = this.getNodeParameter('url', itemIndex) as string; - let responseFormat = this.getNodeParameter( 'options.response.response.responseFormat', 0, @@ -1525,8 +1523,7 @@ export class HttpRequestV3 implements INodeType { Object.assign(newItem.binary as IBinaryKeyData, items[itemIndex].binary); } - const fileName = url.split('/').pop(); - + let binaryData: Buffer | Readable; if (fullResponse) { const returnItem: IDataObject = {}; for (const property of fullResponseProperties) { @@ -1537,19 +1534,12 @@ export class HttpRequestV3 implements INodeType { } newItem.json = returnItem; - - newItem.binary![outputPropertyName] = await this.helpers.prepareBinaryData( - response!.body as Buffer | Readable, - fileName, - ); + binaryData = response!.body; } else { newItem.json = items[itemIndex].json; - - newItem.binary![outputPropertyName] = await this.helpers.prepareBinaryData( - response! as Buffer | Readable, - fileName, - ); + binaryData = response; } + newItem.binary![outputPropertyName] = await this.helpers.prepareBinaryData(binaryData); returnItems.push(newItem); } else if (responseFormat === 'text') { diff --git a/packages/nodes-base/nodes/HttpRequest/test/binaryData/HttpRequest.test.ts b/packages/nodes-base/nodes/HttpRequest/test/binaryData/HttpRequest.test.ts new file mode 100644 index 0000000000..c539f4bc44 --- /dev/null +++ b/packages/nodes-base/nodes/HttpRequest/test/binaryData/HttpRequest.test.ts @@ -0,0 +1,45 @@ +import nock from 'nock'; +import { + setup, + equalityTest, + workflowToTests, + getWorkflowFilenames, + initBinaryDataManager, +} from '@test/nodes/Helpers'; + +describe('Test Binary Data Download', () => { + const workflows = getWorkflowFilenames(__dirname); + const tests = workflowToTests(workflows); + + const baseUrl = 'https://dummy.domain'; + + beforeAll(async () => { + await initBinaryDataManager(); + + nock.disableNetConnect(); + + nock(baseUrl) + .persist() + .get('/path/to/image.png') + .reply(200, Buffer.from('test'), { 'content-type': 'image/png' }); + + nock(baseUrl) + .persist() + .get('/redirect-to-image') + .reply(302, {}, { location: baseUrl + '/path/to/image.png' }); + + nock(baseUrl).persist().get('/custom-content-disposition').reply(200, Buffer.from('testing'), { + 'content-disposition': 'attachment; filename="testing.jpg"', + }); + }); + + afterAll(() => { + nock.restore(); + }); + + const nodeTypes = setup(tests); + + for (const testData of tests) { + test(testData.description, async () => equalityTest(testData, nodeTypes)); + } +}); diff --git a/packages/nodes-base/nodes/HttpRequest/test/binaryData/binaryData.test.json b/packages/nodes-base/nodes/HttpRequest/test/binaryData/binaryData.test.json new file mode 100644 index 0000000000..5e1cc9496a --- /dev/null +++ b/packages/nodes-base/nodes/HttpRequest/test/binaryData/binaryData.test.json @@ -0,0 +1,242 @@ +{ + "name": "Download as Binary Data", + "nodes": [ + { + "name": "When clicking \"Execute Workflow\"", + "type": "n8n-nodes-base.manualTrigger", + "typeVersion": 1, + "parameters": {}, + "position": [ + 580, + 300 + ] + }, + { + "name": "HTTP Request (v1)", + "type": "n8n-nodes-base.httpRequest", + "typeVersion": 1, + "parameters": { + "url": "https://dummy.domain/path/to/image.png", + "responseFormat": "file" + }, + "position": [ + 1020, + -100 + ] + }, + { + "name": "HTTP Request (v2)", + "type": "n8n-nodes-base.httpRequest", + "typeVersion": 2, + "parameters": { + "url": "https://dummy.domain/path/to/image.png", + "responseFormat": "file", + "options": {} + }, + "position": [ + 1020, + 80 + ] + }, + { + "name": "HTTP Request (v3)", + "type": "n8n-nodes-base.httpRequest", + "typeVersion": 3, + "parameters": { + "url": "https://dummy.domain/path/to/image.png", + "options": { + "response": { + "response": { + "responseFormat": "file" + } + } + } + }, + "position": [ + 1020, + 240 + ] + }, + { + "name": "HTTP Request (v4)", + "type": "n8n-nodes-base.httpRequest", + "typeVersion": 4, + "parameters": { + "url": "https://dummy.domain/path/to/image.png", + "options": { + "response": { + "response": { + "responseFormat": "file" + } + } + } + }, + "position": [ + 1020, + 400 + ] + }, + { + "name": "Follow Redirect", + "type": "n8n-nodes-base.httpRequest", + "typeVersion": 4.1, + "parameters": { + "url": "https://dummy.domain/redirect-to-image", + "options": { + "response": { + "response": { + "responseFormat": "file" + } + } + } + }, + "position": [ + 1020, + 560 + ] + }, + { + "name": "Content Disposition", + "type": "n8n-nodes-base.httpRequest", + "typeVersion": 4.1, + "parameters": { + "url": "https://dummy.domain/custom-content-disposition", + "options": { + "response": { + "response": { + "responseFormat": "file" + } + } + } + }, + "position": [ + 1020, + 720 + ] + } + ], + "pinData": { + "HTTP Request (v1)": [ + { + "binary": { + "data": { + "mimeType": "image/png", + "fileType": "image", + "fileExtension": "png", + "fileName": "image.png", + "fileSize": "4 B" + } + }, + "json": {} + } + ], + "HTTP Request (v2)": [ + { + "binary": { + "data": { + "mimeType": "image/png", + "fileType": "image", + "fileExtension": "png", + "fileName": "image.png", + "fileSize": "4 B" + } + }, + "json": {} + } + ], + "HTTP Request (v3)": [ + { + "binary": { + "data": { + "mimeType": "image/png", + "fileType": "image", + "fileExtension": "png", + "fileName": "image.png", + "fileSize": "4 B" + } + }, + "json": {} + } + ], + "HTTP Request (v4)": [ + { + "binary": { + "data": { + "mimeType": "image/png", + "fileType": "image", + "fileExtension": "png", + "fileName": "image.png", + "fileSize": "4 B" + } + }, + "json": {} + } + ], + "Follow Redirect": [ + { + "binary": { + "data": { + "mimeType": "image/png", + "fileType": "image", + "fileExtension": "png", + "fileName": "image.png", + "fileSize": "4 B" + } + }, + "json": {} + } + ], + "Content Disposition": [ + { + "binary": { + "data": { + "mimeType": "image/jpeg", + "fileType": "image", + "fileExtension": "jpg", + "fileName": "testing.jpg", + "fileSize": "7 B" + } + }, + "json": {} + } + ] + }, + "connections": { + "When clicking \"Execute Workflow\"": { + "main": [ + [ + { + "node": "HTTP Request (v1)", + "type": "main", + "index": 0 + }, + { + "node": "HTTP Request (v2)", + "type": "main", + "index": 0 + }, + { + "node": "HTTP Request (v3)", + "type": "main", + "index": 0 + }, + { + "node": "HTTP Request (v4)", + "type": "main", + "index": 0 + }, + { + "node": "Follow Redirect", + "type": "main", + "index": 0 + }, + { + "node": "Content Disposition", + "type": "main", + "index": 0 + } + ] + ] + } + } +} diff --git a/packages/workflow/src/index.ts b/packages/workflow/src/index.ts index 7fe6de45dd..ff6b1d7316 100644 --- a/packages/workflow/src/index.ts +++ b/packages/workflow/src/index.ts @@ -57,5 +57,11 @@ declare module 'http' { rawBody: Buffer; readRawBody(): Promise; _body: boolean; + + // This gets added by the `follow-redirects` package + responseUrl?: string; + + // This is added to response objects for all outgoing requests + req?: ClientRequest; } } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 226d51116f..afa4c212af 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -254,12 +254,6 @@ importers: connect-history-api-fallback: specifier: ^1.6.0 version: 1.6.0 - content-disposition: - specifier: ^0.5.4 - version: 0.5.4 - content-type: - specifier: ^1.0.4 - version: 1.0.4 convict: specifier: ^6.2.4 version: 6.2.4 @@ -495,12 +489,6 @@ importers: '@types/connect-history-api-fallback': specifier: ^1.3.1 version: 1.3.5 - '@types/content-disposition': - specifier: ^0.5.5 - version: 0.5.5 - '@types/content-type': - specifier: ^1.1.5 - version: 1.1.5 '@types/convict': specifier: ^6.1.1 version: 6.1.1 @@ -591,6 +579,12 @@ importers: concat-stream: specifier: ^2.0.0 version: 2.0.0 + content-disposition: + specifier: ^0.5.4 + version: 0.5.4 + content-type: + specifier: ^1.0.4 + version: 1.0.4 cron: specifier: ~1.7.2 version: 1.7.2 @@ -640,6 +634,12 @@ importers: '@types/concat-stream': specifier: ^2.0.0 version: 2.0.0 + '@types/content-disposition': + specifier: ^0.5.5 + version: 0.5.5 + '@types/content-type': + specifier: ^1.1.5 + version: 1.1.5 '@types/cron': specifier: ~1.7.1 version: 1.7.3