Mobrovac has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/259734

Change subject: Make math usable without RESTbase
......................................................................

Make math usable without RESTbase

* Do not contact RESTbase in texvc rendering mode and
  use the well established tex checking that is already build
  into texvc.
* Do not use RESTbase in LaTeXML mode.

Background:
In I1982612e8c6a356e3dbdf447724ac82e5968cc77 RESTbased replaced
texvccheck that was a derivate of texvc designed for people
that wanted to use secure MathML rendering using mathoid.
The integration of mathoid to restbase made this feature obsolete.
However, texvccheck was not only used to check the latex input
that was sent to mathoid, but also the string which was sent to texvc.
Since texvc has already build in tex checking this is not
required and does not improve security.
Finally, users updating from old versions of the math extension
(prior to 2014) that do not have textexvccheck installed,
do not need to compile the texvccheck binary after this change.

PS5: Also treats the case where VisualEditor is not installed.

Bug: T121173
Change-Id: I1bd076b09206869b5ed75280d22e1b36bfb8d8ad

Ask for the MathML and SVG renders only if the input is valid

Bug: T121445
Change-Id: I18b1ef4906f98cea99dca21d5a986a79c02cc233

Minor: rename checkTex() to checkTeX()

Change-Id: I9b1860562b2f4f2112b68c0c3d6f0390f0405fbe

Logging: adjust the log entry levels

Most info-level log entries are actually debug messages, which should
not end up in the production log by default, so adjust them.

Only the Mathoid-powered TeX check fail has been promoted to info so
that we can quickly identify such requests from pages.

Bug: T121445
Change-Id: I3736c59f6425d675befea9438182ee1cdebe5fc5
---
M Math.hooks.php
M MathInputCheckRestbase.php
M MathMathML.php
M MathRenderer.php
M MathRestbaseInterface.php
M MathTexvc.php
M tests/MathRendererTest.php
7 files changed, 74 insertions(+), 34 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Math 
refs/changes/34/259734/1

diff --git a/Math.hooks.php b/Math.hooks.php
index 5b6220f..895e785 100644
--- a/Math.hooks.php
+++ b/Math.hooks.php
@@ -194,7 +194,7 @@
 
                $renderer = MathRenderer::getRenderer( $content, $attributes, 
$mode );
 
-               $checkResult = $renderer->checkTex();
+               $checkResult = $renderer->checkTeX();
 
                if ( $checkResult !== true ) {
                        // Returns the error message
@@ -202,7 +202,7 @@
                }
 
                if ( $renderer->render() ) {
-                       LoggerFactory::getInstance( 'Math' )->info( "Rendering 
successful. Writing output" );
+                       LoggerFactory::getInstance( 'Math' )->debug( "Rendering 
successful. Writing output" );
                        $renderedMath = $renderer->getHtmlOutput();
                } else {
                        LoggerFactory::getInstance( 'Math' )->warning(
diff --git a/MathInputCheckRestbase.php b/MathInputCheckRestbase.php
index bf17b64..0fd2bc5 100644
--- a/MathInputCheckRestbase.php
+++ b/MathInputCheckRestbase.php
@@ -38,6 +38,17 @@
                if ( isset( $e->error->message ) ){
                        if ( $e->error->message === 'Illegal TeX function' ) {
                                return $errorRenderer->getError( 
'math_unknown_function', $e->error->found );
+                       } elseif ( preg_match( '/Math extension/', 
$e->error->message ) ) {
+                               $names = MathHooks::getMathNames();
+                               $mode = $names['mathml'];
+                               try {
+                                       $host = 
$this->restbaseInterface->getUrl( '' );
+                               }
+                               catch ( Exception $ignore ) {
+                                       $host = 'invalid';
+                               }
+                               $msg = $e->error->message;
+                               return $errorRenderer->getError( 
'math_invalidresponse', $mode, $host, $msg );
                        }
                        return $errorRenderer->getError( 'math_syntax_error' );
                }
diff --git a/MathMathML.php b/MathMathML.php
index 1de96be..7f3c185 100644
--- a/MathMathML.php
+++ b/MathMathML.php
@@ -86,7 +86,7 @@
         * @see MathRenderer::render()
        */
        public function render( $forceReRendering = false ) {
-               if ( $this->inputType == 'tex' ) {
+               if ( $this->inputType == 'tex' && $this->mode == 'mathml' ) {
                        $tex = $this->getTex();
                        $displaystyle = false;
                        if ( $this->getMathStyle() == 'inlineDisplaystyle' ) {
@@ -96,10 +96,12 @@
                                $displaystyle = true;
                        }
                        $rbi = new MathRestbaseInterface( $tex, $displaystyle );
-                       $rbi->checkTeX();
-                       $this->mathml = $rbi->getMathML();
-                       $this->svg = $rbi->getSvg();
-                       $this->svgPath = $rbi->getFullSvgUrl();
+                       if ( $rbi->checkTeX() ) {
+                               $this->mathml = $rbi->getMathML();
+                               $this->svg = $rbi->getSvg();
+                               $this->svgPath = $rbi->getFullSvgUrl();
+                       }
+                       $this->changed = false;
                        return $rbi->getSuccess();
                }
                if ( $forceReRendering ) {
diff --git a/MathRenderer.php b/MathRenderer.php
index 1fe7ba7..0b49f12 100644
--- a/MathRenderer.php
+++ b/MathRenderer.php
@@ -165,7 +165,7 @@
                        default:
                                $renderer = new MathMathML( $tex, $params );
                }
-               LoggerFactory::getInstance( 'Math' )->info( 'Start rendering $' 
. $renderer->tex .
+               LoggerFactory::getInstance( 'Math' )->debug( 'Start rendering 
$' . $renderer->tex .
                        '$ in mode ' . $mode );
                return $renderer;
        }
@@ -322,7 +322,7 @@
                # Now save it back to the DB:
                if ( !wfReadOnly() ) {
                        $dbw = $dbw ?: wfGetDB( DB_MASTER );
-                       LoggerFactory::getInstance( 'Math' )->info( 'Store 
entry for $' . $this->tex .
+                       LoggerFactory::getInstance( 'Math' )->debug( 'Store 
entry for $' . $this->tex .
                                '$ in database (hash:' . $this->getMd5() . ')' 
);
                        $outArray = $this->dbOutArray();
                        $method = __METHOD__;
@@ -562,7 +562,7 @@
         * @global $wgMathDisableTexFilter
         * @return bool
         */
-       public function checkTex() {
+       public function checkTeX() {
                if ( $this->texSecure || self::getDisableTexFilter() == 
'always' ) {
                        // equation was already checked or checking is disabled
                        return true;
@@ -573,14 +573,16 @@
                                }
                        }
                        $checker = new MathInputCheckRestbase( 
$this->userInputTex );
-                       if ( $checker->isValid() ) {
-                               $this->setTex( $checker->getValidTex() );
-                               $this->texSecure = true;
-                               return true;
-                       } else {
-                               $this->lastError = $checker->getError();
-                               return false;
+                       try {
+                               if ( $checker->isValid() ) {
+                                       $this->setTex( $checker->getValidTex() 
);
+                                       $this->texSecure = true;
+                                       return true;
+                               }
+                       } catch ( MWException $e ) {
                        }
+                       $this->lastError = $checker->getError();
+                       return false;
                }
        }
 
diff --git a/MathRestbaseInterface.php b/MathRestbaseInterface.php
index 59fc1ec..3ee88db 100644
--- a/MathRestbaseInterface.php
+++ b/MathRestbaseInterface.php
@@ -43,7 +43,7 @@
                $this->calculateHash();
                $request = array(
                        'method' => 'GET',
-                       'url'    => self::getUrl( 
"media/math/render/$type/{$this->hash}" )
+                       'url'    => $this->getUrl( 
"media/math/render/$type/{$this->hash}" )
                );
                $serviceClient = $this->getServiceClient();
                $response = $serviceClient->run( $request );
@@ -83,6 +83,9 @@
                        if ( isset( $json->detail ) && isset( 
$json->detail->success ) ) {
                                $this->success = $json->detail->success;
                                $this->error = $json->detail;
+                       } else {
+                               $this->success = false;
+                               $this->setErrorMessage( 'Math extension cannot 
connect to Restbase.' );
                        }
                        return false;
                }
@@ -104,7 +107,7 @@
                        'body'   => $post
                );
                $serviceClient = $this->getServiceClient();
-               $request['url'] = self::getUrl( 
"media/math/check/{$this->type}" );
+               $request['url'] = $this->getUrl( 
"media/math/check/{$this->type}" );
                $response = $serviceClient->run( $request );
                if ( $response['code'] === 200 ) {
                        $res = $response['body'];
@@ -113,7 +116,7 @@
                        return true;
                } else {
                        $res = $response['body'];
-                       $this->log()->debug( 'Tex check failed:', array(
+                       $this->log()->info( 'Tex check failed:', array(
                                'post'     => $post,
                                'error'    => $response['error'],
                                'url'      => $request['url']
@@ -156,7 +159,7 @@
         * @return string
         * @throws MWException
         */
-       private static function getUrl( $path, $internal = true ) {
+       public function getUrl( $path, $internal = true ) {
                global $wgVirtualRestConfig, $wgMathFullRestbaseURL, 
$wgVisualEditorFullRestbaseURL;
                if ( $internal && isset( 
$wgVirtualRestConfig['modules']['restbase'] ) ) {
                        return "/mathoid/local/v1/$path";
@@ -167,8 +170,9 @@
                if ( $wgVisualEditorFullRestbaseURL ) {
                        return "{$wgVisualEditorFullRestbaseURL}v1/$path";
                }
-               throw new MWException( 'Math extension can not find Restbase 
URL.'.
-                       ' Please specify $wgMathFullRestbaseURL.' );
+               $msg = 'Math extension can not find Restbase URL. Please 
specify $wgMathFullRestbaseURL.';
+               $this->setErrorMessage( $msg );
+               throw new MWException( $msg );
        }
 
        /**
@@ -190,7 +194,7 @@
                try {
                        $request = array(
                                'method' => 'GET',
-                               'url'    => self::getUrl( '?spec' )
+                               'url'    => $this->getUrl( '?spec' )
                        );
                } catch ( Exception $e ) {
                        return false;
@@ -244,7 +248,7 @@
         */
        public function getFullSvgUrl() {
                $this->calculateHash();
-               return self::getUrl( "media/math/render/svg/{$this->hash}", 
false );
+               return $this->getUrl( "media/math/render/svg/{$this->hash}", 
false );
        }
 
        /**
@@ -289,4 +293,11 @@
                return $this->type;
        }
 
+       private function setErrorMessage( $msg ) {
+               $this->error = (object)array(
+                               'error' =>
+                                               (object)array( 'message' => 
$msg )
+               );
+       }
+
 }
diff --git a/MathTexvc.php b/MathTexvc.php
index eec8869..514a512 100644
--- a/MathTexvc.php
+++ b/MathTexvc.php
@@ -46,7 +46,7 @@
                $out['math_html'] = $this->html;
                $out['math_mathml'] = utf8_encode( $this->getMathml() );
                $out['math_inputhash'] = $this->getInputHash();
-               LoggerFactory::getInstance( 'Math' )->info( 'Store Hashpath of 
image' .
+               LoggerFactory::getInstance( 'Math' )->debug( 'Store Hashpath of 
image' .
                        bin2hex( $outmd5_sql ) );
                return $out;
        }
@@ -68,7 +68,7 @@
                        $xhash = unpack( 'H32md5',
                                $dbr->decodeBlob( $rpage->math_outputhash ) . " 
               " );
                        $this->hash = $xhash['md5'];
-                       LoggerFactory::getInstance( 'Math' )->info( 'Hashpath 
of PNG-File:' .
+                       LoggerFactory::getInstance( 'Math' )->debug( 'Hashpath 
of PNG-File:' .
                                bin2hex( $this->hash ) );
                        $this->conservativeness = 
$rpage->math_html_conservativeness;
                        $this->html = $rpage->math_html;
@@ -465,4 +465,18 @@
        public function setOutputHash( $hash ) {
                $this->hash = $hash;
        }
+
+       /**
+        * Skip tex check for texvc rendering mode.
+        * Checking the tex code in texvc mode just adds a dependency to the
+        * texvccheck binary which does not improve security since the same
+        * checks are performed by texvc anyhow. Especially given the fact that
+        * texvccheck was derived from texvc.
+        * @return bool
+        */
+       public function checkTeX() {
+               return true;
+       }
+
+
 }
diff --git a/tests/MathRendererTest.php b/tests/MathRendererTest.php
index 4ab6e68..68f4b5d 100644
--- a/tests/MathRendererTest.php
+++ b/tests/MathRendererTest.php
@@ -96,9 +96,9 @@
                $renderer->expects( $this->never() )->method( 
'readFromDatabase' );
                $renderer->expects( $this->once() )->method( 'setTex' )->with( 
self::TEXVCCHECK_OUTPUT );
 
-               $this->assertEquals( $renderer->checkTex(), true );
+               $this->assertEquals( $renderer->checkTeX(), true );
                // now setTex sould not be called again
-               $this->assertEquals( $renderer->checkTex(), true );
+               $this->assertEquals( $renderer->checkTeX(), true );
 
        }
 
@@ -115,7 +115,7 @@
                $renderer->expects( $this->never() )->method( 
'readFromDatabase' );
                $renderer->expects( $this->never() )->method( 'setTex' );
 
-               $this->assertEquals( $renderer->checkTex(), true );
+               $this->assertEquals( $renderer->checkTeX(), true );
        }
 
        public function testCheckingNewUnknown() {
@@ -132,9 +132,9 @@
                        ->will( $this->returnValue( false ) );
                $renderer->expects( $this->once() )->method( 'setTex' )->with( 
self::TEXVCCHECK_OUTPUT );
 
-               $this->assertEquals( $renderer->checkTex(), true );
+               $this->assertEquals( $renderer->checkTeX(), true );
                // now setTex sould not be called again
-               $this->assertEquals( $renderer->checkTex(), true );
+               $this->assertEquals( $renderer->checkTeX(), true );
        }
 
        public function testCheckingNewKnown() {
@@ -151,9 +151,9 @@
                        ->will( $this->returnValue( true ) );
                $renderer->expects( $this->never() )->method( 'setTex' );
 
-               $this->assertEquals( $renderer->checkTex(), true );
+               $this->assertEquals( $renderer->checkTeX(), true );
                // we don't mark a object as checked even though we rely on the 
database cache
                // so readFromDatabase will be called again
-               $this->assertEquals( $renderer->checkTex(), true );
+               $this->assertEquals( $renderer->checkTeX(), true );
        }
 }

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I3736c59f6425d675befea9438182ee1cdebe5fc5
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Math
Gerrit-Branch: wmf/1.27.0-wmf.8
Gerrit-Owner: Mobrovac <[email protected]>
Gerrit-Reviewer: Physikerwelt <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to