From 0eb0a28f59a71b176c85c4e104f35a4fb81fbb2e 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: Thu, 11 Jul 2024 15:19:15 +0200 Subject: [PATCH 1/2] refactor(core): refactor binary data handling code --- .../core/src/BinaryData/BinaryData.service.ts | 8 ++--- .../src/BinaryData/ObjectStore.manager.ts | 8 ++--- packages/core/src/BinaryData/utils.ts | 3 +- packages/core/src/NodeExecuteFunctions.ts | 29 +++++++++---------- packages/core/test/BinaryData/utils.test.ts | 14 +++++---- .../HttpRequest/V3/HttpRequestV3.node.ts | 8 ++--- packages/workflow/src/Interfaces.ts | 1 + 7 files changed, 30 insertions(+), 41 deletions(-) diff --git a/packages/core/src/BinaryData/BinaryData.service.ts b/packages/core/src/BinaryData/BinaryData.service.ts index 8aadee3454687..20903aa71c40e 100644 --- a/packages/core/src/BinaryData/BinaryData.service.ts +++ b/packages/core/src/BinaryData/BinaryData.service.ts @@ -3,7 +3,7 @@ import prettyBytes from 'pretty-bytes'; import Container, { Service } from 'typedi'; import { BINARY_ENCODING } from 'n8n-workflow'; import { InvalidModeError } from '../errors/invalid-mode.error'; -import { areConfigModes, toBuffer } from './utils'; +import { areConfigModes, binaryToBuffer } from './utils'; import type { Readable } from 'stream'; import type { BinaryData } from './types'; @@ -84,7 +84,7 @@ export class BinaryDataService { const manager = this.managers[this.mode]; if (!manager) { - const buffer = await this.toBuffer(bufferOrStream); + const buffer = await binaryToBuffer(bufferOrStream); binaryData.data = buffer.toString(BINARY_ENCODING); binaryData.fileSize = prettyBytes(buffer.length); @@ -110,10 +110,6 @@ export class BinaryDataService { return binaryData; } - async toBuffer(bufferOrStream: Buffer | Readable) { - return await toBuffer(bufferOrStream); - } - async getAsStream(binaryDataId: string, chunkSize?: number) { const [mode, fileId] = binaryDataId.split(':'); diff --git a/packages/core/src/BinaryData/ObjectStore.manager.ts b/packages/core/src/BinaryData/ObjectStore.manager.ts index f42eba7f29ba6..6f7bb3ef29080 100644 --- a/packages/core/src/BinaryData/ObjectStore.manager.ts +++ b/packages/core/src/BinaryData/ObjectStore.manager.ts @@ -1,7 +1,7 @@ import fs from 'node:fs/promises'; import { Service } from 'typedi'; import { v4 as uuid } from 'uuid'; -import { toBuffer } from './utils'; +import { binaryToBuffer } from './utils'; import { ObjectStoreService } from '../ObjectStore/ObjectStore.service.ee'; import type { Readable } from 'node:stream'; @@ -22,7 +22,7 @@ export class ObjectStoreManager implements BinaryData.Manager { metadata: BinaryData.PreWriteMetadata, ) { const fileId = this.toFileId(workflowId, executionId); - const buffer = await this.toBuffer(bufferOrStream); + const buffer = await binaryToBuffer(bufferOrStream); await this.objectStoreService.put(fileId, buffer, metadata); @@ -100,8 +100,4 @@ export class ObjectStoreManager implements BinaryData.Manager { return `workflows/${workflowId}/executions/${executionId}/binary_data/${uuid()}`; } - - private async toBuffer(bufferOrStream: Buffer | Readable) { - return await toBuffer(bufferOrStream); - } } diff --git a/packages/core/src/BinaryData/utils.ts b/packages/core/src/BinaryData/utils.ts index 3bf99ceaeb7aa..c64cb4315ba3c 100644 --- a/packages/core/src/BinaryData/utils.ts +++ b/packages/core/src/BinaryData/utils.ts @@ -32,7 +32,8 @@ export async function doesNotExist(dir: string) { } } -export async function toBuffer(body: Buffer | Readable) { +/** Converts a buffer or a readable stream to a buffer */ +export async function binaryToBuffer(body: Buffer | Readable) { if (Buffer.isBuffer(body)) return body; return await new Promise((resolve, reject) => { body diff --git a/packages/core/src/NodeExecuteFunctions.ts b/packages/core/src/NodeExecuteFunctions.ts index 0b2b164f156aa..e422a274d2ba0 100644 --- a/packages/core/src/NodeExecuteFunctions.ts +++ b/packages/core/src/NodeExecuteFunctions.ts @@ -158,6 +158,7 @@ import type { BinaryData } from './BinaryData/types'; import merge from 'lodash/merge'; import { InstanceSettings } from './InstanceSettings'; import { SSHClientsManager } from './SSHClientsManager'; +import { binaryToBuffer } from './BinaryData/utils'; axios.defaults.timeout = 300000; // Prevent axios from adding x-form-www-urlencoded headers by default @@ -764,6 +765,11 @@ export function parseIncomingMessage(message: IncomingMessage) { } } +async function binaryToString(body: Buffer | Readable) { + const buffer = await binaryToBuffer(body); + return buffer.toString(); +} + export async function proxyRequestToAxios( workflow: Workflow | undefined, additionalData: IWorkflowExecuteAdditionalData | undefined, @@ -837,9 +843,7 @@ export async function proxyRequestToAxios( let responseData = response.data; if (Buffer.isBuffer(responseData) || responseData instanceof Readable) { - responseData = await Container.get(BinaryDataService) - .toBuffer(responseData) - .then((buffer) => buffer.toString('utf-8')); + responseData = await binaryToString(responseData); } if (configObject.simple === false) { @@ -3091,17 +3095,14 @@ const getRequestHelperFunctions = ( let contentBody: Exclude; if (newResponse.body instanceof Readable && paginationOptions.binaryResult !== true) { - const data = await this.helpers - .binaryToBuffer(newResponse.body as Buffer | Readable) - .then((body) => body.toString()); // Keep the original string version that we can use it to hash if needed - contentBody = data; + contentBody = await binaryToString(newResponse.body as Buffer | Readable); const responseContentType = newResponse.headers['content-type']?.toString() ?? ''; if (responseContentType.includes('application/json')) { - newResponse.body = jsonParse(data, { fallbackValue: {} }); + newResponse.body = jsonParse(contentBody, { fallbackValue: {} }); } else { - newResponse.body = data; + newResponse.body = contentBody; } tempResponseData.__bodyResolved = true; tempResponseData.body = newResponse.body; @@ -3187,9 +3188,7 @@ const getRequestHelperFunctions = ( // now an error manually if the response code is not a success one. let data = tempResponseData.body; if (data instanceof Readable && paginationOptions.binaryResult !== true) { - data = await this.helpers - .binaryToBuffer(tempResponseData.body as Buffer | Readable) - .then((body) => body.toString()); + data = await binaryToString(data as Buffer | Readable); } else if (typeof data === 'object') { data = JSON.stringify(data); } @@ -3400,8 +3399,8 @@ const getBinaryHelperFunctions = ( getBinaryPath, getBinaryStream, getBinaryMetadata, - binaryToBuffer: async (body: Buffer | Readable) => - await Container.get(BinaryDataService).toBuffer(body), + binaryToBuffer, + binaryToString, prepareBinaryData: async (binaryData, filePath, mimeType) => await prepareBinaryData(binaryData, executionId!, workflowId, filePath, mimeType), setBinaryDataBuffer: async (data, binaryData) => @@ -3743,8 +3742,6 @@ export function getExecuteFunctions( ); return dataProxy.getDataProxy(); }, - binaryToBuffer: async (body: Buffer | Readable) => - await Container.get(BinaryDataService).toBuffer(body), async putExecutionToWait(waitTill: Date): Promise { runExecutionData.waitTill = waitTill; if (additionalData.setExecutionStatus) { diff --git a/packages/core/test/BinaryData/utils.test.ts b/packages/core/test/BinaryData/utils.test.ts index bd69795daed40..95a138c00d84f 100644 --- a/packages/core/test/BinaryData/utils.test.ts +++ b/packages/core/test/BinaryData/utils.test.ts @@ -1,17 +1,17 @@ import { Readable } from 'node:stream'; import { createGunzip } from 'node:zlib'; -import { toBuffer } from '@/BinaryData/utils'; +import { binaryToBuffer } from '@/BinaryData/utils'; describe('BinaryData/utils', () => { - describe('toBuffer', () => { + describe('binaryToBuffer', () => { it('should handle buffer objects', async () => { const body = Buffer.from('test'); - expect((await toBuffer(body)).toString()).toEqual('test'); + expect((await binaryToBuffer(body)).toString()).toEqual('test'); }); it('should handle valid uncompressed Readable streams', async () => { const body = Readable.from(Buffer.from('test')); - expect((await toBuffer(body)).toString()).toEqual('test'); + expect((await binaryToBuffer(body)).toString()).toEqual('test'); }); it('should handle valid compressed Readable streams', async () => { @@ -19,13 +19,15 @@ describe('BinaryData/utils', () => { const body = Readable.from( Buffer.from('1f8b08000000000000032b492d2e01000c7e7fd804000000', 'hex'), ).pipe(gunzip); - expect((await toBuffer(body)).toString()).toEqual('test'); + expect((await binaryToBuffer(body)).toString()).toEqual('test'); }); it('should throw on invalid compressed Readable streams', async () => { const gunzip = createGunzip(); const body = Readable.from(Buffer.from('0001f8b080000000000000000', 'hex')).pipe(gunzip); - await expect(toBuffer(body)).rejects.toThrow(new Error('Failed to decompress response')); + await expect(binaryToBuffer(body)).rejects.toThrow( + new Error('Failed to decompress response'), + ); }); }); }); diff --git a/packages/nodes-base/nodes/HttpRequest/V3/HttpRequestV3.node.ts b/packages/nodes-base/nodes/HttpRequest/V3/HttpRequestV3.node.ts index b9b989213b94e..c8f73909cfb1c 100644 --- a/packages/nodes-base/nodes/HttpRequest/V3/HttpRequestV3.node.ts +++ b/packages/nodes-base/nodes/HttpRequest/V3/HttpRequestV3.node.ts @@ -1945,9 +1945,7 @@ export class HttpRequestV3 implements INodeType { false, ) as boolean; - const data = await this.helpers - .binaryToBuffer(response.body as Buffer | Readable) - .then((body) => body.toString()); + const data = await this.helpers.binaryToString(response.body as Buffer | Readable); response.body = jsonParse(data, { ...(neverError ? { fallbackValue: {} } @@ -1959,9 +1957,7 @@ export class HttpRequestV3 implements INodeType { } else { responseFormat = 'text'; if (!response.__bodyResolved) { - const data = await this.helpers - .binaryToBuffer(response.body as Buffer | Readable) - .then((body) => body.toString()); + const data = await this.helpers.binaryToString(response.body as Buffer | Readable); response.body = !data ? undefined : data; } } diff --git a/packages/workflow/src/Interfaces.ts b/packages/workflow/src/Interfaces.ts index 184b00144ff81..5fbde49d7bad7 100644 --- a/packages/workflow/src/Interfaces.ts +++ b/packages/workflow/src/Interfaces.ts @@ -750,6 +750,7 @@ export interface BinaryHelperFunctions { setBinaryDataBuffer(data: IBinaryData, binaryData: Buffer): Promise; copyBinaryFile(): Promise; binaryToBuffer(body: Buffer | Readable): Promise; + binaryToString(body: Buffer | Readable): Promise; getBinaryPath(binaryDataId: string): string; getBinaryStream(binaryDataId: string, chunkSize?: number): Promise; getBinaryMetadata(binaryDataId: string): Promise<{ From 662f914ca6248b1d6d94b9264446123c08ce5beb 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: Thu, 11 Jul 2024 15:54:53 +0200 Subject: [PATCH 2/2] fix(HTTP Request Node): Respect the original encoding of the incoming response --- packages/core/src/NodeExecuteFunctions.ts | 8 +- .../test/encoding/HttpRequest.test.ts | 40 ++++++++++ .../test/encoding/encoding.test.json | 78 +++++++++++++++++++ packages/workflow/src/Interfaces.ts | 2 +- 4 files changed, 125 insertions(+), 3 deletions(-) create mode 100644 packages/nodes-base/nodes/HttpRequest/test/encoding/HttpRequest.test.ts create mode 100644 packages/nodes-base/nodes/HttpRequest/test/encoding/encoding.test.json diff --git a/packages/core/src/NodeExecuteFunctions.ts b/packages/core/src/NodeExecuteFunctions.ts index e422a274d2ba0..d5e0653bb6998 100644 --- a/packages/core/src/NodeExecuteFunctions.ts +++ b/packages/core/src/NodeExecuteFunctions.ts @@ -765,9 +765,13 @@ export function parseIncomingMessage(message: IncomingMessage) { } } -async function binaryToString(body: Buffer | Readable) { +async function binaryToString(body: Buffer | Readable, encoding?: BufferEncoding) { const buffer = await binaryToBuffer(body); - return buffer.toString(); + if (!encoding && body instanceof IncomingMessage) { + parseIncomingMessage(body); + encoding = body.encoding; + } + return buffer.toString(encoding); } export async function proxyRequestToAxios( diff --git a/packages/nodes-base/nodes/HttpRequest/test/encoding/HttpRequest.test.ts b/packages/nodes-base/nodes/HttpRequest/test/encoding/HttpRequest.test.ts new file mode 100644 index 0000000000000..6ca5cfa437aaa --- /dev/null +++ b/packages/nodes-base/nodes/HttpRequest/test/encoding/HttpRequest.test.ts @@ -0,0 +1,40 @@ +import nock from 'nock'; +import { + setup, + equalityTest, + workflowToTests, + getWorkflowFilenames, + initBinaryDataService, +} from '@test/nodes/Helpers'; + +describe('Test Response Encoding', () => { + const workflows = getWorkflowFilenames(__dirname); + const tests = workflowToTests(workflows); + + const baseUrl = 'https://dummy.domain'; + const payload = Buffer.from( + 'El rápido zorro marrón salta sobre el perro perezoso. ¡Qué bello día en París! Árbol, cañón, façade.', + 'latin1', + ); + + beforeAll(async () => { + await initBinaryDataService(); + + nock.disableNetConnect(); + + nock(baseUrl) + .persist() + .get('/index.html') + .reply(200, payload, { 'content-type': 'text/plain; charset=latin1' }); + }); + + afterAll(() => { + nock.restore(); + }); + + const nodeTypes = setup(tests); + + for (const testData of tests) { + test(testData.description, async () => await equalityTest(testData, nodeTypes)); + } +}); diff --git a/packages/nodes-base/nodes/HttpRequest/test/encoding/encoding.test.json b/packages/nodes-base/nodes/HttpRequest/test/encoding/encoding.test.json new file mode 100644 index 0000000000000..14deea23c2ad0 --- /dev/null +++ b/packages/nodes-base/nodes/HttpRequest/test/encoding/encoding.test.json @@ -0,0 +1,78 @@ +{ + "name": "Response Encoding Test", + "nodes": [ + { + "parameters": {}, + "name": "When clicking \"Execute Workflow\"", + "type": "n8n-nodes-base.manualTrigger", + "typeVersion": 1, + "position": [ + 180, + 820 + ], + "id": "635fb102-a760-4b9e-836c-82e71bba7974" + }, + { + "parameters": { + "url": "https://dummy.domain/index.html", + "options": {} + }, + "name": "HTTP Request (v3)", + "type": "n8n-nodes-base.httpRequest", + "typeVersion": 3, + "position": [ + 520, + 720 + ], + "id": "eb243cfd-fbd6-41ef-935d-4ea98617355f" + }, + { + "parameters": { + "url": "https://dummy.domain/index.html", + "options": {} + }, + "name": "HTTP Request (v4)", + "type": "n8n-nodes-base.httpRequest", + "typeVersion": 4, + "position": [ + 520, + 920 + ], + "id": "cc2f185d-df6a-4fa3-b7f4-29f0dbad0f9b" + } + ], + "connections": { + "When clicking \"Execute Workflow\"": { + "main": [ + [ + { + "node": "HTTP Request (v3)", + "type": "main", + "index": 0 + }, + { + "node": "HTTP Request (v4)", + "type": "main", + "index": 0 + } + ] + ] + } + }, + "pinData": { + "HTTP Request (v3)": [ + { + "json": { + "data": "El rápido zorro marrón salta sobre el perro perezoso. ¡Qué bello día en París! Árbol, cañón, façade." + } + } + ], + "HTTP Request (v4)": [ + { + "json": { + "data": "El rápido zorro marrón salta sobre el perro perezoso. ¡Qué bello día en París! Árbol, cañón, façade." + } + } + ] + } +} diff --git a/packages/workflow/src/Interfaces.ts b/packages/workflow/src/Interfaces.ts index 5fbde49d7bad7..68dd5bea06d81 100644 --- a/packages/workflow/src/Interfaces.ts +++ b/packages/workflow/src/Interfaces.ts @@ -750,7 +750,7 @@ export interface BinaryHelperFunctions { setBinaryDataBuffer(data: IBinaryData, binaryData: Buffer): Promise; copyBinaryFile(): Promise; binaryToBuffer(body: Buffer | Readable): Promise; - binaryToString(body: Buffer | Readable): Promise; + binaryToString(body: Buffer | Readable, encoding?: BufferEncoding): Promise; getBinaryPath(binaryDataId: string): string; getBinaryStream(binaryDataId: string, chunkSize?: number): Promise; getBinaryMetadata(binaryDataId: string): Promise<{