From b90c084a406975f03769ce30baab9a522b95a689 Mon Sep 17 00:00:00 2001 From: pemontto Date: Fri, 16 Sep 2022 14:59:08 +0100 Subject: [PATCH 1/6] =?UTF-8?q?=F0=9F=90=9B=20Remove=20empty=20HTTP=20body?= =?UTF-8?q?=20for=20relevant=20requests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/core/src/NodeExecuteFunctions.ts | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/packages/core/src/NodeExecuteFunctions.ts b/packages/core/src/NodeExecuteFunctions.ts index b8b1809fd9751..c637e6cf73a38 100644 --- a/packages/core/src/NodeExecuteFunctions.ts +++ b/packages/core/src/NodeExecuteFunctions.ts @@ -69,7 +69,7 @@ import clientOAuth1, { Token } from 'oauth-1.0a'; import clientOAuth2 from 'client-oauth2'; import crypto, { createHmac } from 'crypto'; // eslint-disable-next-line import/no-extraneous-dependencies -import { get } from 'lodash'; +import { get, isEmpty } from 'lodash'; // eslint-disable-next-line import/no-extraneous-dependencies import express from 'express'; import FormData from 'form-data'; @@ -785,6 +785,13 @@ async function httpRequest( delete axiosRequest.data; } + if ( + ['GET', 'HEAD', 'OPTIONS', 'TRACE'].includes(requestOptions.method as string) && + isEmpty(requestOptions.body) + ) { + delete requestOptions.body; + } + const result = await axios(axiosRequest); if (requestOptions.returnFullResponse) { return { @@ -1220,6 +1227,14 @@ export async function httpRequestWithAuthentication( let credentialsDecrypted: ICredentialDataDecryptedObject | undefined; try { const parentTypes = additionalData.credentialsHelper.getParentTypes(credentialsType); + + if ( + ['GET', 'HEAD', 'OPTIONS', 'TRACE'].includes(requestOptions.method as string) && + isEmpty(requestOptions.body) + ) { + delete requestOptions.body; + } + if (parentTypes.includes('oAuth1Api')) { return await requestOAuth1.call(this, credentialsType, requestOptions, true); } From 986b7fba2be46b58dedb46c86608519ff73117fb Mon Sep 17 00:00:00 2001 From: Jonathan Bennetts Date: Thu, 22 Feb 2024 16:13:30 +0000 Subject: [PATCH 2/6] Update --- packages/core/src/NodeExecuteFunctions.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/core/src/NodeExecuteFunctions.ts b/packages/core/src/NodeExecuteFunctions.ts index a29f2fe8c171c..3e4c4a28f83c8 100644 --- a/packages/core/src/NodeExecuteFunctions.ts +++ b/packages/core/src/NodeExecuteFunctions.ts @@ -1597,7 +1597,6 @@ export async function httpRequestWithAuthentication( let credentialsDecrypted: ICredentialDataDecryptedObject | undefined; try { const parentTypes = additionalData.credentialsHelper.getParentTypes(credentialsType); - if ( ['GET', 'HEAD', 'OPTIONS', 'TRACE'].includes(requestOptions.method as string) && isEmpty(requestOptions.body) @@ -1800,6 +1799,13 @@ export async function requestWithAuthentication( try { const parentTypes = additionalData.credentialsHelper.getParentTypes(credentialsType); + if ( + ['GET', 'HEAD', 'OPTIONS', 'TRACE'].includes(requestOptions.method as string) && + isEmpty(requestOptions.body) + ) { + delete requestOptions.body; + } + if (credentialsType === 'oAuth1Api' || parentTypes.includes('oAuth1Api')) { return await requestOAuth1.call(this, credentialsType, requestOptions, false); } From 997b838f18a05b132a3151ead36c5f12e5cbd20a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Thu, 22 Feb 2024 17:34:55 +0100 Subject: [PATCH 3/6] remove empty body on all request helpers --- packages/core/src/NodeExecuteFunctions.ts | 39 ++++++++++++----------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/packages/core/src/NodeExecuteFunctions.ts b/packages/core/src/NodeExecuteFunctions.ts index 3e4c4a28f83c8..bc9f7ae32797b 100644 --- a/packages/core/src/NodeExecuteFunctions.ts +++ b/packages/core/src/NodeExecuteFunctions.ts @@ -965,9 +965,21 @@ function convertN8nRequestToAxios(n8nRequest: IHttpRequestOptions): AxiosRequest return axiosRequest; } +const NoBodyHttpMethods = ['GET', 'HEAD', 'OPTIONS', 'TRACE']; + +/** Remove empty request body on GET, HEAD, OPTIONS, and TRACE requests */ +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 { + removeEmptyBody(requestOptions); + let axiosRequest = convertN8nRequestToAxios(requestOptions); if ( axiosRequest.data === undefined || @@ -976,12 +988,6 @@ async function httpRequest( delete axiosRequest.data; } - if ( - ['GET', 'HEAD', 'OPTIONS', 'TRACE'].includes(requestOptions.method as string) && - isEmpty(requestOptions.body) - ) { - delete requestOptions.body; - } let result: AxiosResponse; try { result = await axios(axiosRequest); @@ -1286,6 +1292,8 @@ export async function requestOAuth2( oAuth2Options?: IOAuth2Options, isN8nRequest = false, ) { + removeEmptyBody(requestOptions); + const credentials = (await this.getCredentials( credentialsType, )) as unknown as OAuth2CredentialData; @@ -1521,6 +1529,8 @@ export async function requestOAuth1( requestOptions: IHttpRequestOptions | IRequestOptions, isN8nRequest = false, ) { + removeEmptyBody(requestOptions); + const credentials = await this.getCredentials(credentialsType); if (credentials === undefined) { @@ -1594,15 +1604,11 @@ export async function httpRequestWithAuthentication( additionalData: IWorkflowExecuteAdditionalData, additionalCredentialOptions?: IAdditionalCredentialOptions, ) { + removeEmptyBody(requestOptions); + let credentialsDecrypted: ICredentialDataDecryptedObject | undefined; try { const parentTypes = additionalData.credentialsHelper.getParentTypes(credentialsType); - if ( - ['GET', 'HEAD', 'OPTIONS', 'TRACE'].includes(requestOptions.method as string) && - isEmpty(requestOptions.body) - ) { - delete requestOptions.body; - } if (parentTypes.includes('oAuth1Api')) { return await requestOAuth1.call(this, credentialsType, requestOptions, true); @@ -1794,18 +1800,13 @@ export async function requestWithAuthentication( additionalCredentialOptions?: IAdditionalCredentialOptions, itemIndex?: number, ) { + removeEmptyBody(requestOptions); + let credentialsDecrypted: ICredentialDataDecryptedObject | undefined; try { const parentTypes = additionalData.credentialsHelper.getParentTypes(credentialsType); - if ( - ['GET', 'HEAD', 'OPTIONS', 'TRACE'].includes(requestOptions.method as string) && - isEmpty(requestOptions.body) - ) { - delete requestOptions.body; - } - if (credentialsType === 'oAuth1Api' || parentTypes.includes('oAuth1Api')) { return await requestOAuth1.call(this, credentialsType, requestOptions, false); } From 163558e8d338cbfb4a2ffdb17d80dd6e89d28f0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Thu, 22 Feb 2024 18:06:39 +0100 Subject: [PATCH 4/6] add unit tests --- packages/core/src/NodeExecuteFunctions.ts | 2 +- .../core/test/NodeExecuteFunctions.test.ts | 41 +++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/packages/core/src/NodeExecuteFunctions.ts b/packages/core/src/NodeExecuteFunctions.ts index dc5c35f08f913..1e49d54f8008e 100644 --- a/packages/core/src/NodeExecuteFunctions.ts +++ b/packages/core/src/NodeExecuteFunctions.ts @@ -966,7 +966,7 @@ function convertN8nRequestToAxios(n8nRequest: IHttpRequestOptions): AxiosRequest const NoBodyHttpMethods = ['GET', 'HEAD', 'OPTIONS', 'TRACE']; /** Remove empty request body on GET, HEAD, OPTIONS, and TRACE requests */ -const removeEmptyBody = (requestOptions: IHttpRequestOptions | IRequestOptions) => { +export const removeEmptyBody = (requestOptions: IHttpRequestOptions | IRequestOptions) => { const method = requestOptions.method || 'GET'; if (NoBodyHttpMethods.includes(method) && isEmpty(requestOptions.body)) { delete requestOptions.body; diff --git a/packages/core/test/NodeExecuteFunctions.test.ts b/packages/core/test/NodeExecuteFunctions.test.ts index 1511b89ee13a6..ea0c80c64bfa7 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', 'TRACE'] 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', 'TRACE'] 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({}); + }, + ); + }); }); From 3158ae2e21a5b35f3a3ca2de1bf7585d43a28b6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Thu, 22 Feb 2024 18:09:31 +0100 Subject: [PATCH 5/6] remove TRACE --- packages/core/src/NodeExecuteFunctions.ts | 4 ++-- packages/core/test/NodeExecuteFunctions.test.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/core/src/NodeExecuteFunctions.ts b/packages/core/src/NodeExecuteFunctions.ts index 1e49d54f8008e..76ef9162b4fb1 100644 --- a/packages/core/src/NodeExecuteFunctions.ts +++ b/packages/core/src/NodeExecuteFunctions.ts @@ -963,9 +963,9 @@ function convertN8nRequestToAxios(n8nRequest: IHttpRequestOptions): AxiosRequest return axiosRequest; } -const NoBodyHttpMethods = ['GET', 'HEAD', 'OPTIONS', 'TRACE']; +const NoBodyHttpMethods = ['GET', 'HEAD', 'OPTIONS']; -/** Remove empty request body on GET, HEAD, OPTIONS, and TRACE requests */ +/** 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)) { diff --git a/packages/core/test/NodeExecuteFunctions.test.ts b/packages/core/test/NodeExecuteFunctions.test.ts index ea0c80c64bfa7..13791d126caaa 100644 --- a/packages/core/test/NodeExecuteFunctions.test.ts +++ b/packages/core/test/NodeExecuteFunctions.test.ts @@ -463,7 +463,7 @@ describe('NodeExecuteFunctions', () => { }); describe('removeEmptyBody', () => { - test.each(['GET', 'HEAD', 'OPTIONS', 'TRACE'] as IHttpRequestMethods[])( + test.each(['GET', 'HEAD', 'OPTIONS'] as IHttpRequestMethods[])( 'Should remove empty body for %s', async (method) => { const requestOptions = { @@ -475,7 +475,7 @@ describe('NodeExecuteFunctions', () => { }, ); - test.each(['GET', 'HEAD', 'OPTIONS', 'TRACE'] as IHttpRequestMethods[])( + test.each(['GET', 'HEAD', 'OPTIONS'] as IHttpRequestMethods[])( 'Should not remove non-empty body for %s', async (method) => { const requestOptions = { From 3631991abb6cc6e4edbe513be4282323ca7b5026 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Thu, 22 Feb 2024 18:10:43 +0100 Subject: [PATCH 6/6] update import --- packages/core/src/NodeExecuteFunctions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/NodeExecuteFunctions.ts b/packages/core/src/NodeExecuteFunctions.ts index 76ef9162b4fb1..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 { @@ -148,7 +149,6 @@ import Container from 'typedi'; import type { BinaryData } from './BinaryData/types'; import merge from 'lodash/merge'; import { InstanceSettings } from './InstanceSettings'; -import { isEmpty } from 'lodash'; axios.defaults.timeout = 300000; // Prevent axios from adding x-form-www-urlencoded headers by default