new PHP Nominatim\Shell class to wrap shell escaping

diff --git a/lib/Shell.php b/lib/Shell.php
new file mode 100644 (file)
index 0000000..59c4473
--- /dev/null
+++ b/lib/Shell.php
@@ -0,0 +1,80 @@
+<?php
+
+namespace Nominatim;
+
+class Shell
+{
+    public function __construct($sBaseCmd, ...$aParams)
+    {
+        if (!$sBaseCmd) {
+            throw new Exception('Command missing in new() call');
+        }
+        $this->baseCmd = $sBaseCmd;
+        $this->aParams = array();
+        $this->aEnv = null; // null = use the same environment as the current PHP process
+
+        $this->stdoutString = null;
+
+        foreach ($aParams as $sParam) {
+            $this->addParams($sParam);
+        }
+    }
+
+    public function addParams(...$aParams)
+    {
+        foreach ($aParams as $sParam) {
+            if (isset($sParam) && $sParam !== null && $sParam !== '') {
+                array_push($this->aParams, $sParam);
+            }
+        }
+        return $this;
+    }
+
+    public function addEnvPair($sKey, $sVal)
+    {
+        if (isset($sKey) && $sKey && isset($sVal)) {
+            if (!isset($this->aEnv)) $this->aEnv = $_ENV;
+            $this->aEnv = array_merge($this->aEnv, array($sKey => $sVal), $_ENV);
+        }
+        return $this;
+    }
+
+    public function escapedCmd()
+    {
+        $aEscaped = array_map(function ($sParam) {
+            return $this->escapeParam($sParam);
+        }, array_merge(array($this->baseCmd), $this->aParams));
+
+        return join(' ', $aEscaped);
+    }
+
+    public function run()
+    {
+        $sCmd = $this->escapedCmd();
+        // $aEnv does not need escaping, proc_open seems to handle it fine
+
+        $aFDs = array(
+                 0 => array('pipe', 'r'),
+                 1 => STDOUT,
+                 2 => STDERR
+                );
+        $aPipes = null;
+        $hProc = @proc_open($sCmd, $aFDs, $aPipes, null, $this->aEnv);
+        if (!is_resource($hProc)) {
+            throw new \Exception('Unable to run command: ' . $sCmd);
+        }
+
+        fclose($aPipes[0]); // no stdin
+
+        $iStat = proc_close($hProc);
+        return $iStat;
+    }
+
+
+
+    private function escapeParam($sParam)
+    {
+        if (preg_match('/^-*\w+$/', $sParam)) return $sParam;
+        return escapeshellarg($sParam);
+    }
+}

diff --git a/lib/cmd.php b/lib/cmd.php
index 77878c153c73f90440fcbf532413c68ca13dab40..72b666088d1248b601d3edcfad521e99c89da139 100644 (file)
--- a/lib/cmd.php
+++ b/lib/cmd.php
@@ -1,5 +1,6 @@
 <?php
 
 <?php
 

+require_once(CONST_BasePath.'/lib/Shell.php');

 
 function getCmdOpt($aArg, $aSpec, &$aResult, $bExitOnError = false, $bExitOnUnknown = false)
 {
 
 function getCmdOpt($aArg, $aSpec, &$aResult, $bExitOnError = false, $bExitOnUnknown = false)
 {


@@ -148,32 +149,33 @@ function runSQLScript($sScript, $bfatal = true, $bVerbose = false, $bIgnoreError
     // Convert database DSN to psql parameters
     $aDSNInfo = \Nominatim\DB::parseDSN(CONST_Database_DSN);
     if (!isset($aDSNInfo['port']) || !$aDSNInfo['port']) $aDSNInfo['port'] = 5432;
     // Convert database DSN to psql parameters
     $aDSNInfo = \Nominatim\DB::parseDSN(CONST_Database_DSN);
     if (!isset($aDSNInfo['port']) || !$aDSNInfo['port']) $aDSNInfo['port'] = 5432;

-    $sCMD = 'psql'
-        .' -p '.escapeshellarg($aDSNInfo['port'])
-        .' -d '.escapeshellarg($aDSNInfo['database']);
+
+    $oCmd = new \Nominatim\Shell('psql');
+    $oCmd->addParams('--port', $aDSNInfo['port']);
+    $oCmd->addParams('--dbname', $aDSNInfo['database']);

     if (isset($aDSNInfo['hostspec']) && $aDSNInfo['hostspec']) {
     if (isset($aDSNInfo['hostspec']) && $aDSNInfo['hostspec']) {

-        $sCMD .= ' -h ' . escapeshellarg($aDSNInfo['hostspec']);
+        $oCmd->addParams('--host', $aDSNInfo['hostspec']);

     }
     if (isset($aDSNInfo['username']) && $aDSNInfo['username']) {
     }
     if (isset($aDSNInfo['username']) && $aDSNInfo['username']) {

-        $sCMD .= ' -U ' . escapeshellarg($aDSNInfo['username']);
+        $oCmd->addParams('--username', $aDSNInfo['username']);

     }
     }

-    $aProcEnv = null;
-    if (isset($aDSNInfo['password']) && $aDSNInfo['password']) {
-        $aProcEnv = array_merge(array('PGPASSWORD' => $aDSNInfo['password']), $_ENV);
+    if (isset($aDSNInfo['password'])) {
+        $oCmd->addEnvPair('PGPASSWORD', $aDSNInfo['password']);

     }
     if (!$bVerbose) {
     }
     if (!$bVerbose) {

-        $sCMD .= ' -q';
+        $oCmd->addParams('--quiet');

     }
     if ($bfatal && !$bIgnoreErrors) {
     }
     if ($bfatal && !$bIgnoreErrors) {

-        $sCMD .= ' -v ON_ERROR_STOP=1';
+        $oCmd->addParams('-v', 'ON_ERROR_STOP=1');

     }
     }

+

     $aDescriptors = array(
                      0 => array('pipe', 'r'),
                      1 => STDOUT,
                      2 => STDERR
                     );
     $ahPipes = null;
     $aDescriptors = array(
                      0 => array('pipe', 'r'),
                      1 => STDOUT,
                      2 => STDERR
                     );
     $ahPipes = null;

-    $hProcess = @proc_open($sCMD, $aDescriptors, $ahPipes, null, $aProcEnv);
+    $hProcess = @proc_open($oCmd->escapedCmd(), $aDescriptors, $ahPipes, null, $oCmd->aEnv);

     if (!is_resource($hProcess)) {
         fail('unable to start pgsql');
     }
     if (!is_resource($hProcess)) {
         fail('unable to start pgsql');
     }


@@ -193,23 +195,3 @@ function runSQLScript($sScript, $bfatal = true, $bVerbose = false, $bIgnoreError
         fail("pgsql returned with error code ($iReturn)");
     }
 }
         fail("pgsql returned with error code ($iReturn)");
     }
 }

-
-
-function runWithEnv($sCmd, $aEnv)
-{
-    $aFDs = array(
-             0 => array('pipe', 'r'),
-             1 => STDOUT,
-             2 => STDERR
-            );
-    $aPipes = null;
-    $hProc = @proc_open($sCmd, $aFDs, $aPipes, null, $aEnv);
-    if (!is_resource($hProc)) {
-        fail('unable to run command:' . $sCmd);
-    }
-
-    fclose($aPipes[0]); // no stdin
-
-    $iStat = proc_close($hProc);
-    return $iStat;
-}

diff --git a/lib/setup/SetupClass.php b/lib/setup/SetupClass.php
index ac0f8f02f89e33ccc6d0cae4a23c14fb042f98cb..56d9f3451e02701643e67dca36706a34f432274a 100755 (executable)
--- a/lib/setup/SetupClass.php
+++ b/lib/setup/SetupClass.php
@@ -3,6 +3,7 @@
 namespace Nominatim\Setup;
 
 require_once(CONST_BasePath.'/lib/setup/AddressLevelParser.php');
 namespace Nominatim\Setup;
 
 require_once(CONST_BasePath.'/lib/setup/AddressLevelParser.php');

+require_once(CONST_BasePath.'/lib/Shell.php');

 
 class SetupFunctions
 {
 
 class SetupFunctions
 {


@@ -51,7 +52,7 @@ class SetupFunctions
         }
 
         // setting member variables based on command line options stored in $aCMDResult
         }
 
         // setting member variables based on command line options stored in $aCMDResult

-        $this->bQuiet = $aCMDResult['quiet'];
+        $this->bQuiet = isset($aCMDResult['quiet']) && $aCMDResult['quiet'];

         $this->bVerbose = $aCMDResult['verbose'];
 
         //setting default values which are not set by the update.php array
         $this->bVerbose = $aCMDResult['verbose'];
 
         //setting default values which are not set by the update.php array


@@ -76,7 +77,7 @@ class SetupFunctions
             $this->bEnableDiffUpdates = false;
         }
 
             $this->bEnableDiffUpdates = false;
         }
 

-        $this->bDrop = $aCMDResult['drop'];
+        $this->bDrop = isset($aCMDResult['drop']) && $aCMDResult['drop'];

     }
 
     public function createDB()
     }
 
     public function createDB()


@@ -88,19 +89,23 @@ class SetupFunctions
             fail('database already exists ('.CONST_Database_DSN.')');
         }
 
             fail('database already exists ('.CONST_Database_DSN.')');
         }
 

-        $sCreateDBCmd = 'createdb -E UTF-8'
-            .' -p '.escapeshellarg($this->aDSNInfo['port'])
-            .' '.escapeshellarg($this->aDSNInfo['database']);
+        $oCmd = (new \Nominatim\Shell('createdb'))
+                ->addParams('-E', 'UTF-8')
+                ->addParams('-p', $this->aDSNInfo['port']);
+

         if (isset($this->aDSNInfo['username'])) {
         if (isset($this->aDSNInfo['username'])) {

-            $sCreateDBCmd .= ' -U '.escapeshellarg($this->aDSNInfo['username']);
+            $oCmd->addParams('-U', $this->aDSNInfo['username']);
+        }
+        if (isset($this->aDSNInfo['password'])) {
+            $oCmd->addEnvPair('PGPASSWORD', $this->aDSNInfo['password']);

         }
         }

-

         if (isset($this->aDSNInfo['hostspec'])) {
         if (isset($this->aDSNInfo['hostspec'])) {

-            $sCreateDBCmd .= ' -h '.escapeshellarg($this->aDSNInfo['hostspec']);
+            $oCmd->addParams('-h', $this->aDSNInfo['hostspec']);

         }
         }

+        $oCmd->addParams($this->aDSNInfo['database']);

 
 

-        $result = $this->runWithPgEnv($sCreateDBCmd);
-        if ($result != 0) fail('Error executing external command: '.$sCreateDBCmd);
+        $result = $oCmd->run();
+        if ($result != 0) fail('Error executing external command: '.$oCmd->escapedCmd());

     }
 
     public function connect()
     }
 
     public function connect()


@@ -174,39 +179,49 @@ class SetupFunctions
     {
         info('Import data');
 
     {
         info('Import data');
 

-        $osm2pgsql = CONST_Osm2pgsql_Binary;
-        if (!file_exists($osm2pgsql)) {
+        if (!file_exists(CONST_Osm2pgsql_Binary)) {

             echo "Check CONST_Osm2pgsql_Binary in your local settings file.\n";
             echo "Normally you should not need to set this manually.\n";
             echo "Check CONST_Osm2pgsql_Binary in your local settings file.\n";
             echo "Normally you should not need to set this manually.\n";

-            fail("osm2pgsql not found in '$osm2pgsql'");
+            fail("osm2pgsql not found in '".CONST_Osm2pgsql_Binary."'");

         }
 
         }
 

-        $osm2pgsql .= ' -S '.escapeshellarg(CONST_Import_Style);
+        $oCmd = new \Nominatim\Shell(CONST_Osm2pgsql_Binary);
+        $oCmd->addParams('--style', CONST_Import_Style);

 
         if (!is_null(CONST_Osm2pgsql_Flatnode_File) && CONST_Osm2pgsql_Flatnode_File) {
 
         if (!is_null(CONST_Osm2pgsql_Flatnode_File) && CONST_Osm2pgsql_Flatnode_File) {

-            $osm2pgsql .= ' --flat-nodes '.escapeshellarg(CONST_Osm2pgsql_Flatnode_File);
-        }
-
-        if (CONST_Tablespace_Osm2pgsql_Data)
-            $osm2pgsql .= ' --tablespace-slim-data '.escapeshellarg(CONST_Tablespace_Osm2pgsql_Data);
-        if (CONST_Tablespace_Osm2pgsql_Index)
-            $osm2pgsql .= ' --tablespace-slim-index '.escapeshellarg(CONST_Tablespace_Osm2pgsql_Index);
-        if (CONST_Tablespace_Place_Data)
-            $osm2pgsql .= ' --tablespace-main-data '.escapeshellarg(CONST_Tablespace_Place_Data);
-        if (CONST_Tablespace_Place_Index)
-            $osm2pgsql .= ' --tablespace-main-index '.escapeshellarg(CONST_Tablespace_Place_Index);
-        $osm2pgsql .= ' -lsc -O gazetteer --hstore --number-processes 1';
-        $osm2pgsql .= ' -C '.escapeshellarg($this->iCacheMemory);
-        $osm2pgsql .= ' -P '.escapeshellarg($this->aDSNInfo['port']);
+            $oCmd->addParams('--flat-nodes', CONST_Osm2pgsql_Flatnode_File);
+        }
+        if (CONST_Tablespace_Osm2pgsql_Data) {
+            $oCmd->addParams('--tablespace-slim-data', CONST_Tablespace_Osm2pgsql_Data);
+        }
+        if (CONST_Tablespace_Osm2pgsql_Index) {
+            $oCmd->addParams('--tablespace-slim-index', CONST_Tablespace_Osm2pgsql_Index);
+        }
+        if (CONST_Tablespace_Place_Data) {
+            $oCmd->addParams('--tablespace-main-data', CONST_Tablespace_Place_Data);
+        }
+        if (CONST_Tablespace_Place_Index) {
+            $oCmd->addParams('--tablespace-main-index', CONST_Tablespace_Place_Index);
+        }
+        $oCmd->addParams('--latlong', '--slim', '--create');
+        $oCmd->addParams('--output', 'gazetteer');
+        $oCmd->addParams('--hstore');
+        $oCmd->addParams('--number-processes', 1);
+        $oCmd->addParams('--cache', $this->iCacheMemory);
+        $oCmd->addParams('--port', $this->aDSNInfo['port']);
+

         if (isset($this->aDSNInfo['username'])) {
         if (isset($this->aDSNInfo['username'])) {

-            $osm2pgsql .= ' -U '.escapeshellarg($this->aDSNInfo['username']);
+            $oCmd->addParams('--username', $this->aDSNInfo['username']);
+        }
+        if (isset($this->aDSNInfo['password'])) {
+            $oCmd->addEnvPair('PGPASSWORD', $this->aDSNInfo['password']);

         }
         if (isset($this->aDSNInfo['hostspec'])) {
         }
         if (isset($this->aDSNInfo['hostspec'])) {

-            $osm2pgsql .= ' -H '.escapeshellarg($this->aDSNInfo['hostspec']);
+            $oCmd->addParams('--host', $this->aDSNInfo['hostspec']);

         }
         }

-        $osm2pgsql .= ' -d '.escapeshellarg($this->aDSNInfo['database']).' '.escapeshellarg($sOSMFile);
-
-        $this->runWithPgEnv($osm2pgsql);
+        $oCmd->addParams('--database', $this->aDSNInfo['database']);
+        $oCmd->addParams($sOSMFile);
+        $oCmd->run();

 
         if (!$this->sIgnoreErrors && !$this->oDB->getRow('select * from place limit 1')) {
             fail('No Data');
 
         if (!$this->sIgnoreErrors && !$this->oDB->getRow('select * from place limit 1')) {
             fail('No Data');


@@ -529,39 +544,48 @@ class SetupFunctions
 
     public function index($bIndexNoanalyse)
     {
 
     public function index($bIndexNoanalyse)
     {

-        $sBaseCmd = CONST_BasePath.'/nominatim/nominatim.py'
-            .' -d '.escapeshellarg($this->aDSNInfo['database'])
-            .' -P '.escapeshellarg($this->aDSNInfo['port'])
-            .' -t '.escapeshellarg($this->iInstances);
+        $oBaseCmd = (new \Nominatim\Shell(CONST_BasePath.'/nominatim/nominatim.py'))
+                    ->addParams('--database', $this->aDSNInfo['database'])
+                    ->addParams('--port', $this->aDSNInfo['port'])
+                    ->addParams('--threads', $this->iInstances);
+

         if (!$this->bQuiet) {
         if (!$this->bQuiet) {

-            $sBaseCmd .= ' -v';
+            $oBaseCmd->addParams('-v');

         }
         if ($this->bVerbose) {
         }
         if ($this->bVerbose) {

-            $sBaseCmd .= ' -v';
+            $oBaseCmd->addParams('-v');

         }
         if (isset($this->aDSNInfo['hostspec'])) {
         }
         if (isset($this->aDSNInfo['hostspec'])) {

-            $sBaseCmd .= ' -H '.escapeshellarg($this->aDSNInfo['hostspec']);
+            $oBaseCmd->addParams('--host', $this->aDSNInfo['hostspec']);

         }
         if (isset($this->aDSNInfo['username'])) {
         }
         if (isset($this->aDSNInfo['username'])) {

-            $sBaseCmd .= ' -U '.escapeshellarg($this->aDSNInfo['username']);
+            $oBaseCmd->addParams('--user', $this->aDSNInfo['username']);
+        }
+        if (isset($this->aDSNInfo['password'])) {
+            $oBaseCmd->addEnvPair('PGPASSWORD', $this->aDSNInfo['password']);

         }
 
         info('Index ranks 0 - 4');
         }
 
         info('Index ranks 0 - 4');

-        $iStatus = $this->runWithPgEnv($sBaseCmd.' -R 4');
+        $oCmd = (clone $oBaseCmd)->addParams('--maxrank', 4);
+        echo $oCmd->escapedCmd();
+        
+        $iStatus = $oCmd->run();

         if ($iStatus != 0) {
             fail('error status ' . $iStatus . ' running nominatim!');
         }
         if (!$bIndexNoanalyse) $this->pgsqlRunScript('ANALYSE');
 
         info('Index ranks 5 - 25');
         if ($iStatus != 0) {
             fail('error status ' . $iStatus . ' running nominatim!');
         }
         if (!$bIndexNoanalyse) $this->pgsqlRunScript('ANALYSE');
 
         info('Index ranks 5 - 25');

-        $iStatus = $this->runWithPgEnv($sBaseCmd.' -r 5 -R 25');
+        $oCmd = (clone $oBaseCmd)->addParams('--minrank', 5, '--maxrank', 25);
+        $iStatus = $oCmd->run();

         if ($iStatus != 0) {
             fail('error status ' . $iStatus . ' running nominatim!');
         }
         if (!$bIndexNoanalyse) $this->pgsqlRunScript('ANALYSE');
 
         info('Index ranks 26 - 30');
         if ($iStatus != 0) {
             fail('error status ' . $iStatus . ' running nominatim!');
         }
         if (!$bIndexNoanalyse) $this->pgsqlRunScript('ANALYSE');
 
         info('Index ranks 26 - 30');

-        $iStatus = $this->runWithPgEnv($sBaseCmd.' -r 26');
+        $oCmd = (clone $oBaseCmd)->addParams('--minrank', 26);
+        $iStatus = $oCmd->run();

         if ($iStatus != 0) {
             fail('error status ' . $iStatus . ' running nominatim!');
         }
         if ($iStatus != 0) {
             fail('error status ' . $iStatus . ' running nominatim!');
         }


@@ -753,21 +777,21 @@ class SetupFunctions
     {
         if (!file_exists($sFilename)) fail('unable to find '.$sFilename);
 
     {
         if (!file_exists($sFilename)) fail('unable to find '.$sFilename);
 

-        $sCMD = 'psql'
-            .' -p '.escapeshellarg($this->aDSNInfo['port'])
-            .' -d '.escapeshellarg($this->aDSNInfo['database']);
+        $oCmd = (new \Nominatim\Shell('psql'))
+                ->addParams('--port', $this->aDSNInfo['port'])
+                ->addParams('--dbname', $this->aDSNInfo['database']);
+

         if (!$this->bVerbose) {
         if (!$this->bVerbose) {

-            $sCMD .= ' -q';
+            $oCmd->addParams('--quiet');

         }
         if (isset($this->aDSNInfo['hostspec'])) {
         }
         if (isset($this->aDSNInfo['hostspec'])) {

-            $sCMD .= ' -h '.escapeshellarg($this->aDSNInfo['hostspec']);
+            $oCmd->addParams('--host', $this->aDSNInfo['hostspec']);

         }
         if (isset($this->aDSNInfo['username'])) {
         }
         if (isset($this->aDSNInfo['username'])) {

-            $sCMD .= ' -U '.escapeshellarg($this->aDSNInfo['username']);
+            $oCmd->addParams('--username', $this->aDSNInfo['username']);

         }
         }

-        $aProcEnv = null;

         if (isset($this->aDSNInfo['password'])) {
         if (isset($this->aDSNInfo['password'])) {

-            $aProcEnv = array_merge(array('PGPASSWORD' => $this->aDSNInfo['password']), $_ENV);
+            $oCmd->addEnvPair('PGPASSWORD', $this->aDSNInfo['password']);

         }
         $ahGzipPipes = null;
         if (preg_match('/\\.gz$/', $sFilename)) {
         }
         $ahGzipPipes = null;
         if (preg_match('/\\.gz$/', $sFilename)) {


@@ -776,12 +800,14 @@ class SetupFunctions
                              1 => array('pipe', 'w'),
                              2 => array('file', '/dev/null', 'a')
                             );
                              1 => array('pipe', 'w'),
                              2 => array('file', '/dev/null', 'a')
                             );

-            $hGzipProcess = proc_open('zcat '.escapeshellarg($sFilename), $aDescriptors, $ahGzipPipes);
+            $oZcatCmd = new \Nominatim\Shell('zcat', $sFilename);
+
+            $hGzipProcess = proc_open($oZcatCmd->escapedCmd(), $aDescriptors, $ahGzipPipes);

             if (!is_resource($hGzipProcess)) fail('unable to start zcat');
             $aReadPipe = $ahGzipPipes[1];
             fclose($ahGzipPipes[0]);
         } else {
             if (!is_resource($hGzipProcess)) fail('unable to start zcat');
             $aReadPipe = $ahGzipPipes[1];
             fclose($ahGzipPipes[0]);
         } else {

-            $sCMD .= ' -f '.escapeshellarg($sFilename);
+            $oCmd->addParams('--file', $sFilename);

             $aReadPipe = array('pipe', 'r');
         }
         $aDescriptors = array(
             $aReadPipe = array('pipe', 'r');
         }
         $aDescriptors = array(


@@ -790,7 +816,8 @@ class SetupFunctions
                          2 => array('file', '/dev/null', 'a')
                         );
         $ahPipes = null;
                          2 => array('file', '/dev/null', 'a')
                         );
         $ahPipes = null;

-        $hProcess = proc_open($sCMD, $aDescriptors, $ahPipes, null, $aProcEnv);
+
+        $hProcess = proc_open($oCmd->escapedCmd(), $aDescriptors, $ahPipes, null, $oCmd->aEnv);

         if (!is_resource($hProcess)) fail('unable to start pgsql');
         // TODO: error checking
         while (!feof($ahPipes[1])) {
         if (!is_resource($hProcess)) fail('unable to start pgsql');
         // TODO: error checking
         while (!feof($ahPipes[1])) {


@@ -831,21 +858,6 @@ class SetupFunctions
         return $sSql;
     }
 
         return $sSql;
     }
 

-    private function runWithPgEnv($sCmd)
-    {
-        if ($this->bVerbose) {
-            echo "Execute: $sCmd\n";
-        }
-
-        $aProcEnv = null;
-
-        if (isset($this->aDSNInfo['password'])) {
-            $aProcEnv = array_merge(array('PGPASSWORD' => $this->aDSNInfo['password']), $_ENV);
-        }
-
-        return runWithEnv($sCmd, $aProcEnv);
-    }
-

     /**
      * Drop table with the given name if it exists.
      *
     /**
      * Drop table with the given name if it exists.
      *

diff --git a/test/php/Nominatim/ShellTest.php b/test/php/Nominatim/ShellTest.php
new file mode 100644 (file)
index 0000000..d0222ee
--- /dev/null
+++ b/test/php/Nominatim/ShellTest.php
@@ -0,0 +1,120 @@
+<?php
+
+namespace Nominatim;
+
+require_once(CONST_BasePath.'/lib/Shell.php');
+
+class ShellTest extends \PHPUnit\Framework\TestCase
+{
+    public function testNew()
+    {
+        $this->expectException('ArgumentCountError');
+        $this->expectExceptionMessage('Too few arguments to function');
+        $oCmd = new \Nominatim\Shell();
+
+
+        $oCmd = new \Nominatim\Shell('wc', '-l', 'file.txt');
+        $this->assertSame(
+            "wc -l 'file.txt'",
+            $oCmd->escapedCmd()
+        );
+    }
+
+    public function testaddParams()
+    {
+        $oCmd = new \Nominatim\Shell('grep');
+        $oCmd->addParams('-a', 'abc')
+               ->addParams(10);
+
+        $this->assertSame(
+            'grep -a abc 10',
+            $oCmd->escapedCmd(),
+            'no escaping needed, chained'
+        );
+
+        $oCmd = new \Nominatim\Shell('grep');
+        $oCmd->addParams();
+        $oCmd->addParams(null);
+        $oCmd->addParams('');
+
+        $this->assertEmpty($oCmd->aParams);
+        $this->assertSame('grep', $oCmd->escapedCmd(), 'empty params');
+
+        $oCmd = new \Nominatim\Shell('echo', '-n', 0);
+        $this->assertSame(
+            'echo -n 0',
+            $oCmd->escapedCmd(),
+            'zero param'
+        );
+
+        $oCmd = new \Nominatim\Shell('/path with space/do.php');
+        $oCmd->addParams('-a', ' b ');
+        $oCmd->addParams('--flag');
+        $oCmd->addParams('two words');
+        $oCmd->addParams('v=1');
+
+        $this->assertSame(
+            "'/path with space/do.php' -a ' b ' --flag 'two words' 'v=1'",
+            $oCmd->escapedCmd(),
+            'escape whitespace'
+        );
+
+        $oCmd = new \Nominatim\Shell('grep');
+        $oCmd->addParams(';', '|more&', '2>&1');
+
+        $this->assertSame(
+            "grep ';' '|more&' '2>&1'",
+            $oCmd->escapedCmd(),
+            'escape shell characters'
+        );
+    }
+
+    public function testaddEnvPair()
+    {
+        $oCmd = new \Nominatim\Shell('date');
+
+        $oCmd->addEnvPair('one', 'two words')
+             ->addEnvPair('null', null)
+             ->addEnvPair(null, 'null')
+             ->addEnvPair('empty', '')
+             ->addEnvPair('', 'empty');
+
+        $this->assertEquals(
+            array('one' => 'two words', 'empty' => ''),
+            $oCmd->aEnv
+        );
+
+        $oCmd->addEnvPair('one', 'overwrite');
+        $this->assertEquals(
+            array('one' => 'overwrite', 'empty' => ''),
+            $oCmd->aEnv
+        );
+    }
+
+    public function testClone()
+    {
+        $oCmd = new \Nominatim\Shell('wc', '-l', 'file.txt');
+        $oCmd2 = clone $oCmd;
+        $oCmd->addParams('--flag');
+        $oCmd2->addParams('--flag2');
+
+        $this->assertSame(
+            "wc -l 'file.txt' --flag",
+            $oCmd->escapedCmd()
+        );
+
+        $this->assertSame(
+            "wc -l 'file.txt' --flag2",
+            $oCmd2->escapedCmd()
+        );
+    }
+
+    public function testRun()
+    {
+        $oCmd = new \Nominatim\Shell('echo');
+
+        $this->assertSame(0, $oCmd->run());
+
+        // var_dump($sStdout);
+    }
+}

diff --git a/utils/update.php b/utils/update.php
index c1dc2ab938c33886cf5cb8be6cd39b1b2c857e54..b341512c2d598f993fd99013ac93472604951041 100644 (file)
--- a/utils/update.php
+++ b/utils/update.php
@@ -65,30 +65,52 @@ if ($iCacheMemory + 500 > getTotalMemoryMB()) {
     $iCacheMemory = getCacheMemoryMB();
     echo "WARNING: resetting cache memory to $iCacheMemory\n";
 }
     $iCacheMemory = getCacheMemoryMB();
     echo "WARNING: resetting cache memory to $iCacheMemory\n";
 }

-$sOsm2pgsqlCmd = CONST_Osm2pgsql_Binary.' -klas --number-processes 1 -C '.$iCacheMemory.' -O gazetteer -S '.CONST_Import_Style.' -d '.$aDSNInfo['database'].' -P '.$aDSNInfo['port'];
-if (isset($aDSNInfo['username']) && $aDSNInfo['username']) {
-    $sOsm2pgsqlCmd .= ' -U ' . $aDSNInfo['username'];
-}
+
+$oOsm2pgsqlCmd = (new \Nominatim\Shell(CONST_Osm2pgsql_Binary))
+                 ->addParams('--hstore')
+                 ->addParams('--latlong')
+                 ->addParams('--append')
+                 ->addParams('--slim')
+                 ->addParams('--number-processes', 1)
+                 ->addParams('--cache', $iCacheMemory)
+                 ->addParams('--output', 'gazetteer')
+                 ->addParams('--style', CONST_Import_Style)
+                 ->addParams('--database', $aDSNInfo['database'])
+                 ->addParams('--port', $aDSNInfo['port']);
+

 if (isset($aDSNInfo['hostspec']) && $aDSNInfo['hostspec']) {
 if (isset($aDSNInfo['hostspec']) && $aDSNInfo['hostspec']) {

-    $sOsm2pgsqlCmd .= ' -H ' . $aDSNInfo['hostspec'];
+    $oOsm2pgsqlCmd->addParams('--host', $aDSNInfo['hostspec']);
+}
+if (isset($aDSNInfo['username']) && $aDSNInfo['username']) {
+    $oOsm2pgsqlCmd->addParams('--user', $aDSNInfo['username']);

 }
 }

-$aProcEnv = null;

 if (isset($aDSNInfo['password']) && $aDSNInfo['password']) {
 if (isset($aDSNInfo['password']) && $aDSNInfo['password']) {

-    $aProcEnv = array_merge(array('PGPASSWORD' => $aDSNInfo['password']), $_ENV);
+    $oOsm2pgsqlCmd->addEnvPair('PGPASSWORD', $aDSNInfo['password']);

 }
 }

-

 if (!is_null(CONST_Osm2pgsql_Flatnode_File) && CONST_Osm2pgsql_Flatnode_File) {
 if (!is_null(CONST_Osm2pgsql_Flatnode_File) && CONST_Osm2pgsql_Flatnode_File) {

-    $sOsm2pgsqlCmd .= ' --flat-nodes '.CONST_Osm2pgsql_Flatnode_File;
+    $oOsm2pgsqlCmd->addParams('--flat-nodes', CONST_Osm2pgsql_Flatnode_File);

 }
 
 }
 

-$sIndexCmd = CONST_BasePath.'/nominatim/nominatim.py';
-if (!$aResult['quiet']) {
-    $sIndexCmd .= ' -v';
-}
+
+$oIndexCmd = (new \Nominatim\Shell(CONST_BasePath.'/nominatim/nominatim.py'))
+             ->addParams('--database', $aDSNInfo['database'])
+             ->addParams('--port', $aDSNInfo['port'])
+             ->addParams('--threads', $aResult['index-instances']);
+

 if ($aResult['verbose']) {
 if ($aResult['verbose']) {

-    $sIndexCmd .= ' -v';
+    $oIndexCmd->addParams('--verbose');
+}
+if (isset($aDSNInfo['hostspec']) && $aDSNInfo['hostspec']) {
+    $oIndexCmd->addParams('--host', $aDSNInfo['hostspec']);
+}
+if (isset($aDSNInfo['username']) && $aDSNInfo['username']) {
+    $oIndexCmd->addParams('--username', $aDSNInfo['username']);
+}
+if (isset($aDSNInfo['password']) && $aDSNInfo['password']) {
+    $oIndexCmd->addEnvPair('PGPASSWORD', $aDSNInfo['password']);

 }
 
 }
 

+

 if ($aResult['init-updates']) {
     // sanity check that the replication URL is correct
     $sBaseState = file_get_contents(CONST_Replication_Url.'/state.txt');
 if ($aResult['init-updates']) {
     // sanity check that the replication URL is correct
     $sBaseState = file_get_contents(CONST_Replication_Url.'/state.txt');


@@ -104,9 +126,11 @@ if ($aResult['init-updates']) {
         echo "in your local settings file.\n\n";
         fail('CONST_Pyosmium_Binary not configured');
     }
         echo "in your local settings file.\n\n";
         fail('CONST_Pyosmium_Binary not configured');
     }

+

     $aOutput = 0;
     $aOutput = 0;

-    $sCmd = CONST_Pyosmium_Binary.' --help';
-    exec($sCmd, $aOutput, $iRet);
+    $oCMD = new \Nominatim\Shell(CONST_Pyosmium_Binary, '--help');
+    exec($oCMD->escapedCmd(), $aOutput, $iRet);
+

     if ($iRet != 0) {
         echo "Cannot execute pyosmium-get-changes.\n";
         echo "Make sure you have pyosmium installed correctly\n";
     if ($iRet != 0) {
         echo "Cannot execute pyosmium-get-changes.\n";
         echo "Make sure you have pyosmium installed correctly\n";


@@ -132,8 +156,11 @@ if ($aResult['init-updates']) {
 
     // get the appropriate state id
     $aOutput = 0;
 
     // get the appropriate state id
     $aOutput = 0;

-    $sCmd = CONST_Pyosmium_Binary.' -D '.$sWindBack.' --server '.CONST_Replication_Url;
-    exec($sCmd, $aOutput, $iRet);
+    $oCMD = (new \Nominatim\Shell(CONST_Pyosmium_Binary))
+            ->addParams('--start-date', $sWindBack)
+            ->addParams('--server', CONST_Replication_Url);
+
+    exec($oCMD->escapedCmd(), $aOutput, $iRet);

     if ($iRet != 0 || $aOutput[0] == 'None') {
         fail('Error running pyosmium tools');
     }
     if ($iRet != 0 || $aOutput[0] == 'None') {
         fail('Error running pyosmium tools');
     }


@@ -158,7 +185,11 @@ if ($aResult['check-for-updates']) {
         fail('Updates not set up. Please run ./utils/update.php --init-updates.');
     }
 
         fail('Updates not set up. Please run ./utils/update.php --init-updates.');
     }
 

-    system(CONST_BasePath.'/utils/check_server_for_updates.py '.CONST_Replication_Url.' '.$aLastState['sequence_id'], $iRet);
+    $oCmd = (new \Nominatim\Shell(CONST_BasePath.'/utils/check_server_for_updates.py'))
+            ->addParams(CONST_Replication_Url)
+            ->addParams($aLastState['sequence_id']);
+    $iRet = $oCmd->run();
+

     exit($iRet);
 }
 
     exit($iRet);
 }
 


@@ -171,12 +202,12 @@ if (isset($aResult['import-diff']) || isset($aResult['import-file'])) {
     }
 
     // Import the file
     }
 
     // Import the file

-    $sCMD = $sOsm2pgsqlCmd.' '.$sNextFile;
-    echo $sCMD."\n";
-    $iErrorLevel = runWithEnv($sCMD, $aProcEnv);
+    $oCMD = (clone $oOsm2pgsqlCmd)->addParams($sNextFile);
+    echo $oCMD->escapedCmd()."\n";
+    $iRet = $oCMD->run();

 
 

-    if ($iErrorLevel) {
-        fail("Error from osm2pgsql, $iErrorLevel\n");
+    if ($iRet) {
+        fail("Error from osm2pgsql, $iRet\n");

     }
 
     // Don't update the import status - we don't know what this file contains
     }
 
     // Don't update the import status - we don't know what this file contains


@@ -223,11 +254,13 @@ if ($sContentURL) {
 
 if ($bHaveDiff) {
     // import generated change file
 
 if ($bHaveDiff) {
     // import generated change file

-    $sCMD = $sOsm2pgsqlCmd.' '.$sTemporaryFile;
-    echo $sCMD."\n";
-    $iErrorLevel = runWithEnv($sCMD, $aProcEnv);
-    if ($iErrorLevel) {
-        fail("osm2pgsql exited with error level $iErrorLevel\n");
+
+    $oCMD = (clone $oOsm2pgsqlCmd)->addParams($sTemporaryFile);
+    echo $oCMD->escapedCmd()."\n";
+
+    $iRet = $oCMD->run();
+    if ($iRet) {
+        fail("osm2pgsql exited with error level $iRet\n");

     }
 }
 
     }
 }
 


@@ -310,19 +343,11 @@ if ($aResult['recompute-word-counts']) {
 }
 
 if ($aResult['index']) {
 }
 
 if ($aResult['index']) {

-    $sCmd = $sIndexCmd
-            .' -d '.$aDSNInfo['database']
-            .' -P '.$aDSNInfo['port']
-            .' -t '.$aResult['index-instances']
-            .' -r '.$aResult['index-rank'];
-    if (isset($aDSNInfo['hostspec']) && $aDSNInfo['hostspec']) {
-        $sCmd .= ' -H ' . $aDSNInfo['hostspec'];
-    }
-    if (isset($aDSNInfo['username']) && $aDSNInfo['username']) {
-        $sCmd .= ' -U ' . $aDSNInfo['username'];
-    }
+    $oCmd = (clone $oIndexCmd)
+            ->addParams('--minrank', $aResult['index-rank']);

 
 

-    runWithEnv($sCmd, $aProcEnv);
+    // echo $oCmd->escapedCmd()."\n";
+    $oCmd->run();

 
     $oDB->exec('update import_status set indexed = true');
 }
 
     $oDB->exec('update import_status set indexed = true');
 }


@@ -358,18 +383,13 @@ if ($aResult['import-osmosis'] || $aResult['import-osmosis-all']) {
     }
 
     $sImportFile = CONST_InstallPath.'/osmosischange.osc';
     }
 
     $sImportFile = CONST_InstallPath.'/osmosischange.osc';

-    $sCMDDownload = CONST_Pyosmium_Binary.' --server '.CONST_Replication_Url.' -o '.$sImportFile.' -s '.CONST_Replication_Max_Diff_size;
-    $sCMDImport = $sOsm2pgsqlCmd.' '.$sImportFile;
-    $sCMDIndex = $sIndexCmd
-                 .' -d '.$aDSNInfo['database']
-                 .' -P '.$aDSNInfo['port']
-                 .' -t '.$aResult['index-instances'];
-    if (isset($aDSNInfo['hostspec']) && $aDSNInfo['hostspec']) {
-        $sCMDIndex .= ' -H ' . $aDSNInfo['hostspec'];
-    }
-    if (isset($aDSNInfo['username']) && $aDSNInfo['username']) {
-        $sCMDIndex .= ' -U ' . $aDSNInfo['username'];
-    }
+
+    $oCMDDownload = (new \Nominatim\Shell(CONST_Pyosmium_Binary))
+                    ->addParams('--server', CONST_Replication_Url)
+                    ->addParams('--outfile', $sImportFile)
+                    ->addParams('--size', CONST_Replication_Max_Diff_size);
+
+    $oCMDImport = (clone $oOsm2pgsqlCmd)->addParams($sImportFile);

 
     while (true) {
         $fStartTime = time();
 
     while (true) {
         $fStartTime = time();


@@ -399,11 +419,13 @@ if ($aResult['import-osmosis'] || $aResult['import-osmosis-all']) {
                 $fCMDStartTime = time();
                 $iNextSeq = (int) $aLastState['sequence_id'];
                 unset($aOutput);
                 $fCMDStartTime = time();
                 $iNextSeq = (int) $aLastState['sequence_id'];
                 unset($aOutput);

-                echo "$sCMDDownload -I $iNextSeq\n";
+
+                $oCMD = (clone $oCMDDownload)->addParams('--start-id', $iNextSeq);
+                echo $oCMD->escapedCmd()."\n";

                 if (file_exists($sImportFile)) {
                     unlink($sImportFile);
                 }
                 if (file_exists($sImportFile)) {
                     unlink($sImportFile);
                 }

-                exec($sCMDDownload.' -I '.$iNextSeq, $aOutput, $iResult);
+                exec($oCMD->escapedCmd(), $aOutput, $iResult);

 
                 if ($iResult == 3) {
                     echo 'No new updates. Sleeping for '.CONST_Replication_Recheck_Interval." sec.\n";
 
                 if ($iResult == 3) {
                     echo 'No new updates. Sleeping for '.CONST_Replication_Recheck_Interval." sec.\n";


@@ -419,7 +441,8 @@ if ($aResult['import-osmosis'] || $aResult['import-osmosis-all']) {
             // get the newest object from the diff file
             $sBatchEnd = 0;
             $iRet = 0;
             // get the newest object from the diff file
             $sBatchEnd = 0;
             $iRet = 0;

-            exec(CONST_BasePath.'/utils/osm_file_date.py '.$sImportFile, $sBatchEnd, $iRet);
+            $oCMD = new \Nominatim\Shell(CONST_BasePath.'/utils/osm_file_date.py', $sImportFile);
+            exec($oCMD->escapedCmd(), $sBatchEnd, $iRet);

             if ($iRet == 5) {
                 echo "Diff file is empty. skipping import.\n";
                 if (!$aResult['import-osmosis-all']) {
             if ($iRet == 5) {
                 echo "Diff file is empty. skipping import.\n";
                 if (!$aResult['import-osmosis-all']) {


@@ -435,9 +458,11 @@ if ($aResult['import-osmosis'] || $aResult['import-osmosis-all']) {
 
             // Import the file
             $fCMDStartTime = time();
 
             // Import the file
             $fCMDStartTime = time();

-            echo $sCMDImport."\n";
+
+
+            echo $oCMDImport->escapedCmd()."\n";

             unset($sJunk);
             unset($sJunk);

-            $iErrorLevel = runWithEnv($sCMDImport, $aProcEnv);
+            $iErrorLevel = $oCMDImport->run();

             if ($iErrorLevel) {
                 echo "Error executing osm2pgsql: $iErrorLevel\n";
                 exit($iErrorLevel);
             if ($iErrorLevel) {
                 echo "Error executing osm2pgsql: $iErrorLevel\n";
                 exit($iErrorLevel);


@@ -462,11 +487,11 @@ if ($aResult['import-osmosis'] || $aResult['import-osmosis-all']) {
 
         // Index file
         if (!$aResult['no-index']) {
 
         // Index file
         if (!$aResult['no-index']) {

-            $sThisIndexCmd = $sCMDIndex;
+            $oThisIndexCmd = clone($oIndexCmd);

             $fCMDStartTime = time();
 
             $fCMDStartTime = time();
 

-            echo "$sThisIndexCmd\n";
-            $iErrorLevel = runWithEnv($sThisIndexCmd, $aProcEnv);
+            echo $oThisIndexCmd->escapedCmd()."\n";
+            $iErrorLevel = $oThisIndexCmd->run();

             if ($iErrorLevel) {
                 echo "Error: $iErrorLevel\n";
                 exit($iErrorLevel);
             if ($iErrorLevel) {
                 echo "Error: $iErrorLevel\n";
                 exit($iErrorLevel);