fix(core): Remove HTTP body for GET, HEAD, and OPTIONS requests (#3621)

Co-authored-by: Jonathan Bennetts <jonathan.bennetts@gmail.com>
Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <aditya@netroy.in>
This commit is contained in:
pemontto 2024-03-14 22:17:20 +11:00 committed by GitHub
parent bcbff76055
commit d85d0ecf45
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 64 additions and 0 deletions

View file

@ -31,6 +31,7 @@ import { access as fsAccess, writeFile as fsWriteFile } from 'fs/promises';
import { IncomingMessage, type IncomingHttpHeaders } from 'http'; import { IncomingMessage, type IncomingHttpHeaders } from 'http';
import { Agent, type AgentOptions } from 'https'; import { Agent, type AgentOptions } from 'https';
import get from 'lodash/get'; import get from 'lodash/get';
import isEmpty from 'lodash/isEmpty';
import pick from 'lodash/pick'; import pick from 'lodash/pick';
import { extension, lookup } from 'mime-types'; import { extension, lookup } from 'mime-types';
import type { import type {
@ -962,9 +963,21 @@ function convertN8nRequestToAxios(n8nRequest: IHttpRequestOptions): AxiosRequest
return 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( async function httpRequest(
requestOptions: IHttpRequestOptions, requestOptions: IHttpRequestOptions,
): Promise<IN8nHttpFullResponse | IN8nHttpResponse> { ): Promise<IN8nHttpFullResponse | IN8nHttpResponse> {
removeEmptyBody(requestOptions);
let axiosRequest = convertN8nRequestToAxios(requestOptions); let axiosRequest = convertN8nRequestToAxios(requestOptions);
if ( if (
axiosRequest.data === undefined || axiosRequest.data === undefined ||
@ -972,6 +985,7 @@ async function httpRequest(
) { ) {
delete axiosRequest.data; delete axiosRequest.data;
} }
let result: AxiosResponse<any>; let result: AxiosResponse<any>;
try { try {
result = await axios(axiosRequest); result = await axios(axiosRequest);
@ -1276,6 +1290,8 @@ export async function requestOAuth2(
oAuth2Options?: IOAuth2Options, oAuth2Options?: IOAuth2Options,
isN8nRequest = false, isN8nRequest = false,
) { ) {
removeEmptyBody(requestOptions);
const credentials = (await this.getCredentials( const credentials = (await this.getCredentials(
credentialsType, credentialsType,
)) as unknown as OAuth2CredentialData; )) as unknown as OAuth2CredentialData;
@ -1511,6 +1527,8 @@ export async function requestOAuth1(
requestOptions: IHttpRequestOptions | IRequestOptions, requestOptions: IHttpRequestOptions | IRequestOptions,
isN8nRequest = false, isN8nRequest = false,
) { ) {
removeEmptyBody(requestOptions);
const credentials = await this.getCredentials(credentialsType); const credentials = await this.getCredentials(credentialsType);
if (credentials === undefined) { if (credentials === undefined) {
@ -1584,9 +1602,12 @@ export async function httpRequestWithAuthentication(
additionalData: IWorkflowExecuteAdditionalData, additionalData: IWorkflowExecuteAdditionalData,
additionalCredentialOptions?: IAdditionalCredentialOptions, additionalCredentialOptions?: IAdditionalCredentialOptions,
) { ) {
removeEmptyBody(requestOptions);
let credentialsDecrypted: ICredentialDataDecryptedObject | undefined; let credentialsDecrypted: ICredentialDataDecryptedObject | undefined;
try { try {
const parentTypes = additionalData.credentialsHelper.getParentTypes(credentialsType); const parentTypes = additionalData.credentialsHelper.getParentTypes(credentialsType);
if (parentTypes.includes('oAuth1Api')) { if (parentTypes.includes('oAuth1Api')) {
return await requestOAuth1.call(this, credentialsType, requestOptions, true); return await requestOAuth1.call(this, credentialsType, requestOptions, true);
} }
@ -1777,6 +1798,8 @@ export async function requestWithAuthentication(
additionalCredentialOptions?: IAdditionalCredentialOptions, additionalCredentialOptions?: IAdditionalCredentialOptions,
itemIndex?: number, itemIndex?: number,
) { ) {
removeEmptyBody(requestOptions);
let credentialsDecrypted: ICredentialDataDecryptedObject | undefined; let credentialsDecrypted: ICredentialDataDecryptedObject | undefined;
try { try {

View file

@ -4,6 +4,7 @@ import {
parseIncomingMessage, parseIncomingMessage,
parseRequestObject, parseRequestObject,
proxyRequestToAxios, proxyRequestToAxios,
removeEmptyBody,
setBinaryDataBuffer, setBinaryDataBuffer,
} from '@/NodeExecuteFunctions'; } from '@/NodeExecuteFunctions';
import { mkdtempSync, readFileSync } from 'fs'; import { mkdtempSync, readFileSync } from 'fs';
@ -12,7 +13,9 @@ import { mock } from 'jest-mock-extended';
import type { import type {
IBinaryData, IBinaryData,
IHttpRequestMethods, IHttpRequestMethods,
IHttpRequestOptions,
INode, INode,
IRequestOptions,
ITaskDataConnections, ITaskDataConnections,
IWorkflowExecuteAdditionalData, IWorkflowExecuteAdditionalData,
Workflow, Workflow,
@ -458,4 +461,42 @@ describe('NodeExecuteFunctions', () => {
expect(output[0].a === input.a).toEqual(false); 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({});
},
);
});
}); });