Anomie has uploaded a new change for review.

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


Change subject: Improve disable_functions handling in LuaStandalone
......................................................................

Improve disable_functions handling in LuaStandalone

If the user is on a webhost that has proc_open listed in PHP's
disable_functions directive, we should give a better error message.
Until we no longer support PHP below 5.4, we should do the same for
safe_mode. And since we're doing that, we may as well report any other
warnings if proc_open fails, too.

In addition, this cleans up error handling in
Scribunto_LuaEngine::load() so it doesn't pretend the interpreter is
loaded if getInterpreter() throws an exception. Otherwise other code
winds up with PHP fatal errors trying to access a null value.

Bug: 50706
Change-Id: I2887b722e089fd7a526aa7dcab9c80deb343d8ac
---
M Scribunto.i18n.php
M engines/LuaCommon/LuaCommon.php
M engines/LuaStandalone/LuaStandaloneEngine.php
3 files changed, 56 insertions(+), 23 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Scribunto 
refs/changes/89/72089/1

diff --git a/Scribunto.i18n.php b/Scribunto.i18n.php
index 3735b71..91eeb68 100644
--- a/Scribunto.i18n.php
+++ b/Scribunto.i18n.php
@@ -55,6 +55,9 @@
        'scribunto-lua-noreturn' => 'Script error: The module did not return a 
value. It is supposed to return an export table.',
        'scribunto-lua-notarrayreturn' => 'Script error: The module returned 
something other than a table. It is supposed to return an export table.',
        'scribunto-luastandalone-proc-error' => 'Lua error: Cannot create 
process.',
+       'scribunto-luastandalone-proc-error-msg' => 'Lua error: Cannot create 
process: $2',
+       'scribunto-luastandalone-proc-error-proc-open' => 'Lua error: Cannot 
create process: proc_open is not available. Check PHP\'s "disable_functions" 
configuration directive.',
+       'scribunto-luastandalone-proc-error-safe-mode' => 'Lua error: Cannot 
create process. Note that PHP\'s deprecated "safe_mode" configuration directive 
is enabled.',
        'scribunto-luastandalone-decode-error' => 'Lua error: Internal error: 
Unable to decode message.',
        'scribunto-luastandalone-write-error' => 'Lua error: Internal error: 
Error writing to pipe.',
        'scribunto-luastandalone-read-error' => 'Lua error: Internal error: 
Error reading from pipe.',
@@ -129,6 +132,11 @@
        'scribunto-lua-noreturn' => 'Error message.',
        'scribunto-lua-notarrayreturn' => 'Error message.',
        'scribunto-luastandalone-proc-error' => 'Exception message.',
+       'scribunto-luastandalone-proc-error-msg' => 'Exception message. 
Parameters:
+* $1 - (Unused)
+* $2 - Warning/error text from PHP',
+       'scribunto-luastandalone-proc-error-proc-open' => 'Exception message 
displayed when PHP\'s proc_open function is not available, which is needed by 
the LuaStandalone engine.',
+       'scribunto-luastandalone-proc-error-safe-mode' => 'Exception message 
displayed when PHP\'s "safe_mode" configuration directive is enabled.',
        'scribunto-luastandalone-decode-error' => 'Exception message.',
        'scribunto-luastandalone-write-error' => 'Exception message.',
        'scribunto-luastandalone-read-error' => 'Exception message.',
diff --git a/engines/LuaCommon/LuaCommon.php b/engines/LuaCommon/LuaCommon.php
index bd2bd6b..ab893ec 100644
--- a/engines/LuaCommon/LuaCommon.php
+++ b/engines/LuaCommon/LuaCommon.php
@@ -67,32 +67,38 @@
                }
                $this->loaded = true;
 
-               $this->interpreter = $this->newInterpreter();
+               try {
+                       $this->interpreter = $this->newInterpreter();
 
-               $funcs = array(
-                       'loadPackage',
-                       'frameExists',
-                       'newChildFrame',
-                       'getExpandedArgument',
-                       'getAllExpandedArguments',
-                       'expandTemplate',
-                       'callParserFunction',
-                       'preprocess',
-                       'incrementExpensiveFunctionCount',
-               );
+                       $funcs = array(
+                               'loadPackage',
+                               'frameExists',
+                               'newChildFrame',
+                               'getExpandedArgument',
+                               'getAllExpandedArguments',
+                               'expandTemplate',
+                               'callParserFunction',
+                               'preprocess',
+                               'incrementExpensiveFunctionCount',
+                       );
 
-               $lib = array();
-               foreach ( $funcs as $name ) {
-                       $lib[$name] = array( $this, $name );
-               }
+                       $lib = array();
+                       foreach ( $funcs as $name ) {
+                               $lib[$name] = array( $this, $name );
+                       }
 
-               $this->mw = $this->registerInterface( 'mw.lua', $lib,
-                       array( 'allowEnvFuncs' => 
$this->options['allowEnvFuncs'] ) );
+                       $this->mw = $this->registerInterface( 'mw.lua', $lib,
+                               array( 'allowEnvFuncs' => 
$this->options['allowEnvFuncs'] ) );
 
-               $libraries = $this->getLibraries( 'lua', self::$libraryClasses 
);
-               foreach ( $libraries as $name => $class ) {
-                       $this->loadedLibraries[$name] = new $class( $this );
-                       $this->loadedLibraries[$name]->register();
+                       $libraries = $this->getLibraries( 'lua', 
self::$libraryClasses );
+                       foreach ( $libraries as $name => $class ) {
+                               $this->loadedLibraries[$name] = new $class( 
$this );
+                               $this->loadedLibraries[$name]->register();
+                       }
+               } catch ( Exception $ex ) {
+                       $this->loaded = false;
+                       $this->interpreter = null;
+                       throw $ex;
                }
        }
 
diff --git a/engines/LuaStandalone/LuaStandaloneEngine.php 
b/engines/LuaStandalone/LuaStandaloneEngine.php
index 748ec96..af21a7a 100644
--- a/engines/LuaStandalone/LuaStandaloneEngine.php
+++ b/engines/LuaStandalone/LuaStandaloneEngine.php
@@ -141,6 +141,16 @@
                
                wfDebug( __METHOD__.": creating interpreter: $cmd\n" );
 
+               // Check whether proc_open is available before trying to call 
it (e.g.
+               // PHP's disable_functions may have removed it)
+               if ( !function_exists( 'proc_open' ) ) {
+                       throw $this->engine->newException( 
'scribunto-luastandalone-proc-error-proc-open' );
+               }
+
+               // Clear the "last error", so if proc_open fails we can know any
+               // warning was generated by that.
+               @user_error( '' );
+
                $this->proc = proc_open(
                        $cmd, 
                        array(
@@ -150,7 +160,16 @@
                        ),
                        $pipes );
                if ( !$this->proc ) {
-                       throw $this->engine->newException( 
'scribunto-luastandalone-proc-error' );
+                       $err = error_get_last();
+                       if ( !empty( $err['message'] ) ) {
+                               throw $this->engine->newException( 
'scribunto-luastandalone-proc-error-msg',
+                                       array( 'args' => array( $err['message'] 
) ) );
+                       } elseif ( ini_get( 'safe_mode' ) ) {
+                               /** @todo: Remove this case once we no longer 
support PHP 5.3 */
+                               throw $this->engine->newException( 
'scribunto-luastandalone-proc-error-safe-mode' );
+                       } else {
+                               throw $this->engine->newException( 
'scribunto-luastandalone-proc-error' );
+                       }
                }
                $this->writePipe = $pipes[0];
                $this->readPipe = $pipes[1];

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I2887b722e089fd7a526aa7dcab9c80deb343d8ac
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Scribunto
Gerrit-Branch: master
Gerrit-Owner: Anomie <bjor...@wikimedia.org>

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

Reply via email to