Merge branch 'master' into ado-3164-be-create-migration-to-integrate-…

…new-model
n8n-io · Feb 5, 2025 · 45fc491 · 45fc491
2 parents deff9b9 + 2a33d07
commit 45fc491
Show file tree

Hide file tree

Showing 73 changed files with 6,992 additions and 369 deletions.
diff --git a/cypress/composables/ndv.ts b/cypress/composables/ndv.ts
@@ -225,7 +225,7 @@ export function populateMapperFields(fields: ReadonlyArray<[string, string]>) {
 		getParameterInputByName(name).type(value);
 
 		// Click on a parent to dismiss the pop up which hides the field below.
-		getParameterInputByName(name).parent().parent().parent().click('topLeft');
+		getParameterInputByName(name).parent().parent().parent().parent().click('topLeft');
 	}
 }
 

diff --git a/cypress/e2e/16-form-trigger-node.cy.ts b/cypress/e2e/16-form-trigger-node.cy.ts
@@ -62,12 +62,14 @@ describe('n8n Form Trigger', () => {
 		getVisibleSelect().contains('Dropdown').click();
 		cy.contains('button', 'Add Field Option').click();
 		cy.contains('label', 'Field Options')
+			.parent()
 			.parent()
 			.nextAll()
 			.find('[data-test-id="parameter-input-field"]')
 			.eq(0)
 			.type('Option 1');
 		cy.contains('label', 'Field Options')
+			.parent()
 			.parent()
 			.nextAll()
 			.find('[data-test-id="parameter-input-field"]')

diff --git a/cypress/e2e/48-subworkflow-inputs.cy.ts b/cypress/e2e/48-subworkflow-inputs.cy.ts
@@ -90,8 +90,8 @@ describe('Sub-workflow creation and typed usage', () => {
 		clickExecuteNode();
 
 		const expected = [
-			['-1', 'A String', '0:11:true2:3', 'aKey:-1', '[empty object]', 'false'],
-			['-1', 'Another String', '[empty array]', 'aDifferentKey:-1', '[empty array]', 'false'],
+			['-1', 'A String', '0:11:true2:3', 'aKey:-1', '[empty object]', 'true'],
+			['-1', 'Another String', '[empty array]', 'aDifferentKey:-1', '[empty array]', 'true'],
 		];
 		assertOutputTableContent(expected);
 
@@ -110,8 +110,8 @@ describe('Sub-workflow creation and typed usage', () => {
 		clickExecuteNode();
 
 		const expected2 = [
-			['-1', '5', '0:11:true2:3', 'aKey:-1', '[empty object]', 'false'],
-			['-1', '5', '[empty array]', 'aDifferentKey:-1', '[empty array]', 'false'],
+			['-1', '5', '0:11:true2:3', 'aKey:-1', '[empty object]', 'true'],
+			['-1', '5', '[empty array]', 'aDifferentKey:-1', '[empty array]', 'true'],
 		];
 
 		assertOutputTableContent(expected2);
@@ -167,8 +167,8 @@ describe('Sub-workflow creation and typed usage', () => {
 				);
 
 				assertOutputTableContent([
-					['[null]', '[null]', '[null]', '[null]', '[null]', 'false'],
-					['[null]', '[null]', '[null]', '[null]', '[null]', 'false'],
+					['[null]', '[null]', '[null]', '[null]', '[null]', 'true'],
+					['[null]', '[null]', '[null]', '[null]', '[null]', 'true'],
 				]);
 
 				clickExecuteNode();

diff --git a/...s/@n8n/nodes-langchain/nodes/agents/Agent/agents/SqlAgent/other/handlers/postgres.test.ts b/...s/@n8n/nodes-langchain/nodes/agents/Agent/agents/SqlAgent/other/handlers/postgres.test.ts
@@ -0,0 +1,156 @@
+import { mock } from 'jest-mock-extended';
+import type { PostgresNodeCredentials } from 'n8n-nodes-base/nodes/Postgres/v2/helpers/interfaces';
+import type { IExecuteFunctions } from 'n8n-workflow';
+
+import { getPostgresDataSource } from './postgres';
+
+describe('Postgres SSL settings', () => {
+	const credentials = mock<PostgresNodeCredentials>({
+		host: 'localhost',
+		port: 5432,
+		user: 'user',
+		password: 'password',
+		database: 'database',
+	});
+
+	test('ssl is disabled + allowUnauthorizedCerts is false', async () => {
+		const context = mock<IExecuteFunctions>({
+			getCredentials: jest.fn().mockReturnValue({
+				...credentials,
+				ssl: 'disable',
+				allowUnauthorizedCerts: false,
+			}),
+		});
+
+		const dataSource = await getPostgresDataSource.call(context);
+
+		expect(dataSource.options).toMatchObject({
+			ssl: false,
+		});
+	});
+
+	test('ssl is disabled + allowUnauthorizedCerts is true', async () => {
+		const context = mock<IExecuteFunctions>({
+			getCredentials: jest.fn().mockReturnValue({
+				...credentials,
+				ssl: 'disable',
+				allowUnauthorizedCerts: true,
+			}),
+		});
+
+		const dataSource = await getPostgresDataSource.call(context);
+
+		expect(dataSource.options).toMatchObject({
+			ssl: false,
+		});
+	});
+
+	test('ssl is disabled + allowUnauthorizedCerts is undefined', async () => {
+		const context = mock<IExecuteFunctions>({
+			getCredentials: jest.fn().mockReturnValue({
+				...credentials,
+				ssl: 'disable',
+			}),
+		});
+
+		const dataSource = await getPostgresDataSource.call(context);
+
+		expect(dataSource.options).toMatchObject({
+			ssl: false,
+		});
+	});
+
+	test('ssl is allow + allowUnauthorizedCerts is false', async () => {
+		const context = mock<IExecuteFunctions>({
+			getCredentials: jest.fn().mockReturnValue({
+				...credentials,
+				ssl: 'allow',
+				allowUnauthorizedCerts: false,
+			}),
+		});
+
+		const dataSource = await getPostgresDataSource.call(context);
+
+		expect(dataSource.options).toMatchObject({
+			ssl: true,
+		});
+	});
+
+	test('ssl is allow + allowUnauthorizedCerts is true', async () => {
+		const context = mock<IExecuteFunctions>({
+			getCredentials: jest.fn().mockReturnValue({
+				...credentials,
+				ssl: 'allow',
+				allowUnauthorizedCerts: true,
+			}),
+		});
+
+		const dataSource = await getPostgresDataSource.call(context);
+
+		expect(dataSource.options).toMatchObject({
+			ssl: { rejectUnauthorized: false },
+		});
+	});
+
+	test('ssl is allow + allowUnauthorizedCerts is undefined', async () => {
+		const context = mock<IExecuteFunctions>({
+			getCredentials: jest.fn().mockReturnValue({
+				...credentials,
+				ssl: 'allow',
+			}),
+		});
+
+		const dataSource = await getPostgresDataSource.call(context);
+
+		expect(dataSource.options).toMatchObject({
+			ssl: true,
+		});
+	});
+
+	test('ssl is require + allowUnauthorizedCerts is false', async () => {
+		const context = mock<IExecuteFunctions>({
+			getCredentials: jest.fn().mockReturnValue({
+				...credentials,
+				ssl: 'require',
+				allowUnauthorizedCerts: false,
+			}),
+		});
+
+		const dataSource = await getPostgresDataSource.call(context);
+
+		expect(dataSource.options).toMatchObject({
+			ssl: true,
+		});
+	});
+
+	test('ssl is require + allowUnauthorizedCerts is true', async () => {
+		const context = mock<IExecuteFunctions>({
+			getCredentials: jest.fn().mockReturnValue({
+				...credentials,
+				ssl: 'require',
+				allowUnauthorizedCerts: true,
+			}),
+		});
+
+		const dataSource = await getPostgresDataSource.call(context);
+
+		expect(dataSource.options).toMatchObject({
+			ssl: { rejectUnauthorized: false },
+		});
+	});
+
+	test('ssl is require + allowUnauthorizedCerts is undefined', async () => {
+		const context = mock<IExecuteFunctions>({
+			getCredentials: jest.fn().mockReturnValue({
+				...credentials,
+				ssl: 'require',
+			}),
+		});
+
+		const dataSource = await getPostgresDataSource.call(context);
+
+		expect(dataSource.options).toMatchObject({
+			ssl: true,
+		});
+	});
+});
diff --git a/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/SqlAgent/other/handlers/postgres.ts b/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/SqlAgent/other/handlers/postgres.ts
@@ -1,29 +1,23 @@
 import { DataSource } from '@n8n/typeorm';
+import type { PostgresNodeCredentials } from 'n8n-nodes-base/dist/nodes/Postgres/v2/helpers/interfaces';
 import { type IExecuteFunctions } from 'n8n-workflow';
+import type { TlsOptions } from 'tls';
 
 export async function getPostgresDataSource(this: IExecuteFunctions): Promise<DataSource> {
-	const credentials = await this.getCredentials('postgres');
+	const credentials = await this.getCredentials<PostgresNodeCredentials>('postgres');
 
-	const dataSource = new DataSource({
-		type: 'postgres',
-		host: credentials.host as string,
-		port: credentials.port as number,
-		username: credentials.user as string,
-		password: credentials.password as string,
-		database: credentials.database as string,
-	});
-
-	if (credentials.allowUnauthorizedCerts === true) {
-		dataSource.setOptions({
-			ssl: {
-				rejectUnauthorized: true,
-			},
-		});
-	} else {
-		dataSource.setOptions({
-			ssl: !['disable', undefined].includes(credentials.ssl as string | undefined),
-		});
+	let ssl: TlsOptions | boolean = !['disable', undefined].includes(credentials.ssl);
+	if (credentials.allowUnauthorizedCerts && ssl) {
+		ssl = { rejectUnauthorized: false };
 	}
 
-	return dataSource;
+	return new DataSource({
+		type: 'postgres',
+		host: credentials.host,
+		port: credentials.port,
+		username: credentials.user,
+		password: credentials.password,
+		database: credentials.database,
+		ssl,
+	});
 }
diff --git a/packages/@n8n/nodes-langchain/nodes/tools/ToolHttpRequest/test/ToolHttpRequest.node.test.ts b/packages/@n8n/nodes-langchain/nodes/tools/ToolHttpRequest/test/ToolHttpRequest.node.test.ts
@@ -237,6 +237,69 @@ describe('ToolHttpRequest', () => {
 				}),
 			);
 		});
+
+		it('should return the error when receiving text that contains a null character', async () => {
+			helpers.httpRequest.mockResolvedValue({
+				body: 'Hello\0World',
+				headers: {
+					'content-type': 'text/plain',
+				},
+			});
+
+			executeFunctions.getNodeParameter.mockImplementation((paramName: string) => {
+				switch (paramName) {
+					case 'method':
+						return 'GET';
+					case 'url':
+						return 'https://httpbin.org/text/plain';
+					case 'options':
+						return {};
+					case 'placeholderDefinitions.values':
+						return [];
+					default:
+						return undefined;
+				}
+			});
+
+			const { response } = await httpTool.supplyData.call(executeFunctions, 0);
+			const res = await (response as N8nTool).invoke({});
+			expect(helpers.httpRequest).toHaveBeenCalled();
+			// Check that the returned string is formatted as an error message.
+			expect(res).toContain('error');
+			expect(res).toContain('Binary data is not supported');
+		});
+
+		it('should return the error when receiving a JSON response containing a null character', async () => {
+			// Provide a raw JSON string with a literal null character.
+			helpers.httpRequest.mockResolvedValue({
+				body: '{"message":"hello\0world"}',
+				headers: {
+					'content-type': 'application/json',
+				},
+			});
+
+			executeFunctions.getNodeParameter.mockImplementation((paramName: string) => {
+				switch (paramName) {
+					case 'method':
+						return 'GET';
+					case 'url':
+						return 'https://httpbin.org/json';
+					case 'options':
+						return {};
+					case 'placeholderDefinitions.values':
+						return [];
+					default:
+						return undefined;
+				}
+			});
+
+			const { response } = await httpTool.supplyData.call(executeFunctions, 0);
+			const res = await (response as N8nTool).invoke({});
+			expect(helpers.httpRequest).toHaveBeenCalled();
+			// Check that the tool returns an error string rather than resolving to valid JSON.
+			expect(res).toContain('error');
+			expect(res).toContain('Binary data is not supported');
+		});
 	});
 
 	describe('Optimize response', () => {

diff --git a/packages/@n8n/nodes-langchain/nodes/tools/ToolHttpRequest/utils.ts b/packages/@n8n/nodes-langchain/nodes/tools/ToolHttpRequest/utils.ts
@@ -5,7 +5,6 @@ import { JSDOM } from 'jsdom';
 import get from 'lodash/get';
 import set from 'lodash/set';
 import unset from 'lodash/unset';
-import * as mime from 'mime-types';
 import { getOAuth2AdditionalParameters } from 'n8n-nodes-base/dist/nodes/HttpRequest/GenericFunctions';
 import type {
 	IDataObject,
@@ -146,6 +145,25 @@ const defaultOptimizer = <T>(response: T) => {
 	return String(response);
 };
 
+function isBinary(data: unknown) {
+	// Check if data is a Buffer
+	if (Buffer.isBuffer(data)) {
+		return true;
+	}
+
+	// If data is a string, assume it's text unless it contains null characters.
+	if (typeof data === 'string') {
+		// If the string contains a null character, it's likely binary.
+		if (data.includes('\0')) {
+			return true;
+		}
+		return false;
+	}
+
+	// For any other type, assume it's not binary.
+	return false;
+}
+
 const htmlOptimizer = (ctx: ISupplyDataFunctions, itemIndex: number, maxLength: number) => {
 	const cssSelector = ctx.getNodeParameter('cssSelector', itemIndex, '') as string;
 	const onlyContent = ctx.getNodeParameter('onlyContent', itemIndex, false) as boolean;
@@ -755,13 +773,8 @@ export const configureToolFunction = (
 			if (!response) {
 				try {
 					// Check if the response is binary data
-					if (fullResponse?.headers?.['content-type']) {
-						const contentType = fullResponse.headers['content-type'] as string;
-						const mimeType = contentType.split(';')[0].trim();
-
-						if (mime.charset(mimeType) !== 'UTF-8') {
-							throw new NodeOperationError(ctx.getNode(), 'Binary data is not supported');
-						}
+					if (fullResponse.body && isBinary(fullResponse.body)) {
+						throw new NodeOperationError(ctx.getNode(), 'Binary data is not supported');
 					}
 
 					response = optimizeResponse(fullResponse.body);