Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(HTTP Request Node): Respect the original encoding of the incoming response #9869

Merged
merged 2 commits into from
Jul 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading