fix(core): Ensure maxRedirects is used for any http request definin…

…g it (#8706)
n8n-io · Feb 23, 2024 · be8167b · be8167b
1 parent c75b240
commit be8167b
Show file tree

Hide file tree

Showing 5 changed files with 62 additions and 17 deletions.
diff --git a/packages/core/src/NodeExecuteFunctions.ts b/packages/core/src/NodeExecuteFunctions.ts
@@ -480,16 +480,14 @@ export async function parseRequestObject(requestObject: IRequestOptions) {
 	}
 
 	// Axios will follow redirects by default, so we simply tell it otherwise if needed.
+	const { method } = requestObject;
 	if (
-		requestObject.followRedirect === false &&
-		((requestObject.method as string | undefined) || 'get').toLowerCase() === 'get'
-	) {
-		axiosConfig.maxRedirects = 0;
-	}
-	if (
-		requestObject.followAllRedirects === false &&
-		((requestObject.method as string | undefined) || 'get').toLowerCase() !== 'get'
+		(requestObject.followRedirect !== false &&
+			(!method || method === 'GET' || method === 'HEAD')) ||
+		requestObject.followAllRedirects
 	) {
+		axiosConfig.maxRedirects = requestObject.maxRedirects;
+	} else {
 		axiosConfig.maxRedirects = 0;
 	}
 

diff --git a/packages/core/test/NodeExecuteFunctions.test.ts b/packages/core/test/NodeExecuteFunctions.test.ts
@@ -11,6 +11,7 @@ import type { IncomingMessage } from 'http';
 import { mock } from 'jest-mock-extended';
 import type {
 	IBinaryData,
+	IHttpRequestMethods,
 	INode,
 	ITaskDataConnections,
 	IWorkflowExecuteAdditionalData,
@@ -368,6 +369,46 @@ describe('NodeExecuteFunctions', () => {
 			});
 			expect((axiosOptions.httpsAgent as Agent).options.servername).toEqual('example.de');
 		});
+
+		describe('when followRedirect is true', () => {
+			test.each(['GET', 'HEAD'] as IHttpRequestMethods[])(
+				'should set maxRedirects on %s ',
+				async (method) => {
+					const axiosOptions = await parseRequestObject({
+						method,
+						followRedirect: true,
+						maxRedirects: 1234,
+					});
+					expect(axiosOptions.maxRedirects).toEqual(1234);
+				},
+			);
+
+			test.each(['POST', 'PUT', 'PATCH', 'DELETE'] as IHttpRequestMethods[])(
+				'should not set maxRedirects on %s ',
+				async (method) => {
+					const axiosOptions = await parseRequestObject({
+						method,
+						followRedirect: true,
+						maxRedirects: 1234,
+					});
+					expect(axiosOptions.maxRedirects).toEqual(0);
+				},
+			);
+		});
+
+		describe('when followAllRedirects is true', () => {
+			test.each(['GET', 'HEAD', 'POST', 'PUT', 'PATCH', 'DELETE'] as IHttpRequestMethods[])(
+				'should set maxRedirects on %s ',
+				async (method) => {
+					const axiosOptions = await parseRequestObject({
+						method,
+						followAllRedirects: true,
+						maxRedirects: 1234,
+					});
+					expect(axiosOptions.maxRedirects).toEqual(1234);
+				},
+			);
+		});
 	});
 
 	describe('copyInputItems', () => {

diff --git a/packages/nodes-base/nodes/HttpRequest/V1/HttpRequestV1.node.ts b/packages/nodes-base/nodes/HttpRequest/V1/HttpRequestV1.node.ts
@@ -325,14 +325,14 @@ export class HttpRequestV1 implements INodeType {
 							name: 'followAllRedirects',
 							type: 'boolean',
 							default: false,
-							description: 'Whether to follow non-GET HTTP 3xx redirects',
+							description: 'Whether to follow All HTTP 3xx redirects',
 						},
 						{
-							displayName: 'Follow GET Redirect',
+							displayName: 'Follow GET/HEAD Redirect',
 							name: 'followRedirect',
 							type: 'boolean',
 							default: true,
-							description: 'Whether to follow GET HTTP 3xx redirects',
+							description: 'Whether to follow GET or HEAD HTTP 3xx redirects',
 						},
 						{
 							displayName: 'Ignore Response Code',

diff --git a/packages/nodes-base/nodes/HttpRequest/V2/HttpRequestV2.node.ts b/packages/nodes-base/nodes/HttpRequest/V2/HttpRequestV2.node.ts
@@ -340,14 +340,14 @@ export class HttpRequestV2 implements INodeType {
 							name: 'followAllRedirects',
 							type: 'boolean',
 							default: false,
-							description: 'Whether to follow non-GET HTTP 3xx redirects',
+							description: 'Whether to follow All HTTP 3xx redirects',
 						},
 						{
-							displayName: 'Follow GET Redirect',
+							displayName: 'Follow GET/HEAD Redirect',
 							name: 'followRedirect',
 							type: 'boolean',
 							default: true,
-							description: 'Whether to follow GET HTTP 3xx redirects',
+							description: 'Whether to follow GET or HEAD HTTP 3xx redirects',
 						},
 						{
 							displayName: 'Ignore Response Code',

diff --git a/packages/workflow/src/Interfaces.ts b/packages/workflow/src/Interfaces.ts
@@ -552,15 +552,21 @@ export interface IRequestOptions {
 	json?: boolean;
 	useStream?: boolean;
 	encoding?: string | null;
-	followRedirect?: boolean;
-	followAllRedirects?: boolean;
 	timeout?: number;
 	rejectUnauthorized?: boolean;
 	proxy?: string | AxiosProxyConfig;
 	simple?: boolean;
 	gzip?: boolean;
-	maxRedirects?: number;
 	resolveWithFullResponse?: boolean;
+
+	/** Whether to follow GET or HEAD HTTP 3xx redirects @default true */
+	followRedirect?: boolean;
+
+	/** Whether to follow **All** HTTP 3xx redirects @default false */
+	followAllRedirects?: boolean;
+
+	/** Max number of redirects to follow @default 21 */
+	maxRedirects?: number;
 }
 
 export interface PaginationOptions {