Fix installation failure due to unexpected dbpath under CLI installation

1. Make sure `wgSQLiteDataDir` is not empty.
2. Fix a logic bug
  In the current logic, in web environment, SqliteInstaller::dataDirOKmaybeCreate()
  will be executed twice, first in the DBConnect page [1] (with $create=true),
  and the second time in Installer::performInstallation() [2] (with $create=false),
  this is a sanity check.
  But in cli environment, SqliteInstaller::dataDirOKmaybeCreate() will
  be only executed once, called by Installer::performInstallation() (with $create=false).
  So the Cli installer will abort because the data directory is checked
  without the behavior of creating the directory.
  In this case, I split dataDirOKmaybeCreate() into checkDataDir() and
  createDataDir() according to its responsibility. And for web installation,
  we just check the directory on DBConnect page instead of creating it and
  then actually creating it in setupDatabase().
3. Add a unit test for SqliteInstaller::dataDirOKmaybeCreate

[1] DBConnect page call SqliteInstaller::submitConnectForm(),
  ::submitConnectForm() call ::dataDirOKmaybeCreate()
[2] Installer::performInstallation() call SqliteInstaller::setupDatabase(),
  ::setupDatabase() call ::dataDirOKmaybeCreate()

Bug: T217855
Change-Id: I139036b265716e9898fb76ba907c194f005ea318

diff --git a/includes/installer/SqliteInstaller.php b/includes/installer/SqliteInstaller.php
index cf91ccd..d012f1b 100644 (file)
--- a/includes/installer/SqliteInstaller.php
+++ b/includes/installer/SqliteInstaller.php
@@ -71,16 +71,19 @@ class SqliteInstaller extends DatabaseInstaller {
        }
 
        public function getGlobalDefaults() {
+               global $IP;
                $defaults = parent::getGlobalDefaults();
-               if ( isset( $_SERVER['DOCUMENT_ROOT'] ) ) {
-                       $path = str_replace(
-                               [ '/', '\\' ],
-                               DIRECTORY_SEPARATOR,
-                               dirname( $_SERVER['DOCUMENT_ROOT'] ) . '/data'
-                       );
-
-                       $defaults['wgSQLiteDataDir'] = $path;
+               if ( !empty( $_SERVER['DOCUMENT_ROOT'] ) ) {
+                       $path = dirname( $_SERVER['DOCUMENT_ROOT'] );
+               } else {
+                       // We use $IP when unable to get $_SERVER['DOCUMENT_ROOT']
+                       $path = $IP;
                }
+               $defaults['wgSQLiteDataDir'] = str_replace(
+                       [ '/', '\\' ],
+                       DIRECTORY_SEPARATOR,
+                       $path . '/data'
+               );
                return $defaults;
        }
 
@@ -122,7 +125,7 @@ class SqliteInstaller extends DatabaseInstaller {
 
                # Try realpath() if the directory already exists
                $dir = self::realpath( $this->getVar( 'wgSQLiteDataDir' ) );
-               $result = self::dataDirOKmaybeCreate( $dir, true /* create? */ );
+               $result = self::checkDataDir( $dir );
                if ( $result->isOK() ) {
                        # Try expanding again in case we've just created it
                        $dir = self::realpath( $dir );
@@ -135,12 +138,17 @@ class SqliteInstaller extends DatabaseInstaller {
        }
 
        /**
-        * @param string $dir
-        * @param bool $create
-        * @return Status
+        * Check if the data directory is writable or can be created
+        * @param string $dir Path to the data directory
+        * @return Status Return fatal Status if $dir un-writable or no permission to create a directory
         */
-       private static function dataDirOKmaybeCreate( $dir, $create = false ) {
-               if ( !is_dir( $dir ) ) {
+       private static function checkDataDir( $dir ) : Status {
+               if ( is_dir( $dir ) ) {
+                       if ( !is_readable( $dir ) ) {
+                               return Status::newFatal( 'config-sqlite-dir-unwritable', $dir );
+                       }
+               } else {
+                       // Check the parent directory if $dir not exists
                        if ( !is_writable( dirname( $dir ) ) ) {
                                $webserverGroup = Installer::maybeGetWebserverPrimaryGroup();
                                if ( $webserverGroup !== null ) {
@@ -156,25 +164,25 @@ class SqliteInstaller extends DatabaseInstaller {
                                        );
                                }
                        }
+               }
+               return Status::newGood();
+       }
 
-                       # Called early on in the installer, later we just want to sanity check
-                       # if it's still writable
-                       if ( $create ) {
-                               Wikimedia\suppressWarnings();
-                               $ok = wfMkdirParents( $dir, 0700, __METHOD__ );
-                               Wikimedia\restoreWarnings();
-                               if ( !$ok ) {
-                                       return Status::newFatal( 'config-sqlite-mkdir-error', $dir );
-                               }
-                               # Put a .htaccess file in in case the user didn't take our advice
-                               file_put_contents( "$dir/.htaccess", "Deny from all\n" );
+       /**
+        * @param string $dir Path to the data directory
+        * @return Status Return good Status if without error
+        */
+       private static function createDataDir( $dir ) : Status {
+               if ( !is_dir( $dir ) ) {
+                       Wikimedia\suppressWarnings();
+                       $ok = wfMkdirParents( $dir, 0700, __METHOD__ );
+                       Wikimedia\restoreWarnings();
+                       if ( !$ok ) {
+                               return Status::newFatal( 'config-sqlite-mkdir-error', $dir );
                        }
                }
-               if ( !is_writable( $dir ) ) {
-                       return Status::newFatal( 'config-sqlite-dir-unwritable', $dir );
-               }
-
-               # We haven't blown up yet, fall through
+               # Put a .htaccess file in in case the user didn't take our advice
+               file_put_contents( "$dir/.htaccess", "Deny from all\n" );
                return Status::newGood();
        }
 
@@ -217,10 +225,15 @@ class SqliteInstaller extends DatabaseInstaller {
        public function setupDatabase() {
                $dir = $this->getVar( 'wgSQLiteDataDir' );
 
-               # Sanity check. We checked this before but maybe someone deleted the
-               # data dir between then and now
-               $dir_status = self::dataDirOKmaybeCreate( $dir, false /* create? */ );
-               if ( !$dir_status->isOK() ) {
+               # Sanity check (Only available in web installation). We checked this before but maybe someone
+               # deleted the data dir between then and now
+               $dir_status = self::checkDataDir( $dir );
+               if ( $dir_status->isGood() ) {
+                       $res = self::createDataDir( $dir );
+                       if ( !$res->isGood() ) {
+                               return $res;
+                       }
+               } else {
                        return $dir_status;
                }
 

diff --git a/tests/phpunit/unit/includes/installer/SqliteInstallerTest.php b/tests/phpunit/unit/includes/installer/SqliteInstallerTest.php
new file mode 100644 (file)
index 0000000..19a2973
--- /dev/null
+++ b/tests/phpunit/unit/includes/installer/SqliteInstallerTest.php
@@ -0,0 +1,68 @@
+<?php
+
+/**
+ * @group sqlite
+ * @group Database
+ * @group medium
+ */
+class SqliteInstallerTest extends \MediaWikiUnitTestCase {
+       /**
+        * @covers SqliteInstaller::checkDataDir
+        */
+       public function testCheckDataDir() {
+               $method = new ReflectionMethod( SqliteInstaller::class, 'checkDataDir' );
+               $method->setAccessible( true );
+
+               # Test 1: Should return fatal Status if $dir exist and it un-writable
+               if ( ( isset( $_SERVER['USER'] ) && $_SERVER['USER'] !== 'root' ) && !wfIsWindows() ) {
+                       // We can't simulate this environment under Windows or login as root
+                       $dir = sys_get_temp_dir() . '/' . uniqid( 'MediaWikiTest' );
+                       mkdir( $dir, 0000 );
+                       /** @var Status $status */
+                       $status = $method->invoke( null, $dir );
+                       $this->assertFalse( $status->isGood() );
+                       $this->assertSame( 'config-sqlite-dir-unwritable', $status->getErrors()[0]['message'] );
+                       rmdir( $dir );
+               }
+
+               # Test 2: Should return fatal Status if $dir not exist and it parent also not exist
+               $dir = sys_get_temp_dir() . '/' . uniqid( 'MediaWikiTest' ) . '/' . uniqid( 'MediaWikiTest' );
+               $status = $method->invoke( null, $dir );
+               $this->assertFalse( $status->isGood() );
+
+               # Test 3: Should return good Status if $dir not exist and it parent writable
+               $dir = sys_get_temp_dir() . '/' . uniqid( 'MediaWikiTest' );
+               /** @var Status $status */
+               $status = $method->invoke( null, $dir );
+               $this->assertTrue( $status->isGood() );
+       }
+
+       /**
+        * @covers SqliteInstaller::createDataDir
+        */
+       public function testCreateDataDir() {
+               $method = new ReflectionMethod( SqliteInstaller::class, 'createDataDir' );
+               $method->setAccessible( true );
+
+               # Test 1: Should return fatal Status if $dir not exist and it parent un-writable
+               if ( ( isset( $_SERVER['USER'] ) && $_SERVER['USER'] !== 'root' ) && !wfIsWindows() ) {
+                       // We can't simulate this environment under Windows or login as root
+                       $random = uniqid( 'MediaWikiTest' );
+                       $dir = sys_get_temp_dir() . '/' . $random . '/' . uniqid( 'MediaWikiTest' );
+                       mkdir( sys_get_temp_dir() . "/$random", 0000 );
+                       /** @var Status $status */
+                       $status = $method->invoke( null, $dir );
+                       $this->assertFalse( $status->isGood() );
+                       $this->assertSame( 'config-sqlite-mkdir-error', $status->getErrors()[0]['message'] );
+                       rmdir( sys_get_temp_dir() . "/$random" );
+               }
+
+               # Test 2: Test .htaccess content after created successfully
+               $dir = sys_get_temp_dir() . '/' . uniqid( 'MediaWikiTest' );
+               $status = $method->invoke( null, $dir );
+               $this->assertTrue( $status->isGood() );
+               $this->assertSame( "Deny from all\n", file_get_contents( "$dir/.htaccess" ) );
+               unlink( "$dir/.htaccess" );
+               rmdir( $dir );
+       }
+}