Skip to content

Commit

Permalink
fix(core): Remove circular refs from Code and push msg (#5741)
Browse files Browse the repository at this point in the history
* remove circular refs from code items (and lint fixes)

* cleanup

---------

* add some tests

Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <aditya@netroy.in>
  • Loading branch information
flipswitchingmonkey and netroy committed Mar 21, 2023
1 parent 199a91b commit b6d8a0f
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 12 deletions.
4 changes: 2 additions & 2 deletions 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';

Expand Down Expand Up @@ -38,7 +38,7 @@ export abstract class AbstractPush<T> {

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
Expand Down
1 change: 0 additions & 1 deletion packages/cli/src/sso/saml/routes/saml.controller.ee.ts
Expand Up @@ -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));
Expand Down
27 changes: 20 additions & 7 deletions packages/nodes-base/nodes/Code/utils.ts
Expand Up @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/workflow/src/index.ts
Expand Up @@ -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,
Expand Down
25 changes: 25 additions & 0 deletions packages/workflow/src/utils.ts
Expand Up @@ -62,6 +62,31 @@ export const jsonParse = <T>(jsonString: string, options?: JSONParseOptions<T>):
}
};

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<void> =>
new Promise((resolve) => {
setTimeout(resolve, ms);
Expand Down
17 changes: 16 additions & 1 deletion 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', () => {
Expand All @@ -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 = {
Expand Down

0 comments on commit b6d8a0f

Please sign in to comment.