From 1df4c29d73073788ba3859668112e8bb46171a6c Mon Sep 17 00:00:00 2001 From: "jeremy@jermolene.com" Date: Sat, 1 Oct 2022 09:47:26 +0100 Subject: [PATCH 1/4] Relax the restriction on the let widget being unable to create variables starting with a dollar --- core/modules/widgets/let.js | 8 +++----- editions/tw5.com/tiddlers/widgets/LetWidget.tid | 5 ++++- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/core/modules/widgets/let.js b/core/modules/widgets/let.js index 5c4b3aa90..afd3a2f20 100644 --- a/core/modules/widgets/let.js +++ b/core/modules/widgets/let.js @@ -51,11 +51,9 @@ LetWidget.prototype.computeAttributes = function() { $tw.utils.each($tw.utils.getOrderedAttributesFromParseTreeNode(this.parseTreeNode),function(attribute) { var value = self.computeAttribute(attribute), name = attribute.name; - if(name.charAt(0) !== "$") { - // Now that it's prepped, we're allowed to look this variable up - // when defining later variables - self.currentValueFor[name] = value; - } + // Now that it's prepped, we're allowed to look this variable up + // when defining later variables + self.currentValueFor[name] = value; }); // Run through again, setting variables and looking for differences $tw.utils.each(this.currentValueFor,function(value,name) { diff --git a/editions/tw5.com/tiddlers/widgets/LetWidget.tid b/editions/tw5.com/tiddlers/widgets/LetWidget.tid index 2d2b73338..b27306cfb 100644 --- a/editions/tw5.com/tiddlers/widgets/LetWidget.tid +++ b/editions/tw5.com/tiddlers/widgets/LetWidget.tid @@ -1,5 +1,6 @@ title: LetWidget created: 20211028115900000 +modified: 20221001094658854 tags: Widgets caption: let @@ -12,10 +13,12 @@ caption: let The content of the <<.wid let>> widget is the scope for the value assigned to the variable. |!Attribute |!Description | -|//{attributes not starting with $}// |Each attribute name specifies a variable name. The attribute value is assigned to the variable | +|//{attributes}// |Each attribute name specifies a variable name. The attribute value is assigned to the variable | Attributes are evaluated in the order they are written. Attributes with the same name are allowed. Each time a duplicate attribute is encountered, it will replace the existing value set by the earlier duplicate. +<<.note """<<.from-version "5.2.4">> There is no longer any restriction on using variable names that start with the $ character.""">> + ! Examples Consider a case where you need to set multiple variables, where some depend on the evaluation of others. From 8d1e6b5d23bc2342d4de9e6657cc0073e3d01856 Mon Sep 17 00:00:00 2001 From: "jeremy@jermolene.com" Date: Sat, 1 Oct 2022 10:03:50 +0100 Subject: [PATCH 2/4] Test plugin should run tests in shadow tiddlers Missed off #6961, preventing the tests introduced there from running --- plugins/tiddlywiki/jasmine/jasmine-plugin.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/tiddlywiki/jasmine/jasmine-plugin.js b/plugins/tiddlywiki/jasmine/jasmine-plugin.js index f508b8f03..3377ebfbd 100644 --- a/plugins/tiddlywiki/jasmine/jasmine-plugin.js +++ b/plugins/tiddlywiki/jasmine/jasmine-plugin.js @@ -12,7 +12,7 @@ The main module of the Jasmine test plugin for TiddlyWiki5 /*global $tw: true */ "use strict"; -var TEST_TIDDLER_FILTER = "[type[application/javascript]tag[$:/tags/test-spec]]"; +var TEST_TIDDLER_FILTER = "[all[tiddlers+shadows]type[application/javascript]tag[$:/tags/test-spec]]"; exports.name = "jasmine"; // Ensure this startup module is executed in the right order. From db6abb970339f612149cf825a8d9c28acfa69e97 Mon Sep 17 00:00:00 2001 From: Jeremy Ruston Date: Sat, 1 Oct 2022 10:13:40 +0100 Subject: [PATCH 3/4] Improve recursion detection for transclusion and filters (#6970) --- core/modules/filters.js | 21 +++++-- core/modules/widgets/error.js | 63 +++++++++++++++++++ core/modules/widgets/widget.js | 55 +++++++++++----- .../tiddlers/tests/data/filters/Recursion.tid | 15 +++++ .../tests/data/transclude/Recursion.tid | 13 ++++ .../tw5.com/tiddlers/widgets/ErrorWidget.tid | 17 +++++ 6 files changed, 163 insertions(+), 21 deletions(-) create mode 100644 core/modules/widgets/error.js create mode 100644 editions/test/tiddlers/tests/data/filters/Recursion.tid create mode 100644 editions/test/tiddlers/tests/data/transclude/Recursion.tid create mode 100644 editions/tw5.com/tiddlers/widgets/ErrorWidget.tid diff --git a/core/modules/filters.js b/core/modules/filters.js index 221206e6b..1bb5fe9ff 100644 --- a/core/modules/filters.js +++ b/core/modules/filters.js @@ -12,6 +12,9 @@ Adds tiddler filtering methods to the $tw.Wiki object. /*global $tw: false */ "use strict"; +/* Maximum permitted filter recursion depth */ +var MAX_FILTER_DEPTH = 300; + /* Parses an operation (i.e. a run) within a filter string operators: Array of array of operator nodes into which results should be inserted @@ -328,7 +331,7 @@ exports.compileFilter = function(filterString) { })()); }); // Return a function that applies the operations to a source iterator of tiddler titles - var compiled = $tw.perf.measure("filter: " + filterString,function filterFunction(source,widget) { + var fnMeasured = $tw.perf.measure("filter: " + filterString,function filterFunction(source,widget) { if(!source) { source = self.each; } else if(typeof source === "object") { // Array or hashmap @@ -338,9 +341,15 @@ exports.compileFilter = function(filterString) { widget = $tw.rootWidget; } var results = new $tw.utils.LinkedList(); - $tw.utils.each(operationFunctions,function(operationFunction) { - operationFunction(results,source,widget); - }); + self.filterRecursionCount = (self.filterRecursionCount || 0) + 1; + if(self.filterRecursionCount < MAX_FILTER_DEPTH) { + $tw.utils.each(operationFunctions,function(operationFunction) { + operationFunction(results,source,widget); + }); + } else { + results.push("/**-- Excessive filter recursion --**/"); + } + self.filterRecursionCount = self.filterRecursionCount - 1; return results.toArray(); }); if(this.filterCacheCount >= 2000) { @@ -350,9 +359,9 @@ exports.compileFilter = function(filterString) { this.filterCache = Object.create(null); this.filterCacheCount = 0; } - this.filterCache[filterString] = compiled; + this.filterCache[filterString] = fnMeasured; this.filterCacheCount++; - return compiled; + return fnMeasured; }; })(); diff --git a/core/modules/widgets/error.js b/core/modules/widgets/error.js new file mode 100644 index 000000000..6a4a607f1 --- /dev/null +++ b/core/modules/widgets/error.js @@ -0,0 +1,63 @@ +/*\ +title: $:/core/modules/widgets/error.js +type: application/javascript +module-type: widget + +Error widget + +\*/ +(function(){ + +/*jslint node: true, browser: true */ +/*global $tw: false */ +"use strict"; + +var Widget = require("$:/core/modules/widgets/widget.js").widget; + +var ErrorWidget = function(parseTreeNode,options) { + this.initialise(parseTreeNode,options); +}; + +/* +Inherit from the base widget class +*/ +ErrorWidget.prototype = new Widget(); + +/* +Render this widget into the DOM +*/ +ErrorWidget.prototype.render = function(parent,nextSibling) { + this.parentDomNode = parent; + this.computeAttributes(); + this.execute(); + var message = this.getAttribute("$message","Unknown error"), + domNode = this.document.createElement("span"); + domNode.appendChild(this.document.createTextNode(message)); + domNode.className = "tc-error"; + parent.insertBefore(domNode,nextSibling); + this.domNodes.push(domNode); +}; + +/* +Compute the internal state of the widget +*/ +ErrorWidget.prototype.execute = function() { + // Nothing to do for a text node +}; + +/* +Selectively refreshes the widget if needed. Returns true if the widget or any of its children needed re-rendering +*/ +ErrorWidget.prototype.refresh = function(changedTiddlers) { + var changedAttributes = this.computeAttributes(); + if(changedAttributes["$message"]) { + this.refreshSelf(); + return true; + } else { + return false; + } +}; + +exports.error = ErrorWidget; + +})(); diff --git a/core/modules/widgets/widget.js b/core/modules/widgets/widget.js index 7034c9f37..60f55e8bb 100755 --- a/core/modules/widgets/widget.js +++ b/core/modules/widgets/widget.js @@ -12,6 +12,9 @@ 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 @@ -358,6 +361,20 @@ Widget.prototype.assignAttributes = function(domNode,options) { } }; +/* +Get the number of ancestor widgets for this widget +*/ +Widget.prototype.getAncestorCount = function() { + if(this.ancestorCount === undefined) { + if(this.parentWidget) { + this.ancestorCount = this.parentWidget.getAncestorCount() + 1; + } else { + this.ancestorCount = 0; + } + } + return this.ancestorCount; +}; + /* Make child widgets correspondng to specified parseTreeNodes */ @@ -365,21 +382,29 @@ Widget.prototype.makeChildWidgets = function(parseTreeNodes,options) { options = options || {}; this.children = []; var self = this; - // Create set variable widgets for each variable - $tw.utils.each(options.variables,function(value,name) { - var setVariableWidget = { - type: "set", - attributes: { - name: {type: "string", value: name}, - value: {type: "string", value: value} - }, - children: parseTreeNodes - }; - parseTreeNodes = [setVariableWidget]; - }); - $tw.utils.each(parseTreeNodes || (this.parseTreeNode && this.parseTreeNode.children),function(childNode) { - self.children.push(self.makeChildWidget(childNode)); - }); + // 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")} + }})); + } else { + // Create set variable widgets for each variable + $tw.utils.each(options.variables,function(value,name) { + var setVariableWidget = { + type: "set", + attributes: { + name: {type: "string", value: name}, + value: {type: "string", value: value} + }, + children: parseTreeNodes + }; + parseTreeNodes = [setVariableWidget]; + }); + // Create the child widgets + $tw.utils.each(parseTreeNodes || (this.parseTreeNode && this.parseTreeNode.children),function(childNode) { + self.children.push(self.makeChildWidget(childNode)); + }); + } }; /* diff --git a/editions/test/tiddlers/tests/data/filters/Recursion.tid b/editions/test/tiddlers/tests/data/filters/Recursion.tid new file mode 100644 index 000000000..13d80e521 --- /dev/null +++ b/editions/test/tiddlers/tests/data/filters/Recursion.tid @@ -0,0 +1,15 @@ +title: Filters/Recursion +description: Filter recursion detection +type: text/vnd.tiddlywiki-multiple +tags: [[$:/tags/wiki-test-spec]] + +title: Output + +\whitespace trim +\define myfilter() [subfilter] + +<$text text={{{ [subfilter] }}}/> ++ +title: ExpectedResult + +

/**-- Excessive filter recursion --**/

\ No newline at end of file diff --git a/editions/test/tiddlers/tests/data/transclude/Recursion.tid b/editions/test/tiddlers/tests/data/transclude/Recursion.tid new file mode 100644 index 000000000..d75e671eb --- /dev/null +++ b/editions/test/tiddlers/tests/data/transclude/Recursion.tid @@ -0,0 +1,13 @@ +title: Transclude/Recursion +description: Transclusion recursion detection +type: text/vnd.tiddlywiki-multiple +tags: [[$:/tags/wiki-test-spec]] + +title: Output + +\whitespace trim +<$transclude $tiddler="Output"/> ++ +title: ExpectedResult + +

Recursive transclusion error in transclude widget

\ No newline at end of file diff --git a/editions/tw5.com/tiddlers/widgets/ErrorWidget.tid b/editions/tw5.com/tiddlers/widgets/ErrorWidget.tid new file mode 100644 index 000000000..392afea40 --- /dev/null +++ b/editions/tw5.com/tiddlers/widgets/ErrorWidget.tid @@ -0,0 +1,17 @@ +caption: error +created: 20220909111836951 +modified: 20220909111836951 +tags: Widgets +title: ErrorWidget +type: text/vnd.tiddlywiki + +<<.from-version "5.3.0">> The <<.wlink ErrorWidget>> widget is used by the core to display error messages such as the recursion errors reported by the <<.wlink TranscludeWidget>> widget. + +The <<.wlink ErrorWidget>> does not provide any useful functionality to end users. It is only required by the core for technical reasons. + +! Content and Attributes + +The content of the <<.wlink ErrorWidget>> widget is ignored. + +|!Attribute |!Description | +|$message |The error message | From 47f80339b2de7070cff1422a5a608bfc4310b826 Mon Sep 17 00:00:00 2001 From: "jeremy@jermolene.com" Date: Sat, 1 Oct 2022 10:15:14 +0100 Subject: [PATCH 4/4] Update transclude widget to use error widget Missed off #6970 --- core/modules/widgets/transclude.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/core/modules/widgets/transclude.js b/core/modules/widgets/transclude.js index 18645cd27..ff30c841d 100755 --- a/core/modules/widgets/transclude.js +++ b/core/modules/widgets/transclude.js @@ -70,11 +70,9 @@ TranscludeWidget.prototype.execute = function() { // Check for recursion if(parser) { if(this.parentWidget && this.parentWidget.hasVariable("transclusion",recursionMarker)) { - parseTreeNodes = [{type: "element", tag: "span", attributes: { - "class": {type: "string", value: "tc-error"} - }, children: [ - {type: "text", text: $tw.language.getString("Error/RecursiveTransclusion")} - ]}]; + parseTreeNodes = [{type: "error", attributes: { + "$message": {type: "string", value: $tw.language.getString("Error/RecursiveTransclusion")} + }]; } } // Construct the child widgets