jenkins-bot has submitted this change and it was merged.

Change subject: Improve validation of ScribuntoContent
......................................................................


Improve validation of ScribuntoContent

Implement Content::prepareSave() to ensure that any content
directly passed to WikiPage::doEditContent() that doesn't run edit
filters will still be validated. We have to use prepareSave() instead of
Content::isValid() because validation depends upon the current Title.

Create a ScribuntoContent::validate() convenience function to hold the
logic for that and add a todo to use it in the EditFilterMerged hook.

Also, remove a parser test that depended upon being able to save invalid
modules directly, as what it is testing is no longer possible (unless it
pre-dates making valid syntax a requirement).

Bug: T145548
Change-Id: Ie57eff36100963f02899d669df7375577f7375e1
---
M common/Hooks.php
M common/ScribuntoContent.php
M tests/engines/LuaCommon/luaParserTests.txt
3 files changed, 22 insertions(+), 18 deletions(-)

Approvals:
  Anomie: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/common/Hooks.php b/common/Hooks.php
index 7969a33..b6582a4 100644
--- a/common/Hooks.php
+++ b/common/Hooks.php
@@ -367,6 +367,8 @@
        }
 
        /**
+        * @todo this should use the EditFilterMergedContent hook instead
+        *       so it can use ScribuntoContent::validate()
         * @param EditPage $editor
         * @param string $text
         * @param string $error
diff --git a/common/ScribuntoContent.php b/common/ScribuntoContent.php
index 4a4325d..a00eb7b 100644
--- a/common/ScribuntoContent.php
+++ b/common/ScribuntoContent.php
@@ -19,6 +19,22 @@
        }
 
        /**
+        * Checks whether the script is valid
+        *
+        * @param Title $title
+        * @return Status
+        */
+       public function validate( Title $title ) {
+               $engine = Scribunto::newDefaultEngine();
+               $engine->setTitle( $title );
+               return $engine->validate( $this->getNativeData(), 
$title->getPrefixedDBkey() );
+       }
+
+       public function prepareSave( WikiPage $page, $flags, $parentRevId, User 
$user ) {
+               return $this->validate( $page->getTitle() );
+       }
+
+       /**
         * Parse the Content object and generate a ParserOutput from the result.
         *
         * @param $title Title The page title to use as a context for rendering
@@ -87,9 +103,7 @@
 
                // Validate the script, and include an error message and 
tracking
                // category if it's invalid
-               $engine = Scribunto::newDefaultEngine();
-               $engine->setTitle( $title );
-               $status = $engine->validate( $text, $title->getPrefixedDBkey() 
);
+               $status = $this->validate( $title );
                if ( !$status->isOK() ) {
                        $output->setText( self::getPOText( $output ) .
                                Html::rawElement( 'div', array( 'class' => 
'errorbox' ),
@@ -117,6 +131,9 @@
                        return $output;
                }
 
+               $engine = Scribunto::newDefaultEngine();
+               $engine->setTitle( $title );
+
                // Add HTML for the actual script
                $language = $engine->getGeSHiLanguage();
                if ( $wgScribuntoUseGeSHi && $language ) {
diff --git a/tests/engines/LuaCommon/luaParserTests.txt 
b/tests/engines/LuaCommon/luaParserTests.txt
index a38f938..9e076f1 100644
--- a/tests/engines/LuaCommon/luaParserTests.txt
+++ b/tests/engines/LuaCommon/luaParserTests.txt
@@ -179,12 +179,6 @@
 
 
 !! article
-Module:Parse error
-!! text
-{{{{
-!! endarticle
-
-!! article
 Module:Metatables
 !! text
 local p, mt1, mt2 = {}, {}, {}
@@ -244,15 +238,6 @@
 {{#invoke:test|blah}}
 !! result
 <p><strong class="error"><span class="scribunto-error" 
id="mw-scribunto-error-0">Script error: The function &quot;blah&quot; does not 
exist.</span></strong>
-</p>
-!! end
-
-!! test
-Scribunto: parse error
-!! input
-{{#invoke:Parse error|blah}}
-!! result
-<p><strong class="error"><span class="scribunto-error" 
id="mw-scribunto-error-0">Lua error in Module:Parse_error at line 1: unexpected 
symbol near '{'.</span></strong>
 </p>
 !! end
 

-- 
To view, visit https://gerrit.wikimedia.org/r/310463
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ie57eff36100963f02899d669df7375577f7375e1
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/extensions/Scribunto
Gerrit-Branch: master
Gerrit-Owner: Legoktm <legoktm.wikipe...@gmail.com>
Gerrit-Reviewer: Anomie <bjor...@wikimedia.org>
Gerrit-Reviewer: Brian Wolff <bawolff...@gmail.com>
Gerrit-Reviewer: Jackmcbarn <jackmcb...@gmail.com>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to