* Fixed template loop check, broken by changes in parse order

* Added template recursion depth limit. It needs a small limit, because of exorbitant stack space usage, xdebug compatibility problems, and the potential for O(N^2) memory usage in the template loop check.
* Made these two error messages more obvious in the parser output, with <span class="error"> instead of a comment. This is similar to the #expr error messages, which seem to have been well received by our template programmer community.

diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index 048eba3..f620f90 100644 (file)
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -891,6 +891,14 @@ $wgMaxArticleSize  = 2048; # Maximum article size in kilobytes
 
 $wgMaxPPNodeCount = 1000000;  # A complexity limit on template expansion
 
+/**
+ * Maximum recursion depth for templates within templates.
+ * The current parser adds two levels to the PHP call stack for each template, 
+ * and xdebug limits the call stack to 100 by default. So this should hopefully
+ * stop the parser before it hits the xdebug limit.
+ */
+$wgMaxTemplateDepth = 40;
+
 $wgExtraSubtitle       = '';
 $wgSiteSupportPage     = ''; # A page where you users can receive donations
 
@@ -2863,4 +2871,4 @@ $wgParserConf = array(
  *     $wgExceptionHooks[] = array( $class, $funcname )
  * Hooks should return strings or false
  */
-$wgExceptionHooks = array();
\ No newline at end of file
+$wgExceptionHooks = array();

diff --git a/includes/Parser.php b/includes/Parser.php
index 74b58ef..c5d98f6 100644 (file)
--- a/includes/Parser.php
+++ b/includes/Parser.php
@@ -87,9 +87,7 @@ class Parser
        var $mIncludeCount, $mArgStack, $mLastSection, $mInPre;
        var $mInterwikiLinkHolders, $mLinkHolders;
        var $mIncludeSizes, $mPPNodeCount, $mDefaultSort;
-       var $mTplExpandCache,// empty-frame expansion cache
-           $mTemplatePath;     // stores an unsorted hash of all the templates already loaded
-                               // in this path. Used for loop detection.
+       var $mTplExpandCache; // empty-frame expansion cache
        var $mTplRedirCache, $mTplDomCache, $mHeadings;
 
        # Temporary
@@ -225,7 +223,6 @@ class Parser
                $this->mUniqPrefix = "\x7fUNIQ" . Parser::getRandomString();
 
                # Clear these on every parse, bug 4549
-               $this->mTemplatePath = array();
                $this->mTplExpandCache = $this->mTplRedirCache = $this->mTplDomCache = array();
 
                $this->mShowToc = true;
@@ -3277,9 +3274,6 @@ class Parser
                }
                wfProfileOut( __METHOD__.'-modifiers' );
 
-               # Save path level before recursing into functions & templates.
-               $lastPathLevel = $this->mTemplatePath;
-
                # Parser functions
                if ( !$found ) {
                        wfProfileIn( __METHOD__ . '-pfunc' );
@@ -3357,11 +3351,17 @@ class Parser
                                        $wgContLang->findVariantLink($part1, $title);
                                }
                                # Do infinite loop check
-                               if ( isset( $this->mTemplatePath[$titleText] ) ) {
+                               if ( isset( $frame->loopCheckHash[$titleText] ) ) {
                                        $found = true;
-                                       $text = "[[$part1]]" . $this->insertStripItem( '<!-- WARNING: template loop detected -->' );
+                                       $text = "<span class=\"error\">Template loop detected: [[$titleText]]</span>";
                                        wfDebug( __METHOD__.": template loop broken at '$titleText'\n" );
                                }
+                               # Do recursion depth check
+                               $limit = $this->mOptions->getMaxTemplateDepth();
+                               if ( $frame->depth >= $limit ) {
+                                       $found = true;
+                                       $text = "<span class=\"error\">Template recursion depth limit exceeded ($limit)</span>";
+                               }
                        }
                }
 
@@ -3412,8 +3412,6 @@ class Parser
                # Recover the source wikitext and return it
                if ( !$found ) {
                        $text = '{{' . $frame->implode( '|', $titleWithSpaces, $args ) . '}}';
-                       # Prune lower levels off the recursion check path
-                       $this->mTemplatePath = $lastPathLevel;
                        wfProfileOut( $fname );
                        return $text;
                }
@@ -3422,8 +3420,8 @@ class Parser
                if ( $isDOM ) {
                        # Clean up argument array
                        $newFrame = $frame->newChild( $args, $title );
-                       # Add a new element to the templace recursion path
-                       $this->mTemplatePath[$titleText] = 1;
+                       # Add a new element to the templace loop detection hashtable
+                       $newFrame->loopCheckHash[$titleText] = true;
 
                        if ( $titleText !== false && count( $newFrame->args ) == 0 ) {
                                # Expansion is eligible for the empty-frame cache
@@ -3434,6 +3432,7 @@ class Parser
                                        $this->mTplExpandCache[$titleText] = $text;
                                }
                        } else {
+                               # Uncached expansion
                                $text = $newFrame->expand( $text );
                        }
                }
@@ -3455,9 +3454,6 @@ class Parser
                        $text = "\n" . $text;
                }
                
-               # Prune lower levels off the recursion check path
-               $this->mTemplatePath = $lastPathLevel;
-
                if ( !$this->incrementIncludeSize( 'post-expand', strlen( $text ) ) ) {
                        # Error, oversize inclusion
                        $text = "[[$originalTitle]]" . 
@@ -4356,8 +4352,6 @@ class Parser
         *   found                     The text returned is valid, stop processing the template. This
         *                             is on by default.
         *   nowiki                    Wiki markup in the return value should be escaped
-        *   noparse                   Unsafe HTML tags should not be stripped, etc.
-        *   noargs                    Don't replace triple-brace arguments in the return value
         *   isHTML                    The returned text is HTML, armour it against wikitext transformation
         *
         * @public
@@ -5327,6 +5321,17 @@ class PPFrame {
        var $parser, $title;
        var $titleCache;
 
+       /**
+        * Hashtable listing templates which are disallowed for expansion in this frame,
+        * having been encountered previously in parent frames.
+        */
+       var $loopCheckHash;
+
+       /**
+        * Recursion depth of this frame, top = 0
+        */
+       var $depth;
+
        const NO_ARGS = 1;
        const NO_TEMPLATES = 2;
        const STRIP_COMMENTS = 4;
@@ -5343,6 +5348,8 @@ class PPFrame {
                $this->parser = $parser;
                $this->title = $parser->mTitle;
                $this->titleCache = array( $this->title ? $this->title->getPrefixedDBkey() : false );
+               $this->loopCheckHash = array();
+               $this->depth = 0;
        }
 
        /**
@@ -5586,8 +5593,7 @@ class PPFrame {
  * Expansion frame with template arguments
  */
 class PPTemplateFrame extends PPFrame {
-       var $parser, $args, $parent;
-       var $titleCache;
+       var $args, $parent;
 
        function __construct( $parser, $parent = false, $args = array(), $title = false ) {
                $this->parser = $parser;
@@ -5596,6 +5602,8 @@ class PPTemplateFrame extends PPFrame {
                $this->title = $title;
                $this->titleCache = $parent->titleCache;
                $this->titleCache[] = $title ? $title->getPrefixedDBkey() : false;
+               $this->loopCheckHash = /*clone*/ $parent->loopCheckHash;
+               $this->depth = $parent->depth + 1;
        }
 
        function __toString() {

diff --git a/includes/ParserOptions.php b/includes/ParserOptions.php
index a499ee2..5bab3eb 100644 (file)
--- a/includes/ParserOptions.php
+++ b/includes/ParserOptions.php
@@ -22,6 +22,7 @@ class ParserOptions
        var $mInterfaceMessage;          # Which lang to call for PLURAL and GRAMMAR
        var $mMaxIncludeSize;            # Maximum size of template expansions, in bytes
        var $mMaxPPNodeCount;            # Maximum number of nodes touched by PPFrame::expand()
+       var $mMaxTemplateDepth;          # Maximum recursion depth for templates within templates
        var $mRemoveComments;            # Remove HTML comments. ONLY APPLIES TO PREPROCESS OPERATIONS
        var $mTemplateCallback;          # Callback for template fetching
        var $mEnableLimitReport;         # Enable limit report in an HTML comment on output
@@ -40,6 +41,7 @@ class ParserOptions
        function getInterfaceMessage()              { return $this->mInterfaceMessage; }
        function getMaxIncludeSize()                { return $this->mMaxIncludeSize; }
        function getMaxPPNodeCount()                { return $this->mMaxPPNodeCount; }
+       function getMaxTemplateDepth()              { return $this->mMaxTemplateDepth; }
        function getRemoveComments()                { return $this->mRemoveComments; }
        function getTemplateCallback()              { return $this->mTemplateCallback; }
        function getEnableLimitReport()             { return $this->mEnableLimitReport; }
@@ -72,6 +74,7 @@ class ParserOptions
        function setInterfaceMessage( $x )          { return wfSetVar( $this->mInterfaceMessage, $x); }
        function setMaxIncludeSize( $x )            { return wfSetVar( $this->mMaxIncludeSize, $x ); }
        function setMaxPPNodeCount( $x )            { return wfSetVar( $this->mMaxPPNodeCount, $x ); }
+       function setMaxTemplateDepth( $x )          { return wfSetVar( $this->mMaxTemplateDepth, $x ); }
        function setRemoveComments( $x )            { return wfSetVar( $this->mRemoveComments, $x ); }
        function setTemplateCallback( $x )          { return wfSetVar( $this->mTemplateCallback, $x ); }
        function enableLimitReport( $x = true )     { return wfSetVar( $this->mEnableLimitReport, $x ); }
@@ -92,7 +95,7 @@ class ParserOptions
        function initialiseFromUser( $userInput ) {
                global $wgUseTeX, $wgUseDynamicDates, $wgInterwikiMagic, $wgAllowExternalImages;
                global $wgAllowExternalImagesFrom, $wgAllowSpecialInclusion, $wgMaxArticleSize;
-               global $wgMaxPPNodeCount;
+               global $wgMaxPPNodeCount, $wgMaxTemplateDepth;
                $fname = 'ParserOptions::initialiseFromUser';
                wfProfileIn( $fname );
                if ( !$userInput ) {
@@ -122,6 +125,7 @@ class ParserOptions
                $this->mInterfaceMessage = false;
                $this->mMaxIncludeSize = $wgMaxArticleSize * 1024;
                $this->mMaxPPNodeCount = $wgMaxPPNodeCount;
+               $this->mMaxTemplateDepth = $wgMaxTemplateDepth;
                $this->mRemoveComments = true;
                $this->mTemplateCallback = array( 'Parser', 'statelessFetchTemplate' );
                $this->mEnableLimitReport = false;