From e932b09016e8321083a7d685e8650282180ae4f5 Mon Sep 17 00:00:00 2001 From: Cameron Fischer Date: Sat, 25 May 2024 05:56:19 -0400 Subject: [PATCH] More robust infinite recursion handling with custom exception (#7882) * Introduced preliminary idea for infinite recurse exception * Better handling of infinite recursion But it could be better still... * the TransclusionError is a proper error Moved the magic number to be on the error's class. Not sure if that's a great idea. * Fixed minor minor issue that came up in conflict The minor fix to the jasmine regexp that escaped a '+' somehow broke some random test. --- core/modules/utils/errors.js | 23 +++++++++++ core/modules/widgets/transclude.js | 25 ++++++++++- core/modules/widgets/widget.js | 9 +--- .../tests/data/transclude/Recursion.tid | 3 +- editions/test/tiddlers/tests/test-widget.js | 41 +++++++++++++++++++ 5 files changed, 92 insertions(+), 9 deletions(-) create mode 100644 core/modules/utils/errors.js diff --git a/core/modules/utils/errors.js b/core/modules/utils/errors.js new file mode 100644 index 000000000..fac4b3fa7 --- /dev/null +++ b/core/modules/utils/errors.js @@ -0,0 +1,23 @@ +/*\ +title: $:/core/modules/utils/errors.js +type: application/javascript +module-type: utils + +Custom errors for TiddlyWiki. + +\*/ +(function(){ + +function TranscludeRecursionError() { + Error.apply(this,arguments); + this.signatures = Object.create(null); +}; + +/* Maximum permitted depth of the widget tree for recursion detection */ +TranscludeRecursionError.MAX_WIDGET_TREE_DEPTH = 1000; + +TranscludeRecursionError.prototype = Object.create(Error); + +exports.TranscludeRecursionError = TranscludeRecursionError; + +})(); diff --git a/core/modules/widgets/transclude.js b/core/modules/widgets/transclude.js index d30ab1fa7..35b4941bd 100755 --- a/core/modules/widgets/transclude.js +++ b/core/modules/widgets/transclude.js @@ -30,7 +30,30 @@ TranscludeWidget.prototype.render = function(parent,nextSibling) { this.parentDomNode = parent; this.computeAttributes(); this.execute(); - this.renderChildren(parent,nextSibling); + try { + this.renderChildren(parent,nextSibling); + } catch(error) { + if(error instanceof $tw.utils.TranscludeRecursionError) { + // We were infinite looping. + // We need to try and abort as much of the loop as we can, so we will keep "throwing" upward until we find a transclusion that has a different signature. + // Hopefully that will land us just outside where the loop began. That's where we want to issue an error. + // Rendering widgets beneath this point may result in a freezing browser if they explode exponentially. + var transcludeSignature = this.getVariable("transclusion"); + if(this.getAncestorCount() > $tw.utils.TranscludeRecursionError.MAX_WIDGET_TREE_DEPTH - 50) { + // For the first fifty transcludes we climb up, we simply collect signatures. + // We're assuming that those first 50 will likely include all transcludes involved in the loop. + error.signatures[transcludeSignature] = true; + } else if(!error.signatures[transcludeSignature]) { + // Now that we're past the first 50, let's look for the first signature that wasn't in the loop. That'll be where we print the error and resume rendering. + this.children = [this.makeChildWidget({type: "error", attributes: { + "$message": {type: "string", value: $tw.language.getString("Error/RecursiveTransclusion")} + }})]; + this.renderChildren(parent,nextSibling); + return; + } + } + throw error; + } }; /* diff --git a/core/modules/widgets/widget.js b/core/modules/widgets/widget.js index 69f63a684..cb8e5e881 100755 --- a/core/modules/widgets/widget.js +++ b/core/modules/widgets/widget.js @@ -12,9 +12,6 @@ Widget base class /*global $tw: false */ "use strict"; -/* Maximum permitted depth of the widget tree for recursion detection */ -var MAX_WIDGET_TREE_DEPTH = 1000; - /* Create a widget object for a parse tree node parseTreeNode: reference to the parse tree node to be rendered @@ -494,10 +491,8 @@ Widget.prototype.makeChildWidgets = function(parseTreeNodes,options) { this.children = []; var self = this; // Check for too much recursion - if(this.getAncestorCount() > MAX_WIDGET_TREE_DEPTH) { - this.children.push(this.makeChildWidget({type: "error", attributes: { - "$message": {type: "string", value: $tw.language.getString("Error/RecursiveTransclusion")} - }})); + if(this.getAncestorCount() > $tw.utils.TranscludeRecursionError.MAX_WIDGET_TREE_DEPTH) { + throw new $tw.utils.TranscludeRecursionError(); } else { // Create set variable widgets for each variable $tw.utils.each(options.variables,function(value,name) { diff --git a/editions/test/tiddlers/tests/data/transclude/Recursion.tid b/editions/test/tiddlers/tests/data/transclude/Recursion.tid index d75e671eb..b834f3765 100644 --- a/editions/test/tiddlers/tests/data/transclude/Recursion.tid +++ b/editions/test/tiddlers/tests/data/transclude/Recursion.tid @@ -7,7 +7,8 @@ title: Output \whitespace trim <$transclude $tiddler="Output"/> + + title: ExpectedResult -

Recursive transclusion error in transclude widget

\ No newline at end of file +Recursive transclusion error in transclude widget \ No newline at end of file diff --git a/editions/test/tiddlers/tests/test-widget.js b/editions/test/tiddlers/tests/test-widget.js index 0d1351f31..1c7665a53 100755 --- a/editions/test/tiddlers/tests/test-widget.js +++ b/editions/test/tiddlers/tests/test-widget.js @@ -160,6 +160,47 @@ describe("Widget module", function() { expect(wrapper.innerHTML).toBe("Recursive transclusion error in transclude widget"); }); + it("should handle single-tiddler recursion with branching nodes", function() { + var wiki = new $tw.Wiki(); + // Add a tiddler + wiki.addTiddlers([ + {title: "TiddlerOne", text: "<$tiddler tiddler='TiddlerOne'><$transclude /> <$transclude />"}, + ]); + // Test parse tree + var parseTreeNode = {type: "widget", children: [ + {type: "transclude", attributes: { + "tiddler": {type: "string", value: "TiddlerOne"} + }} + ]}; + // Construct the widget node + var widgetNode = createWidgetNode(parseTreeNode,wiki); + // Render the widget node to the DOM + var wrapper = renderWidgetNode(widgetNode); + // Test the rendering + expect(wrapper.innerHTML).toBe("Recursive transclusion error in transclude widget Recursive transclusion error in transclude widget"); + }); + + it("should handle many-tiddler recursion with branching nodes", function() { + var wiki = new $tw.Wiki(); + // Add a tiddler + wiki.addTiddlers([ + {title: "TiddlerOne", text: "<$transclude tiddler='TiddlerTwo'/> <$transclude tiddler='TiddlerTwo'/>"}, + {title: "TiddlerTwo", text: "<$transclude tiddler='TiddlerOne'/>"} + ]); + // Test parse tree + var parseTreeNode = {type: "widget", children: [ + {type: "transclude", attributes: { + "tiddler": {type: "string", value: "TiddlerOne"} + }} + ]}; + // Construct the widget node + var widgetNode = createWidgetNode(parseTreeNode,wiki); + // Render the widget node to the DOM + var wrapper = renderWidgetNode(widgetNode); + // Test the rendering + expect(wrapper.innerHTML).toBe("Recursive transclusion error in transclude widget"); + }); + it("should deal with SVG elements", function() { var wiki = new $tw.Wiki(); // Construct the widget node