From b6d8a0f98526bfc98a3d9a722dafce4a53e715ec Mon Sep 17 00:00:00 2001 From: Michael Auerswald Date: Tue, 21 Mar 2023 15:34:30 +0100 Subject: [PATCH] fix(core): Remove circular refs from Code and push msg (#5741) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * remove circular refs from code items (and lint fixes) * cleanup --------- * add some tests Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ --- packages/cli/src/push/abstract.push.ts | 4 +-- .../src/sso/saml/routes/saml.controller.ee.ts | 1 - packages/nodes-base/nodes/Code/utils.ts | 27 ++++++++++++++----- packages/workflow/src/index.ts | 2 +- packages/workflow/src/utils.ts | 25 +++++++++++++++++ packages/workflow/test/utils.test.ts | 17 +++++++++++- 6 files changed, 64 insertions(+), 12 deletions(-) diff --git a/packages/cli/src/push/abstract.push.ts b/packages/cli/src/push/abstract.push.ts index 633cb126a8..a9022bdc5c 100644 --- a/packages/cli/src/push/abstract.push.ts +++ b/packages/cli/src/push/abstract.push.ts @@ -1,4 +1,4 @@ -import { LoggerProxy as Logger } from 'n8n-workflow'; +import { jsonStringify, LoggerProxy as Logger } from 'n8n-workflow'; import type { IPushDataType } from '@/Interfaces'; import { eventBus } from '../eventbus'; @@ -38,7 +38,7 @@ export abstract class AbstractPush { Logger.debug(`Send data of type "${type}" to editor-UI`, { dataType: type, sessionId }); - const sendData = JSON.stringify({ type, data }); + const sendData = jsonStringify({ type, data }, { replaceCircularRefs: true }); if (sessionId === undefined) { // Send to all connected clients diff --git a/packages/cli/src/sso/saml/routes/saml.controller.ee.ts b/packages/cli/src/sso/saml/routes/saml.controller.ee.ts index b88ac17b26..7e6d026f0c 100644 --- a/packages/cli/src/sso/saml/routes/saml.controller.ee.ts +++ b/packages/cli/src/sso/saml/routes/saml.controller.ee.ts @@ -135,7 +135,6 @@ export class SamlController { private async handleInitSSO(res: express.Response) { const result = this.samlService.getLoginRequestUrl(); if (result?.binding === 'redirect') { - // Return the redirect URL directly return res.send(result.context.context); } else if (result?.binding === 'post') { return res.send(getInitSSOFormView(result.context as PostBindingContext)); diff --git a/packages/nodes-base/nodes/Code/utils.ts b/packages/nodes-base/nodes/Code/utils.ts index e68f3cd27c..d4adfde155 100644 --- a/packages/nodes-base/nodes/Code/utils.ts +++ b/packages/nodes-base/nodes/Code/utils.ts @@ -12,15 +12,28 @@ function isTraversable(maybe: unknown): maybe is IDataObject { * Stringify any non-standard JS objects (e.g. `Date`, `RegExp`) inside output items at any depth. */ export function standardizeOutput(output: IDataObject) { - for (const [key, value] of Object.entries(output)) { - if (!isTraversable(value)) continue; + const knownObjects = new WeakSet(); - output[key] = - value.constructor.name !== 'Object' - ? JSON.stringify(value) // Date, RegExp, etc. - : standardizeOutput(value); + function standardizeOutputRecursive(obj: IDataObject): IDataObject { + for (const [key, value] of Object.entries(obj)) { + if (!isTraversable(value)) continue; + + if (typeof value === 'object' && value !== null) { + if (knownObjects.has(value)) { + // Found circular reference + continue; + } + knownObjects.add(value); + } + + obj[key] = + value.constructor.name !== 'Object' + ? JSON.stringify(value) // Date, RegExp, etc. + : standardizeOutputRecursive(value); + } + return obj; } - + standardizeOutputRecursive(output); return output; } diff --git a/packages/workflow/src/index.ts b/packages/workflow/src/index.ts index 117440e417..63e71425fa 100644 --- a/packages/workflow/src/index.ts +++ b/packages/workflow/src/index.ts @@ -23,7 +23,7 @@ export * from './WorkflowErrors'; export * from './WorkflowHooks'; export * from './VersionedNodeType'; export { LoggerProxy, NodeHelpers, ObservableObject, TelemetryHelpers }; -export { deepCopy, jsonParse, sleep, fileTypeFromMimeType, assert } from './utils'; +export { deepCopy, jsonParse, jsonStringify, sleep, fileTypeFromMimeType, assert } from './utils'; export { isINodeProperties, isINodePropertyOptions, diff --git a/packages/workflow/src/utils.ts b/packages/workflow/src/utils.ts index 9bb0444339..6f7a9fdad1 100644 --- a/packages/workflow/src/utils.ts +++ b/packages/workflow/src/utils.ts @@ -62,6 +62,31 @@ export const jsonParse = (jsonString: string, options?: JSONParseOptions): } }; +type JSONStringifyOptions = { + replaceCircularRefs?: boolean; + circularRefReplacement?: string; +}; + +const getReplaceCircularReferencesFn = (options: JSONStringifyOptions) => { + const knownObjects = new WeakSet(); + return (key: any, value: any) => { + if (typeof value === 'object' && value !== null) { + if (knownObjects.has(value)) { + return options?.circularRefReplacement ?? '[Circular Reference]'; + } + knownObjects.add(value); + } + return value; + }; +}; + +export const jsonStringify = (obj: unknown, options: JSONStringifyOptions = {}): string => { + const replacer = options?.replaceCircularRefs + ? getReplaceCircularReferencesFn(options) + : undefined; + return JSON.stringify(obj, replacer); +}; + export const sleep = async (ms: number): Promise => new Promise((resolve) => { setTimeout(resolve, ms); diff --git a/packages/workflow/test/utils.test.ts b/packages/workflow/test/utils.test.ts index 04cfe0da58..0cc34f4ef4 100644 --- a/packages/workflow/test/utils.test.ts +++ b/packages/workflow/test/utils.test.ts @@ -1,4 +1,4 @@ -import { jsonParse, deepCopy } from '@/utils'; +import { jsonParse, jsonStringify, deepCopy } from '@/utils'; describe('jsonParse', () => { it('parses JSON', () => { @@ -17,6 +17,21 @@ describe('jsonParse', () => { }); }); +describe('jsonStringify', () => { + const source: any = { a: 1, b: 2 }; + source.c = source; + + it('should throw errors on circular references by default', () => { + expect(() => jsonStringify(source)).toThrow('Converting circular structure to JSON'); + }); + + it('should break circular references when requested', () => { + expect(jsonStringify(source, { replaceCircularRefs: true })).toEqual( + '{"a":1,"b":2,"c":"[Circular Reference]"}', + ); + }); +}); + describe('deepCopy', () => { it('should deep copy an object', () => { const serializable = {