From d41546b899e75c0decbf2fe2f0841b33c9b39bc9 Mon Sep 17 00:00:00 2001 From: Elias Meire Date: Wed, 20 Sep 2023 14:40:06 +0200 Subject: [PATCH] fix(core): Make parsing of content-type and content-disposition headers more flexible (#7217) fixes #7149 --- packages/core/package.json | 4 - packages/core/src/NodeExecuteFunctions.ts | 299 +++++++++++------- .../core/test/NodeExecuteFunctions.test.ts | 91 +++++- pnpm-lock.yaml | 20 -- 4 files changed, 262 insertions(+), 152 deletions(-) diff --git a/packages/core/package.json b/packages/core/package.json index 8c09e7c6e0..9edb4c7db8 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -35,8 +35,6 @@ ], "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", @@ -52,8 +50,6 @@ "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 6ebd77d101..f033400fcd 100644 --- a/packages/core/src/NodeExecuteFunctions.ts +++ b/packages/core/src/NodeExecuteFunctions.ts @@ -11,100 +11,12 @@ /* eslint-disable @typescript-eslint/no-shadow */ -import type { - GenericValue, - IAdditionalCredentialOptions, - IAllExecuteFunctions, - IBinaryData, - IContextObject, - ICredentialDataDecryptedObject, - ICredentialsExpressionResolveValues, - IDataObject, - IExecuteResponsePromiseData, - IExecuteWorkflowInfo, - IHttpRequestOptions, - IN8nHttpFullResponse, - IN8nHttpResponse, - INode, - INodeCredentialDescription, - INodeCredentialsDetails, - INodeExecutionData, - IOAuth2Options, - IRunExecutionData, - ISourceData, - ITaskDataConnections, - IWebhookData, - IWebhookDescription, - IWorkflowDataProxyAdditionalKeys, - IWorkflowDataProxyData, - IWorkflowExecuteAdditionalData, - Workflow, - WorkflowActivateMode, - WorkflowExecuteMode, - IExecuteData, - IGetNodeParameterOptions, - NodeParameterValueType, - NodeExecutionWithMetadata, - IPairedItemData, - ICredentialTestFunctions, - BinaryHelperFunctions, - NodeHelperFunctions, - RequestHelperFunctions, - FunctionsBase, - IExecuteFunctions, - IExecuteSingleFunctions, - IHookFunctions, - ILoadOptionsFunctions, - IPollFunctions, - ITriggerFunctions, - IWebhookFunctions, - BinaryMetadata, - FileSystemHelperFunctions, - INodeType, - INodePropertyCollection, - INodePropertyOptions, - FieldType, - INodeProperties, -} from 'n8n-workflow'; -import { - createDeferredPromise, - isObjectEmpty, - isResourceMapperValue, - NodeApiError, - NodeHelpers, - NodeOperationError, - WorkflowDataProxy, - LoggerProxy as Logger, - OAuth2GrantType, - deepCopy, - fileTypeFromMimeType, - ExpressionError, - 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, type IncomingHttpHeaders } from 'http'; -import { stringify } from 'qs'; -import type { Token } from 'oauth-1.0a'; -import clientOAuth1 from 'oauth-1.0a'; import type { ClientOAuth2Options, ClientOAuth2RequestObject, ClientOAuth2TokenData, } from '@n8n/client-oauth2'; import { ClientOAuth2 } from '@n8n/client-oauth2'; -import crypto, { createHmac } from 'crypto'; -import get from 'lodash/get'; -import type { Request, Response } from 'express'; -import FormData from 'form-data'; -import path from 'path'; -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 { AxiosError, AxiosPromise, @@ -114,34 +26,120 @@ import type { Method, } from 'axios'; import axios from 'axios'; -import url, { URL, URLSearchParams } from 'url'; -import { Readable } from 'stream'; -import { access as fsAccess, writeFile as fsWriteFile } from 'fs/promises'; +import crypto, { createHmac } from 'crypto'; +import type { Request, Response } from 'express'; +import FileType from 'file-type'; +import FormData from 'form-data'; import { createReadStream } from 'fs'; +import { access as fsAccess, writeFile as fsWriteFile } from 'fs/promises'; +import { IncomingMessage, type IncomingHttpHeaders } from 'http'; +import { Agent } from 'https'; +import get from 'lodash/get'; +import pick from 'lodash/pick'; +import { extension, lookup } from 'mime-types'; +import type { + BinaryHelperFunctions, + BinaryMetadata, + FieldType, + FileSystemHelperFunctions, + FunctionsBase, + GenericValue, + IAdditionalCredentialOptions, + IAllExecuteFunctions, + IBinaryData, + IContextObject, + ICredentialDataDecryptedObject, + ICredentialTestFunctions, + ICredentialsExpressionResolveValues, + IDataObject, + IExecuteData, + IExecuteFunctions, + IExecuteResponsePromiseData, + IExecuteSingleFunctions, + IExecuteWorkflowInfo, + IGetNodeParameterOptions, + IHookFunctions, + IHttpRequestOptions, + ILoadOptionsFunctions, + IN8nHttpFullResponse, + IN8nHttpResponse, + INode, + INodeCredentialDescription, + INodeCredentialsDetails, + INodeExecutionData, + INodeProperties, + INodePropertyCollection, + INodePropertyOptions, + INodeType, + IOAuth2Options, + IPairedItemData, + IPollFunctions, + IRunExecutionData, + ISourceData, + ITaskDataConnections, + ITriggerFunctions, + IWebhookData, + IWebhookDescription, + IWebhookFunctions, + IWorkflowDataProxyAdditionalKeys, + IWorkflowDataProxyData, + IWorkflowExecuteAdditionalData, + NodeExecutionWithMetadata, + NodeHelperFunctions, + NodeParameterValueType, + RequestHelperFunctions, + Workflow, + WorkflowActivateMode, + WorkflowExecuteMode, +} from 'n8n-workflow'; +import { + ExpressionError, + LoggerProxy as Logger, + NodeApiError, + NodeHelpers, + NodeOperationError, + NodeSSLError, + OAuth2GrantType, + WorkflowDataProxy, + createDeferredPromise, + deepCopy, + fileTypeFromMimeType, + isObjectEmpty, + isResourceMapperValue, + validateFieldType, +} from 'n8n-workflow'; +import type { Token } from 'oauth-1.0a'; +import clientOAuth1 from 'oauth-1.0a'; +import path from 'path'; +import { stringify } from 'qs'; +import type { OptionsWithUri, OptionsWithUrl } from 'request'; +import type { RequestPromiseOptions } from 'request-promise-native'; +import { Readable } from 'stream'; +import url, { URL, URLSearchParams } from 'url'; import { BinaryDataManager } from './BinaryDataManager'; -import type { ExtendedValidationResult, IResponseError, IWorkflowSettings } from './Interfaces'; -import { extractValue } from './ExtractValue'; -import { getClientCredentialsToken } from './OAuth2Helper'; +import { binaryToBuffer } from './BinaryDataManager/utils'; import { + BINARY_DATA_STORAGE_PATH, + BLOCK_FILE_ACCESS_TO_N8N_FILES, + CONFIG_FILES, CUSTOM_EXTENSION_ENV, PLACEHOLDER_EMPTY_EXECUTION_ID, - BLOCK_FILE_ACCESS_TO_N8N_FILES, RESTRICT_FILE_ACCESS_TO, - CONFIG_FILES, - BINARY_DATA_STORAGE_PATH, UM_EMAIL_TEMPLATES_INVITE, UM_EMAIL_TEMPLATES_PWRESET, } from './Constants'; -import { binaryToBuffer } from './BinaryDataManager/utils'; +import { extractValue } from './ExtractValue'; +import type { ExtendedValidationResult, IResponseError, IWorkflowSettings } from './Interfaces'; +import { getClientCredentialsToken } from './OAuth2Helper'; +import { getSecretsProxy } from './Secrets'; +import { getUserN8nFolderPath } from './UserSettings'; import { getAllWorkflowExecutionMetadata, getWorkflowExecutionMetadata, setAllWorkflowExecutionMetadata, setWorkflowExecutionMetadata, } from './WorkflowExecutionMetadata'; -import { getSecretsProxy } from './Secrets'; -import { getUserN8nFolderPath } from './UserSettings'; axios.defaults.timeout = 300000; // Prevent axios from adding x-form-www-urlencoded headers by default @@ -604,26 +602,91 @@ 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; +interface IContentType { + type: string; + parameters: { + charset: string; + [key: string]: string; + }; +} + +interface IContentDisposition { + type: string; + filename?: string; +} + +function parseHeaderParameters(parameters: string[]): Record { + return parameters.reduce( + (acc, param) => { + const [key, value] = param.split('='); + acc[key.toLowerCase().trim()] = decodeURIComponent(value); + return acc; + }, + {} as Record, + ); +} + +function parseContentType(contentType?: string): IContentType | null { + if (!contentType) { + return null; } - const contentDispositionHeader = message.headers['content-disposition']; - if (contentDispositionHeader?.length) { - const { - type, - parameters: { filename }, - } = parseContentDisposition(contentDispositionHeader); - message.contentDisposition = { type, filename }; + const [type, ...parameters] = contentType.split(';'); + + return { + type: type.toLowerCase(), + parameters: { charset: 'utf-8', ...parseHeaderParameters(parameters) }, + }; +} + +function parseFileName(filename?: string): string | undefined { + if (filename?.startsWith('"') && filename?.endsWith('"')) { + return filename.slice(1, -1); + } + + return filename; +} + +// https://datatracker.ietf.org/doc/html/rfc5987 +function parseFileNameStar(filename?: string): string | undefined { + const [_encoding, _locale, content] = filename?.split("'") ?? []; + + return content; +} + +function parseContentDisposition(contentDisposition?: string): IContentDisposition | null { + if (!contentDisposition) { + return null; + } + + // This is invalid syntax, but common + // Example 'filename="example.png"' (instead of 'attachment; filename="example.png"') + if (!contentDisposition.startsWith('attachment') && !contentDisposition.startsWith('inline')) { + contentDisposition = `attachment; ${contentDisposition}`; + } + + const [type, ...parameters] = contentDisposition.split(';'); + + const parsedParameters = parseHeaderParameters(parameters); + + return { + type, + filename: + parseFileNameStar(parsedParameters['filename*']) ?? parseFileName(parsedParameters.filename), + }; +} + +export function parseIncomingMessage(message: IncomingMessage) { + const contentType = parseContentType(message.headers['content-type']); + if (contentType) { + const { type, parameters } = contentType; + message.contentType = type; + message.encoding = parameters.charset.toLowerCase() as BufferEncoding; + } + + const contentDisposition = parseContentDisposition(message.headers['content-disposition']); + if (contentDisposition) { + message.contentDisposition = contentDisposition; } } diff --git a/packages/core/test/NodeExecuteFunctions.test.ts b/packages/core/test/NodeExecuteFunctions.test.ts index d981c1d246..77261c2b61 100644 --- a/packages/core/test/NodeExecuteFunctions.test.ts +++ b/packages/core/test/NodeExecuteFunctions.test.ts @@ -1,7 +1,12 @@ -import nock from 'nock'; -import { join } from 'path'; -import { tmpdir } from 'os'; -import { readFileSync, mkdtempSync } from 'fs'; +import { BinaryDataManager } from '@/BinaryDataManager'; +import { + getBinaryDataBuffer, + parseIncomingMessage, + proxyRequestToAxios, + setBinaryDataBuffer, +} from '@/NodeExecuteFunctions'; +import { mkdtempSync, readFileSync } from 'fs'; +import type { IncomingMessage } from 'http'; import { mock } from 'jest-mock-extended'; import type { IBinaryData, @@ -11,12 +16,9 @@ import type { Workflow, WorkflowHooks, } from 'n8n-workflow'; -import { BinaryDataManager } from '@/BinaryDataManager'; -import { - setBinaryDataBuffer, - getBinaryDataBuffer, - proxyRequestToAxios, -} from '@/NodeExecuteFunctions'; +import nock from 'nock'; +import { tmpdir } from 'os'; +import { join } from 'path'; import { initLogger } from './helpers/utils'; const temporaryDir = mkdtempSync(join(tmpdir(), 'n8n')); @@ -136,6 +138,75 @@ describe('NodeExecuteFunctions', () => { }); }); + describe('parseIncomingMessage', () => { + it('parses valid content-type header', () => { + const message = mock({ + headers: { 'content-type': 'application/json', 'content-disposition': undefined }, + }); + parseIncomingMessage(message); + + expect(message.contentType).toEqual('application/json'); + }); + + it('parses valid content-type header with parameters', () => { + const message = mock({ + headers: { + 'content-type': 'application/json; charset=utf-8', + 'content-disposition': undefined, + }, + }); + parseIncomingMessage(message); + + expect(message.contentType).toEqual('application/json'); + }); + + it('parses valid content-disposition header with filename*', () => { + const message = mock({ + headers: { + 'content-type': undefined, + 'content-disposition': + 'attachment; filename="screenshot%20(1).png"; filename*=UTF-8\'\'screenshot%20(1).png', + }, + }); + parseIncomingMessage(message); + + expect(message.contentDisposition).toEqual({ + filename: 'screenshot (1).png', + type: 'attachment', + }); + }); + + it('parses valid content-disposition header with filename and trailing ";"', () => { + const message = mock({ + headers: { + 'content-type': undefined, + 'content-disposition': 'inline; filename="screenshot%20(1).png";', + }, + }); + parseIncomingMessage(message); + + expect(message.contentDisposition).toEqual({ + filename: 'screenshot (1).png', + type: 'inline', + }); + }); + + it('parses non standard content-disposition with missing type', () => { + const message = mock({ + headers: { + 'content-type': undefined, + 'content-disposition': 'filename="screenshot%20(1).png";', + }, + }); + parseIncomingMessage(message); + + expect(message.contentDisposition).toEqual({ + filename: 'screenshot (1).png', + type: 'attachment', + }); + }); + }); + describe('proxyRequestToAxios', () => { const baseUrl = 'http://example.de'; const workflow = mock(); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index be5a4146a0..d966251bb2 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -576,12 +576,6 @@ 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 @@ -631,12 +625,6 @@ 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 @@ -6968,14 +6956,6 @@ packages: dependencies: '@types/node': 18.16.16 - /@types/content-disposition@0.5.5: - resolution: {integrity: sha512-v6LCdKfK6BwcqMo+wYW05rLS12S0ZO0Fl4w1h4aaZMD7bqT3gVUns6FvLJKGZHQmYn3SX55JWGpziwJRwVgutA==} - dev: true - - /@types/content-type@1.1.5: - resolution: {integrity: sha512-dgMN+syt1xb7Hk8LU6AODOfPlvz5z1CbXpPuJE5ZrX9STfBOIXF09pEB8N7a97WT9dbngt3ksDCm6GW6yMrxfQ==} - dev: true - /@types/convict@6.1.1: resolution: {integrity: sha512-R+JLaTvhsD06p4jyjUDtbd5xMtZTRE3c0iI+lrFWZogSVEjgTWPYwvJPVf+t92E+yrlbXa4X4Eg9ro6gPdUt4w==} dependencies: