Skip to content

Commit

Permalink
More robust infinite recursion handling with custom exception (#7882)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
flibbles authored May 25, 2024
1 parent 074d35c commit e932b09
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 9 deletions.
23 changes: 23 additions & 0 deletions core/modules/utils/errors.js
Original file line number Diff line number Diff line change
@@ -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;

})();
25 changes: 24 additions & 1 deletion core/modules/widgets/transclude.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
};

/*
Expand Down
9 changes: 2 additions & 7 deletions core/modules/widgets/widget.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
3 changes: 2 additions & 1 deletion editions/test/tiddlers/tests/data/transclude/Recursion.tid
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ title: Output

\whitespace trim
<$transclude $tiddler="Output"/>

+
title: ExpectedResult

<p><span class="tc-error">Recursive transclusion error in transclude widget</span></p>
<span class="tc-error">Recursive transclusion error in transclude widget</span>
41 changes: 41 additions & 0 deletions editions/test/tiddlers/tests/test-widget.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,47 @@ describe("Widget module", function() {
expect(wrapper.innerHTML).toBe("<span class=\"tc-error\">Recursive transclusion error in transclude widget</span>");
});

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 /></$tiddler>"},
]);
// 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("<span class=\"tc-error\">Recursive transclusion error in transclude widget</span> <span class=\"tc-error\">Recursive transclusion error in transclude widget</span>");
});

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("<span class=\"tc-error\">Recursive transclusion error in transclude widget</span>");
});

it("should deal with SVG elements", function() {
var wiki = new $tw.Wiki();
// Construct the widget node
Expand Down

0 comments on commit e932b09

Please sign in to comment.