From d85d0ecf45e8f256536bdd7cad6aab85971e8e43 Mon Sep 17 00:00:00 2001
From: pemontto <939704+pemontto@users.noreply.github.com>
Date: Thu, 14 Mar 2024 22:17:20 +1100
Subject: [PATCH] fix(core): Remove HTTP body for GET, HEAD, and OPTIONS
 requests (#3621)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: Jonathan Bennetts <jonathan.bennetts@gmail.com>
Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <aditya@netroy.in>
---
 packages/core/src/NodeExecuteFunctions.ts     | 23 +++++++++++
 .../core/test/NodeExecuteFunctions.test.ts    | 41 +++++++++++++++++++
 2 files changed, 64 insertions(+)

diff --git a/packages/core/src/NodeExecuteFunctions.ts b/packages/core/src/NodeExecuteFunctions.ts
index 5f711bae0c1e3..cac599b5cad13 100644
--- a/packages/core/src/NodeExecuteFunctions.ts
+++ b/packages/core/src/NodeExecuteFunctions.ts
@@ -31,6 +31,7 @@ import { access as fsAccess, writeFile as fsWriteFile } from 'fs/promises';
 import { IncomingMessage, type IncomingHttpHeaders } from 'http';
 import { Agent, type AgentOptions } from 'https';
 import get from 'lodash/get';
+import isEmpty from 'lodash/isEmpty';
 import pick from 'lodash/pick';
 import { extension, lookup } from 'mime-types';
 import type {
@@ -962,9 +963,21 @@ function convertN8nRequestToAxios(n8nRequest: IHttpRequestOptions): AxiosRequest
 	return axiosRequest;
 }
 
+const NoBodyHttpMethods = ['GET', 'HEAD', 'OPTIONS'];
+
+/** Remove empty request body on GET, HEAD, and OPTIONS requests */
+export const removeEmptyBody = (requestOptions: IHttpRequestOptions | IRequestOptions) => {
+	const method = requestOptions.method || 'GET';
+	if (NoBodyHttpMethods.includes(method) && isEmpty(requestOptions.body)) {
+		delete requestOptions.body;
+	}
+};
+
 async function httpRequest(
 	requestOptions: IHttpRequestOptions,
 ): Promise<IN8nHttpFullResponse | IN8nHttpResponse> {
+	removeEmptyBody(requestOptions);
+
 	let axiosRequest = convertN8nRequestToAxios(requestOptions);
 	if (
 		axiosRequest.data === undefined ||
@@ -972,6 +985,7 @@ async function httpRequest(
 	) {
 		delete axiosRequest.data;
 	}
+
 	let result: AxiosResponse<any>;
 	try {
 		result = await axios(axiosRequest);
@@ -1276,6 +1290,8 @@ export async function requestOAuth2(
 	oAuth2Options?: IOAuth2Options,
 	isN8nRequest = false,
 ) {
+	removeEmptyBody(requestOptions);
+
 	const credentials = (await this.getCredentials(
 		credentialsType,
 	)) as unknown as OAuth2CredentialData;
@@ -1511,6 +1527,8 @@ export async function requestOAuth1(
 	requestOptions: IHttpRequestOptions | IRequestOptions,
 	isN8nRequest = false,
 ) {
+	removeEmptyBody(requestOptions);
+
 	const credentials = await this.getCredentials(credentialsType);
 
 	if (credentials === undefined) {
@@ -1584,9 +1602,12 @@ export async function httpRequestWithAuthentication(
 	additionalData: IWorkflowExecuteAdditionalData,
 	additionalCredentialOptions?: IAdditionalCredentialOptions,
 ) {
+	removeEmptyBody(requestOptions);
+
 	let credentialsDecrypted: ICredentialDataDecryptedObject | undefined;
 	try {
 		const parentTypes = additionalData.credentialsHelper.getParentTypes(credentialsType);
+
 		if (parentTypes.includes('oAuth1Api')) {
 			return await requestOAuth1.call(this, credentialsType, requestOptions, true);
 		}
@@ -1777,6 +1798,8 @@ export async function requestWithAuthentication(
 	additionalCredentialOptions?: IAdditionalCredentialOptions,
 	itemIndex?: number,
 ) {
+	removeEmptyBody(requestOptions);
+
 	let credentialsDecrypted: ICredentialDataDecryptedObject | undefined;
 
 	try {
diff --git a/packages/core/test/NodeExecuteFunctions.test.ts b/packages/core/test/NodeExecuteFunctions.test.ts
index 1511b89ee13a6..13791d126caaa 100644
--- a/packages/core/test/NodeExecuteFunctions.test.ts
+++ b/packages/core/test/NodeExecuteFunctions.test.ts
@@ -4,6 +4,7 @@ import {
 	parseIncomingMessage,
 	parseRequestObject,
 	proxyRequestToAxios,
+	removeEmptyBody,
 	setBinaryDataBuffer,
 } from '@/NodeExecuteFunctions';
 import { mkdtempSync, readFileSync } from 'fs';
@@ -12,7 +13,9 @@ import { mock } from 'jest-mock-extended';
 import type {
 	IBinaryData,
 	IHttpRequestMethods,
+	IHttpRequestOptions,
 	INode,
+	IRequestOptions,
 	ITaskDataConnections,
 	IWorkflowExecuteAdditionalData,
 	Workflow,
@@ -458,4 +461,42 @@ describe('NodeExecuteFunctions', () => {
 			expect(output[0].a === input.a).toEqual(false);
 		});
 	});
+
+	describe('removeEmptyBody', () => {
+		test.each(['GET', 'HEAD', 'OPTIONS'] as IHttpRequestMethods[])(
+			'Should remove empty body for %s',
+			async (method) => {
+				const requestOptions = {
+					method,
+					body: {},
+				} as IHttpRequestOptions | IRequestOptions;
+				removeEmptyBody(requestOptions);
+				expect(requestOptions.body).toEqual(undefined);
+			},
+		);
+
+		test.each(['GET', 'HEAD', 'OPTIONS'] as IHttpRequestMethods[])(
+			'Should not remove non-empty body for %s',
+			async (method) => {
+				const requestOptions = {
+					method,
+					body: { test: true },
+				} as IHttpRequestOptions | IRequestOptions;
+				removeEmptyBody(requestOptions);
+				expect(requestOptions.body).toEqual({ test: true });
+			},
+		);
+
+		test.each(['POST', 'PUT', 'PATCH', 'DELETE'] as IHttpRequestMethods[])(
+			'Should not remove empty body for %s',
+			async (method) => {
+				const requestOptions = {
+					method,
+					body: {},
+				} as IHttpRequestOptions | IRequestOptions;
+				removeEmptyBody(requestOptions);
+				expect(requestOptions.body).toEqual({});
+			},
+		);
+	});
 });