Skip to content

Commit

Permalink
fix(HTTP Request Node): Respect the original encoding of the incoming…
Browse files Browse the repository at this point in the history
… response (#9869)
  • Loading branch information
netroy authored Jul 11, 2024
1 parent e84ab35 commit 2d19aef
Show file tree
Hide file tree
Showing 9 changed files with 152 additions and 41 deletions.
8 changes: 2 additions & 6 deletions packages/core/src/BinaryData/BinaryData.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);

Expand All @@ -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(':');

Expand Down
8 changes: 2 additions & 6 deletions packages/core/src/BinaryData/ObjectStore.manager.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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);

Expand Down Expand Up @@ -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);
}
}
3 changes: 2 additions & 1 deletion packages/core/src/BinaryData/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<Buffer>((resolve, reject) => {
body
Expand Down
33 changes: 17 additions & 16 deletions packages/core/src/NodeExecuteFunctions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -764,6 +765,15 @@ export function parseIncomingMessage(message: IncomingMessage) {
}
}

async function binaryToString(body: Buffer | Readable, encoding?: BufferEncoding) {
const buffer = await binaryToBuffer(body);
if (!encoding && body instanceof IncomingMessage) {
parseIncomingMessage(body);
encoding = body.encoding;
}
return buffer.toString(encoding);
}

export async function proxyRequestToAxios(
workflow: Workflow | undefined,
additionalData: IWorkflowExecuteAdditionalData | undefined,
Expand Down Expand Up @@ -837,9 +847,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) {
Expand Down Expand Up @@ -3091,17 +3099,14 @@ const getRequestHelperFunctions = (
let contentBody: Exclude<IN8nHttpResponse, Buffer>;

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;
Expand Down Expand Up @@ -3187,9 +3192,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);
}
Expand Down Expand Up @@ -3400,8 +3403,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) =>
Expand Down Expand Up @@ -3743,8 +3746,6 @@ export function getExecuteFunctions(
);
return dataProxy.getDataProxy();
},
binaryToBuffer: async (body: Buffer | Readable) =>
await Container.get(BinaryDataService).toBuffer(body),
async putExecutionToWait(waitTill: Date): Promise<void> {
runExecutionData.waitTill = waitTill;
if (additionalData.setExecutionStatus) {
Expand Down
14 changes: 8 additions & 6 deletions packages/core/test/BinaryData/utils.test.ts
Original file line number Diff line number Diff line change
@@ -1,31 +1,33 @@
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 () => {
const gunzip = createGunzip();
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'),
);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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: {} }
Expand All @@ -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;
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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));
}
});
Original file line number Diff line number Diff line change
@@ -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."
}
}
]
}
}
1 change: 1 addition & 0 deletions packages/workflow/src/Interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -750,6 +750,7 @@ export interface BinaryHelperFunctions {
setBinaryDataBuffer(data: IBinaryData, binaryData: Buffer): Promise<IBinaryData>;
copyBinaryFile(): Promise<never>;
binaryToBuffer(body: Buffer | Readable): Promise<Buffer>;
binaryToString(body: Buffer | Readable, encoding?: BufferEncoding): Promise<string>;
getBinaryPath(binaryDataId: string): string;
getBinaryStream(binaryDataId: string, chunkSize?: number): Promise<Readable>;
getBinaryMetadata(binaryDataId: string): Promise<{
Expand Down

0 comments on commit 2d19aef

Please sign in to comment.