fix MailboxHandler -> adding mailbox with empty quota field

'--quota -1' gets parsed as two options ("quota" and "1"), but it's
meant to be "quota => -1".

Make sure '-1' is considered as a value, not as an option.

Also get rid of unset()'ing $params[$i] and (now?) superfluous recursive
calls to __parseParams() to make the code less confusing.

.github/workflows/php.yml

@@ -29,7 +29,6 @@ jobs:
       run: mkdir -p build/logs || true
 
     - name: Coveralls
-      run: php vendor/bin/php-coveralls -v --coverage_clover=coverage.xml  || true
+      run: vendor/bin/coveralls ./clover.xml || true
       env:
-          COVERALLS_RUN_LOCALLY: 1
-          COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_REPO_TOKEN }}
+        COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_REPO_TOKEN }}

INSTALL.TXT

@@ -7,14 +7,13 @@
 
 REQUIREMENTS
 ------------
-- Postfix 2.0 or higher.
-- Apache 1.3.27 / Lighttpd 1.3.15 or higher.
-- PHP 5.1.2 or higher.
+- Postfix
+- Apache / Lighttpd 
+- PHP (for web server)
 - one of the following databases:
-  - MySQL 3.23 or higher (5.x recommended)
-  - MariaDB (counts as MySQL ;-)
-  - PostgreSQL 7.4 (or higher)
-  - SQLite 3.12 (or higher)
+  - MariaDB/MySQL
+  - PostgreSQL
+  - SQLite
 
 
 READ THIS FIRST!
@@ -42,10 +41,10 @@ DOCUMENTS/ folder.
 
 (if you installed PostfixAdmin as RPM or DEB package, you can obviously skip this step.)
 
-Assuming we are installing Postfixadmin into /srv/postfixadmin, then something like this should work :
+Assuming we are installing Postfixadmin into /srv/postfixadmin, then something like this should work. Please check https://github.com/postfixadmin/postfixadmin/releases to get the latest stable release first (the 3.2.4 version/url below is probably stale)
 
   $ cd /srv/
-  $ wget -O postfixadmin.tgz https://github.com/postfixadmin/postfixadmin/archive/postfixadmin-3.2.tar.gz
+  $ wget -O postfixadmin.tgz https://github.com/postfixadmin/postfixadmin/archive/postfixadmin-3.2.4.tar.gz
   $ tar -zxvf postfixadmin.tgz
   $ mv postfixadmin-postfixadmin-3.2 postfixadmin
 
@@ -54,7 +53,7 @@ Alternatively :
   $ cd /srv
   $ git clone https://github.com/postfixadmin/postfixadmin.git
   $ cd postfixadmin
-  $ git checkout postfixadmin-3.2.2
+  $ git checkout postfixadmin-3.2.4
 
 2. Setup Web Server
 -------------------
@@ -85,6 +84,7 @@ For MySQL:
   CREATE DATABASE postfix;
   CREATE USER 'postfix'@'localhost' IDENTIFIED BY 'choose_a_password';
   GRANT ALL PRIVILEGES ON `postfix` . * TO 'postfix'@'localhost';
+  FLUSH PRIVILEGES;
 
 For PostgreSQL:
   CREATE USER postfix WITH PASSWORD 'whatever';

composer.json

@@ -14,9 +14,9 @@
         "check-format": "php-cs-fixer fix --ansi --dry-run --diff",
         "format": "php-cs-fixer fix --ansi",
         "lint": "@php ./vendor/bin/parallel-lint --exclude vendor/ --exclude lib/block_random_int.php --exclude lib/array_column.php .",
-        "test": "DATABASE=sqlite ./vendor/bin/phpunit --coverage-clover ./coverage.xml tests/",
+        "test": "DATABASE=sqlite ./vendor/bin/phpunit --coverage-clover ./clover.xml tests/",
         "test-fixup": "mkdir -p templates_c ; test -f config.local.php || touch config.local.php",
-        "psalm": "@php ./vendor/bin/psalm --no-cache --show-info=false "
+        "psalm": "@php ./vendor/bin/psalm --show-info=false "
     },
     "require": {
         "php": ">=7.0"
@@ -25,10 +25,10 @@
         "ext-mysqli":  "*",
         "ext-sqlite3": "*",
         "friendsofphp/php-cs-fixer": "*",
-        "jakub-onderka/php-parallel-lint": "^1.0",
+        "php-parallel-lint/php-parallel-lint": "^1.0",
         "php":  ">7.2.0",
-        "php-coveralls/php-coveralls" : "*",
-        "phpunit/phpunit": "^6|^7",
+        "cedx/coveralls": "^11.0",
+        "phpunit/phpunit": "8.*",
         "vimeo/psalm":"^3.0",
         "shardj/zf1-future" : "^1.12"
     },
@@ -38,5 +38,10 @@
             "functions.inc.php",
             "lib/smarty/libs/bootstrap.php"
         ]
+    },
+    "support": {
+        "irc": "irc://irc.freenode.org/postfixadmin",
+        "issues": "https://github.com/postfixadmin/postfixadmin/issues",
+        "chat": "https://gitter.im/postfixadmin/Lobby"
     }
 }

config.inc.php

@@ -184,7 +184,7 @@ $CONF['smtp_sendmail_tls'] = 'NO';
 // mysql_encrypt = useful for PAM integration
 // authlib = support for courier-authlib style passwords - also set $CONF['authlib_default_flavor']
 // dovecot:CRYPT-METHOD = use dovecotpw -s 'CRYPT-METHOD'. Example: dovecot:CRAM-MD5
-// php_crypt:CRYPT-METHOD:DIFFICULTY:PREFIX = use PHP built in crypt()-function. Example: php_crypt:SHA512:50000
+// php_crypt:CRYPT-METHOD:DIFFICULTY = use PHP built in crypt()-function. Example: php_crypt:SHA512:50000
 // - php_crypt CRYPT-METHOD: Supported values are DES, MD5, BLOWFISH, SHA256, SHA512
 // - php_crypt DIFFICULTY: Larger value is more secure, but uses more CPU and time for each login.
 // - php_crypt DIFFICULTY: Set this according to your CPU processing power.
@@ -194,7 +194,6 @@ $CONF['smtp_sendmail_tls'] = 'NO';
 //     - don't use dovecot:* methods that include the username in the hash - you won't be able to login to PostfixAdmin in this case
 //     - you'll need at least dovecot 2.1 for salted passwords ('doveadm pw' 2.0.x doesn't support the '-t' option)
 //     - dovecot 2.0.0 - 2.0.7 is not supported
-// - php_crypt PREFIX: hash has specified prefix - example: php_crypt:SHA512::{SHA256-CRYPT}
 // sha512.b64 - {SHA512-CRYPT.B64} (base64 encoded sha512) (no dovecot dependency; should support migration from md5crypt)
 $CONF['encrypt'] = 'md5crypt';
 

functions.inc.php

@@ -1061,15 +1061,12 @@ function _pacrypt_dovecot($pw, $pw_db = '') {
 /**
  * Supports DES, MD5, BLOWFISH, SHA256, SHA512 methods.
  *
- * Via config we support an optional prefix (e.g. if you need hashes to start with {SHA256-CRYPT} and optional rounds (hardness) setting.
- *
  * @param string $pw
  * @param string $pw_db (can be empty if setting a new password)
  * @return string crypt'ed password; if it matches $pw_db then $pw is the original password.
  */
-function _pacrypt_php_crypt($pw, $pw_db)
-{
-    $configEncrypt = Config::read_string('encrypt');
+function _pacrypt_php_crypt($pw, $pw_db) {
+    global $CONF;
 
     // use PHPs crypt(), which uses the system's crypt()
     // same algorithms as used in /etc/shadow
@@ -1077,48 +1074,31 @@ function _pacrypt_php_crypt($pw, $pw_db)
     // the algorithm for a new hash is chosen by feeding a salt with correct magic to crypt()
     // set $CONF['encrypt'] to 'php_crypt' to use the default SHA512 crypt method
     // set $CONF['encrypt'] to 'php_crypt:METHOD' to use another method; methods supported: DES, MD5, BLOWFISH, SHA256, SHA512
-    // set $CONF['encrypt'] to 'php_crypt:METHOD:difficulty' where difficulty is between 1000-999999999
-    // set $CONF['encrypt'] to 'php_crypt:METHOD:difficulty:PREFIX' to prefix the hash with the {PREFIX} etc.
     // tested on linux
 
-    $prefix = '';
-
     if (strlen($pw_db) > 0) {
         // existing pw provided. send entire password hash as salt for crypt() to figure out
         $salt = $pw_db;
-
-        // if there was a prefix in the password, use this (override anything given in the config).
-
-        if (preg_match('/^\{([-A-Z0-9]+)\}(.+)$/', $pw_db, $method_matches)) {
-            $salt = $method_matches[2];
-            $prefix = "{" . $method_matches[1] . "}";
-        }
-
     } else {
         $salt_method = 'SHA512'; // hopefully a reasonable default (better than MD5)
         $hash_difficulty = '';
         // no pw provided. create new password hash
-        if (strpos($configEncrypt, ':') !== false) {
+        if (strpos($CONF['encrypt'], ':') !== false) {
             // use specified hash method
-            $spec = explode(':', $configEncrypt);
-            $salt_method = $spec[1];
-            if (isset($spec[2])) {
-                $hash_difficulty = $spec[2];
-            }
-            if (isset($spec[3])) {
-                $prefix = $spec[3]; // hopefully something like {SHA256-CRYPT}
+            $split_method = explode(':', $CONF['encrypt']);
+            $salt_method = $split_method[1];
+            if (count($split_method) >= 3) {
+                $hash_difficulty = $split_method[2];
             }
         }
         // create appropriate salt for selected hash method
         $salt = _php_crypt_generate_crypt_salt($salt_method, $hash_difficulty);
     }
-
+    // send it to PHPs crypt()
     $password = crypt($pw, $salt);
-
-    return "{$prefix}{$password}";
+    return $password;
 }
 
-
 /**
  * @param string $hash_type must be one of: MD5, DES, BLOWFISH, SHA256 or SHA512  (default)
  * @param int hash difficulty

languages/language-update.sh

@@ -21,7 +21,7 @@
 
 function update_string_list() {
 	for file in en.lang $filelist ; do
-		echo "<?php include('$file'); print join(\"\\n\", array_keys(\$PALANG)) . \"\\n\"; ?>" | php > $file.strings
+		echo "<?php \$CONF['admin_name'] = ''; include('$file'); print join(\"\\n\", array_keys(\$PALANG)) . \"\\n\"; ?>" | php > $file.strings
 	done
 
 	for file in $filelist ; do

model/AliasHandler.php

@@ -311,7 +311,10 @@ class AliasHandler extends PFAHandler {
     protected function read_from_db_postprocess($db_result) {
         foreach ($db_result as $key => $value) {
             # split comma-separated 'goto' into an array
-            $db_result[$key]['goto'] = explode(',', $db_result[$key]['goto']);
+            $goto = $db_result[$key]['goto'] ?? null;
+            if (is_string($goto)) {
+                $db_result[$key]['goto'] = explode(',', $goto);
+            }
 
             # Vacation enabled?
             list($db_result[$key]['on_vacation'], $db_result[$key]['goto']) = remove_from_array($db_result[$key]['goto'], $this->getVacationAlias());

model/Config.php

@@ -50,33 +50,22 @@ final class Config {
 
         $newConfig = $_this->getAll();
 
-        foreach ($config as $names => $value) {
-            $name = $_this->__configVarNames($names);
-
-            switch (count($name)) {
-            case 3:
-                $newConfig[$name[0]][$name[1]][$name[2]] = $value;
-                break;
-            case 2:
-                $newConfig[$name[0]][$name[1]] = $value;
-                break;
-            case 1:
-                $newConfig[$name[0]] = $value;
-                break;
-            }
+        foreach ($config as $name => $value) {
+            $newConfig[$name] = $value;
         }
+
         $_this->setAll($newConfig);
     }
 
     /**
-     * @return array
      * @param string $var
+     * @return array
      */
     public static function read_array($var) {
         $stuff = self::read($var);
 
         if (!is_array($stuff)) {
-            trigger_error('In '.__FUNCTION__.": expected config $var to be an array, but received a " . gettype($stuff), E_USER_ERROR);
+            trigger_error('In ' . __FUNCTION__ . ": expected config $var to be an array, but received a " . gettype($stuff), E_USER_ERROR);
         }
 
         return $stuff;
@@ -94,7 +83,7 @@ final class Config {
         }
 
         if (!is_string($stuff)) {
-            trigger_error('In '.__FUNCTION__.": expected config $var to be a string, but received a " . gettype($stuff), E_USER_ERROR);
+            trigger_error('In ' . __FUNCTION__ . ": expected config $var to be a string, but received a " . gettype($stuff), E_USER_ERROR);
             return '';
         }
 
@@ -121,34 +110,12 @@ final class Config {
             return $config;
         }
 
-        $name = $_this->__configVarNames($var);
-
-        switch (count($name)) {
-        case 3:
-            $zero = $name[0];
-            $one = $name[1];
-            $two = $name[2];
-            if (isset($config[$zero], $config[$zero][$one], $config[$zero][$one][$two])) {
-                return $config[$zero][$one][$two];
-            }
-            break;
-        case 2:
-            $zero = $name[0];
-            $one = $name[1];
-            if (isset($config[$zero], $config[$zero][$one])) {
-                return $config[$zero][$one];
-            }
-            break;
-        case 1:
-            $zero = $name[0];
-            if (isset($config[$zero])) {
-                return $config[$zero];
-            }
-            break;
+        if (isset($config[$var])) {
+            return $config[$var];
         }
 
-        if (!in_array(join('.', $name), self::$deprecated_options)) {
-            error_log('Config::read(): attempt to read undefined config option "' . join('.', $name) . '", returning null');
+        if (!in_array($var, self::$deprecated_options)) {
+            error_log('Config::read(): attempt to read undefined config option "' . $var . '", returning null');
         }
 
         return null;
@@ -194,7 +161,7 @@ final class Config {
         }
 
         if (!is_string($value)) {
-            trigger_error('In '.__FUNCTION__.": expected config $var to be a string, but received a " . gettype($value), E_USER_ERROR);
+            trigger_error('In ' . __FUNCTION__ . ": expected config $var to be a string, but received a " . gettype($value), E_USER_ERROR);
             error_log("config $var should be a string, found: " . json_encode($value));
             return false;
         }
@@ -222,7 +189,6 @@ final class Config {
     }
 
 
-
     /**
      * Get translated text from $PALANG
      * (wrapper for self::read(), see also the comments there)
@@ -232,15 +198,14 @@ final class Config {
      * @access public
      */
     public static function lang($var) {
-        $value = self::read("__LANG.{$var}");
+        $languages = self::read_array('__LANG');
 
-        if (is_null($value)) {
-            return '';
-        }
+        $value = $languages[$var] ?? '';
 
         if (!is_string($value)) {
-            trigger_error('In '.__FUNCTION__.": expected config $var to be a string , but received a " . gettype($value), E_USER_ERROR);
+            trigger_error('In ' . __FUNCTION__ . ": expected config $var to be a string , but received a " . gettype($value), E_USER_ERROR);
         }
+
         return $value;
     }
 
@@ -253,7 +218,18 @@ final class Config {
      * @return string value of $PALANG[$var], parsed by sprintf
      */
     public static function lang_f($var, $value) {
-        return self::read_f('__LANG.'. $var, $value);
+        $all = self::read_array('__LANG');
+
+        $text = $all[$var] ?? '';
+
+        $newtext = sprintf($text, $value);
+
+        # check if sprintf changed something - if not, there are chances that $text didn't contain a %s
+        if ($text == $newtext) {
+            error_log("$var used via read_f, but nothing replaced (value $value)");
+        }
+
+        return $newtext;
     }
 
     /**
@@ -270,23 +246,6 @@ final class Config {
     public function setAll(array $config) {
         $this->config = $config;
     }
-
-    /**
-     * Checks $name for dot notation to create dynamic Configure::$var as an array when needed.
-     *
-     * @param mixed $name Name to split
-     * @return array Name separated in items through dot notation
-     * @access private
-     */
-    private function __configVarNames($name) {
-        if (is_string($name)) {
-            if (strpos($name, ".")) {
-                return explode(".", $name);
-            }
-            return array($name);
-        }
-        return $name;
-    }
 }
 
 /* vim: set expandtab softtabstop=4 tabstop=4 shiftwidth=4: */

model/MailboxHandler.php

@@ -227,6 +227,11 @@ class MailboxHandler extends PFAHandler {
             $this->values['quota'] = $this->values['quota'] * $multiplier; # convert quota from MB to bytes
         }
 
+        // Avoid trying to store '' in an integer field
+        if($this->values['quota'] === '') {
+            $this->values['quota'] = 0;
+        }
+
         $ah = new AliasHandler($this->new, $this->admin_username);
 
         $ah->calledBy('MailboxHandler');

public/setup.php

@@ -37,11 +37,12 @@ require(dirname(__FILE__) . '/../templates/header.php');
             //
             $f_phpversion = function_exists("phpversion");
             $f_apache_get_version = function_exists("apache_get_version");
-            $f_mysql_connect = function_exists("mysql_connect");
-            $f_mysqli_connect = function_exists("mysqli_connect");
-            $f_pg_connect = function_exists("pg_connect");
-            $f_sqlite_open = class_exists("SQLite3");
-            $f_pdo = class_exists('PDO');
+
+            $m_pdo       = extension_loaded("PDO");
+            $m_pdo_mysql = extension_loaded("pdo_mysql");
+            $m_pdo_pgsql = extension_loaded('pdo_pgsql');
+            $m_pdo_sqlite= extension_loaded("pdo_sqlite");
+
             $f_session_start = function_exists("session_start");
             $f_preg_match = function_exists("preg_match");
             $f_mb_encode_mimeheader = function_exists("mb_encode_mimeheader");
@@ -105,68 +106,42 @@ require(dirname(__FILE__) . '/../templates/header.php');
                 print "Create the file, and edit as appropriate (e.g. select database type etc)<br />";
             }
 
-            //
             // Check if there is support for at least 1 database
-            //
-            if (($f_pdo == 0) and ($f_mysql_connect == 0) and ($f_mysqli_connect == 0) and ($f_pg_connect == 0) and ($f_sqlite_open == 0)) {
-                print "<li><b>Error: There is no database support in your PHP setup</b><br />\n";
-                print "To install MySQL 3.23 or 4.0 support on FreeBSD:<br />\n";
-                print "<pre>% cd /usr/ports/databases/php{$phpversion}-mysql/\n";
-                print "% make clean install\n";
-                print " - or with portupgrade -\n";
-                print "% portinstall php{$phpversion}-mysql</pre>\n";
-                if ($phpversion >= 5) {
-                    print "To install MySQL 4.1 support on FreeBSD:<br />\n";
-                    print "<pre>% cd /usr/ports/databases/php5-mysqli/\n";
-                    print "% make clean install\n";
-                    print " - or with portupgrade -\n";
-                    print "% portinstall php5-mysqli</pre>\n";
-                }
-                print "To install PostgreSQL support on FreeBSD:<br />\n";
-                print "<pre>% cd /usr/ports/databases/php{$phpversion}-pgsql/\n";
-                print "% make clean install\n";
-                print " - or with portupgrade -\n";
-                print "% portinstall php{$phpversion}-pgsql</pre></li>\n";
+            if (($m_pdo == 0) and ($m_pdo_mysql == 0) and ($m_pdo_sqlite == 0) and ($m_pdo_psql == 0) ) {
+                print "<li><b>Error: There is no database (PDO) support in your PHP setup</b><br />\n";
+                print "<span style='color: red'>
+                    You MUST install a suitable PHP PDO extension (e.g. pdo_pgsql, pdo_mysql or pdo_sqlite).
+                    </span>\n</li>";
                 $error += 1;
             }
 
-            if ($f_mysqli_connect == 1) {
-                print "<li>Database - MySQL (mysqli_ functions) - Found\n";
-                if (Config::read_string('database_type') != 'mysqli') {
-                    print "<br>(change the database_type to 'mysqli' in config.local.php if you want to use MySQL)\n";
-                }
-                print "</li>";
+            if ($m_pdo_mysql == 1) {
+                print "<li>Database - PDO MySQL - Found</li>";
             } else {
-                print "<li>Database - MySQL (mysqli_ functions) - Not found</li>";
-            }
-
-
-            if (Config::read_string('database_type') == 'mysql') {
-                print "<li><strong><span style='color: red'>Warning:</span> your configured database_type 'mysql' is deprecated; you must move to use 'mysqli'</strong> in your config.local.php.</li>\n";
-                $error++;
+                print "<li>Database - MySQL (pdo_mysql) extension not found</li>";
             }
 
             //
             // PostgreSQL functions
             //
-            if ($f_pg_connect == 1) {
-                print "<li>Database : PostgreSQL support (pg_ functions) - Found\n";
+            if ($m_pdo_pgsql == 1) {
+                print "<li>Database : PDO PostgreSQL - Found \n";
                 if (Config::read_string('database_type') != 'pgsql') {
                     print "<br>(change the database_type to 'pgsql' in config.local.php if you want to use PostgreSQL)\n";
                 }
                 print "</li>";
             } else {
-                print "<li>Database - PostgreSQL (pg_ functions) - Not found</li>";
+                print "<li>Database - PostgreSQL (pdo_pgsql) extension not found</li>";
             }
 
-            if ($f_sqlite_open == 1) {
-                print "<li>Database : SQLite support (SQLite3) - Found \n";
+            if ($m_pdo_sqlite == 1) {
+                print "<li>Database : PDO SQLite - Found \n";
                 if (Config::read_string('database_type') != 'sqlite') {
                     print "<br>(change the database_type to 'sqlite' in config.local.php if you want to use SQLite)\n";
                 }
                 print "</li>";
             } else {
-                print "<li>Database - SQLite (SQLite3) - Not found</li>";
+                print "<li>Database - SQLite (pdo_sqlite) extension not found</li>";
             }
 
             //
@@ -226,8 +201,8 @@ require(dirname(__FILE__) . '/../templates/header.php');
             if ($f_mb_encode_mimeheader == 1) {
                 print "<li>Depends on: multibyte string - Found</li>\n";
             } else {
-                print "<li><b>Error: Depends on: multibyte string - NOT FOUND</b><br />\n";
-                print "To install multibyte string support, install php$phpversion-mbstring</li>\n";
+                print "<li><b>Error: Depends on: multibyte string - mbstring extension missing.</b><br />\n";
+                print "To install multibyte string support, perhaps install php$phpversion-mbstring</li>\n";
                 $error += 1;
             }
 
@@ -238,8 +213,8 @@ require(dirname(__FILE__) . '/../templates/header.php');
             if ($f_imap_open == 1) {
                 print "<li>IMAP functions - Found</li>\n";
             } else {
-                print "<li><b>Warning: May depend on: IMAP functions - Not Found</b><br />\n";
-                print "To install IMAP support, install php$phpversion-imap<br />\n";
+                print "<li><b>Warning: Optional dependency 'imap' extension missing</b><br />\n";
+                print "To install IMAP support, perhaps install php$phpversion-imap<br />\n";
                 print "Without IMAP support, you won't be able to create subfolders when creating mailboxes.</li>\n";
             }
 

public/upgrade.php

@@ -103,7 +103,7 @@ function _db_add_field($table, $field, $fieldtype, $after = '') {
 
 function echo_out($text) {
     if (defined('PHPUNIT_TEST')) {
-        error_log("" . $text);
+        //error_log("" . $text);
     } else {
         echo $text . "\n";
     }

scripts/postfixadmin-cli.php

@@ -314,34 +314,21 @@ class PostfixAdmin {
      * @param array $params Parameters to parse
      */
     public function parseParams($params) {
-        $this->__parseParams($params);
-    }
-
-    /**
-     * Helper for recursively parsing params
-     */
-    private function __parseParams($params) {
         $count = count($params);
         for ($i = 0; $i < $count; $i++) {
-            if (isset($params[$i])) {
-                if ($params[$i] != '' && $params[$i]{0} === '-') {
-                    $key = substr($params[$i], 1);
-                    $this->params[$key] = true;
-                    unset($params[$i]);
-                    if (isset($params[++$i])) {
-                        # TODO: ideally we should know if a parameter can / must have a value instead of whitelisting known valid values starting with '-' (probably only bool doesn't need a value)
-                        if ($params[$i]{0} !== '-' or $params[$i] != '-1') {
-                            $this->params[$key] = $params[$i];
-                            unset($params[$i]);
-                        } else {
-                            $i--;
-                            $this->__parseParams($params);
-                        }
+            if ($params[$i] != '' && $params[$i]{0} === '-' && $params[$i] != '-1') {
+                $key = substr($params[$i], 1);
+                if (isset($params[$i+1])) {
+                    # TODO: ideally we should know if a parameter can / must have a value instead of whitelisting known valid values starting with '-' (probably only bool doesn't need a value)
+                    if ($params[$i+1]{0} === '-' && $params[$i+1] != '-1') {
+                        $this->params[$key] = true;
+                    } else {
+                        $this->params[$key] = $params[$i+1];
+                        $i++;
                     }
-                } else {
-                    $this->args[] = $params[$i];
-                    unset($params[$i]);
                 }
+            } else {
+                $this->args[] = $params[$i];
             }
         }
     }

tests/AliasHandlerTest.php

@@ -9,7 +9,7 @@ class AliasHandlerTest extends \PHPUnit\Framework\TestCase {
         $this->assertEmpty($results);
     }
 
-    public function tearDown() {
+    public function tearDown() : void {
         $_SESSION = [];
         db_query('DELETE FROM alias');
         db_query('DELETE FROM domain');

tests/ConfigTest.php

@@ -0,0 +1,33 @@
+<?php
+
+class ConfigTest extends \PHPUnit\Framework\TestCase {
+    public function setUp() : void {
+        $c = Config::getInstance();
+
+        $all = $c->getAll();
+
+        $all['xmlrpc_enabled'] = false;
+
+        $c->setAll($all);
+
+        parent::setUp();
+    }
+
+    public function testLangF() {
+        $x = Config::lang_f('must_be_numeric', 'foo@bar');
+
+        $this->assertEquals('foo@bar must be numeric', $x);
+    }
+
+    public function testLang() {
+        $x = Config::lang('must_be_numeric', 'foo@bar');
+
+        $this->assertEquals('%s must be numeric', $x);
+    }
+
+    public function testBool() {
+        $x = Config::bool('xmlrpc_enabled');
+
+        $this->assertFalse($x);
+    }
+}

tests/CreatePageBrowserTest.php

@@ -1,7 +1,7 @@
 <?php
 
 class CreatePageBrowserTest extends \PHPUnit\Framework\TestCase {
-    public function tearDown() {
+    public function tearDown() : void {
         $this->cleanup();
     }
 

tests/DbBasicTest.php

@@ -3,7 +3,7 @@
 class DbBasicTest extends \PHPUnit\Framework\TestCase {
     private $test_domain;
 
-    public function setUp() {
+    public function setUp() : void {
         $db = db_connect();
         $test_domain = 'test' . uniqid() . '.com';
         $this->test_domain = $test_domain;

tests/MailboxHandlerTest.php

@@ -1,7 +1,7 @@
 <?php
 
 class MailboxHandlerTest extends \PHPUnit\Framework\TestCase {
-    public function tearDown() {
+    public function tearDown() : void {
         db_query('DELETE FROM mailbox');
         db_query('DELETE FROM domain');
         db_query('DELETE FROM domain_admins');

tests/PacryptTest.php

@@ -1,9 +1,7 @@
 <?php
 
-class PaCryptTest extends \PHPUnit\Framework\TestCase
-{
-    public function testMd5Crypt()
-    {
+class PaCryptTest extends \PHPUnit\Framework\TestCase {
+    public function testMd5Crypt() {
         $hash = _pacrypt_md5crypt('test', '');
 
         $this->assertNotEmpty($hash);
@@ -12,8 +10,7 @@ class PaCryptTest extends \PHPUnit\Framework\TestCase
         $this->assertEquals($hash, _pacrypt_md5crypt('test', $hash));
     }
 
-    public function testCrypt()
-    {
+    public function testCrypt() {
 
         // E_NOTICE if we pass in '' for the salt
         $hash = _pacrypt_crypt('test', 'sa');
@@ -24,8 +21,7 @@ class PaCryptTest extends \PHPUnit\Framework\TestCase
         $this->assertEquals($hash, _pacrypt_crypt('test', $hash));
     }
 
-    public function testMySQLEncrypt()
-    {
+    public function testMySQLEncrypt() {
         if (!db_mysql()) {
             $this->markTestSkipped('Not using MySQL');
         }
@@ -49,8 +45,7 @@ class PaCryptTest extends \PHPUnit\Framework\TestCase
         );
     }
 
-    public function testAuthlib()
-    {
+    public function testAuthlib() {
         global $CONF;
 
         // too many options!
@@ -71,8 +66,7 @@ class PaCryptTest extends \PHPUnit\Framework\TestCase
         }
     }
 
-    public function testPacryptDovecot()
-    {
+    public function testPacryptDovecot() {
         global $CONF;
         if (!file_exists('/usr/bin/doveadm')) {
             $this->markTestSkipped("No /usr/bin/doveadm");
@@ -88,8 +82,9 @@ class PaCryptTest extends \PHPUnit\Framework\TestCase
         $this->assertEquals($expected_hash, _pacrypt_dovecot('test', $expected_hash));
     }
 
-    public function testPhpCrypt()
-    {
+    public function testPhpCrypt() {
+        global $CONF;
+
         $config = Config::getInstance();
         Config::write('encrypt', 'php_crypt:MD5');
 
@@ -104,44 +99,11 @@ class PaCryptTest extends \PHPUnit\Framework\TestCase
 
         $fail = _pacrypt_php_crypt('bar', $expected);
 
-    }
-
-    public function testPhpCryptHandlesPrefixAndOrRounds()
-    {
-        // try with 1000 rounds
-        Config::write('encrypt', 'php_crypt:SHA256:1000');
-        $password = 'hello';
-
-        $randomHash = '$5$VhqhhsXJtPFeBX9e$kz3/CMIEu80bKdtDAcISIrDfdwtc.ilR68Vb3hNhu/7';
-        $randomHashWithPrefix = '{SHA256-CRYPT}' . $randomHash;
-
-        $new = _pacrypt_php_crypt($password, '');
-
-        $this->assertNotEquals($randomHash, $new); // salts should be different.
-
-        $enc = _pacrypt_php_crypt($password, $randomHash);
-        $this->assertEquals($enc, $randomHash);
-
-        $this->assertEquals($randomHash, _pacrypt_php_crypt("hello", $randomHash));
-        $this->assertEquals($randomHash, _pacrypt_crypt("hello", $randomHash));
-
-        Config::write('encrypt', 'php_crypt:SHA256::{SHA256-CRYPT}');
-
-        $enc = _pacrypt_php_crypt("hello", $randomHash);
-        $this->assertEquals($randomHash, $enc); // we passed in something lacking the prefix, so we shouldn't have added it in.
-        $this->assertTrue(hash_equals($randomHash, $enc));
-
-        // should cope with this :
-        $enc = _pacrypt_php_crypt($password, '');
-
-        $this->assertEquals($enc, _pacrypt_php_crypt($password, $enc));
 
-        $this->assertRegExp('/^\{SHA256-CRYPT\}/', $enc);
-        $this->assertGreaterThan(20, strlen($enc));
+        $this->assertNotEquals($fail, $expected);
     }
 
-    public function testPhpCryptRandomString()
-    {
+    public function testPhpCryptRandomString() {
         $str1 = _php_crypt_random_string('abcdefg123456789', 2);
         $str2 = _php_crypt_random_string('abcdefg123456789', 2);
         $str3 = _php_crypt_random_string('abcdefg123456789', 2);
@@ -152,11 +114,10 @@ class PaCryptTest extends \PHPUnit\Framework\TestCase
 
         // it should be difficult for us to get three salts of the same value back...
         // not impossible though.
-        $this->assertFalse(strcmp($str1, $str2) == 0 && strcmp($str1, $str3) == 0);
+        $this->assertFalse( strcmp($str1, $str2) == 0 && strcmp($str1, $str3) == 0 );
     }
 
-    public function testSha512B64()
-    {
+    public function testSha512B64() {
         $str1 = _pacrypt_sha512_b64('test', '');
         $str2 = _pacrypt_sha512_b64('test', '');
 
@@ -177,6 +138,6 @@ class PaCryptTest extends \PHPUnit\Framework\TestCase
 
         $this->assertFalse(hash_equals('test', $str3));
 
-        $this->assertTrue(hash_equals(_pacrypt_sha512_b64('foo', $str3), $str3));
+        $this->assertTrue(hash_equals(_pacrypt_sha512_b64('foo',$str3), $str3));
     }
 }

tests/RemoteAliasTest.php

@@ -13,17 +13,6 @@ class RemoteAliasTest extends RemoteTest {
         global $CONF;
     }
 
-
-    /**
-     * Adds the test recipient data to the database.
-     */
-    public function setUp() {
-        parent::setUp();
-    }
-    public function tearDown() {
-        parent::tearDown();
-    }
-
     public function testGet() {
         /* although we created an alias record, for users, this isn't returned... */
         $this->assertEquals($this->alias->get(), array());

tests/RemoteTest.php

@@ -12,7 +12,7 @@ abstract class RemoteTest extends \PHPUnit\Framework\TestCase {
 
     protected $xmlrpc_client;
 
-    public function setUp() {
+    public function setUp() : void {
         parent::setUp();
 
         if ($this->server_url == 'http://change.me/to/work') {

tests/RemoteVacationTest.php

@@ -13,7 +13,7 @@ class RemoteVacationTest extends RemoteTest {
     /**
      * Adds the test recipient data to the database.
      */
-    public function setUp() {
+    public function setUp() : void {
         // Ensure config.inc.php is vaguely correct.
         global $CONF;
         if ($CONF['vacation'] != 'YES' || $CONF['vacation_control'] != "YES") {
@@ -36,7 +36,7 @@ class RemoteVacationTest extends RemoteTest {
 
     public function testGetDetails() {
         $details = $this->vacation->getDetails();
-        $this->assertFalse($details); // empty by default (thansk to tearDown/setUp);
+        $this->assertFalse($details); // empty by default (thanks to tearDown/setUp);
     }
 
     public function testSetAway() {

tests/bootstrap.php

@@ -22,7 +22,7 @@ if (getenv('DATABASE') == 'sqlite' || getenv('DATABASE') == false) {
     }
     touch($db_file);
 
-    echo "Using: SQLite database for tests - $db_file \n";
+    error_log("Using: SQLite database for tests - $db_file");
 }
 if (getenv('DATABASE') == 'postgresql') {
     $user = getenv('PGUSER') ?: 'postgres';
@@ -40,7 +40,7 @@ if (getenv('DATABASE') == 'postgresql') {
     Config::write('database_name', 'postfixadmin');
     Config::write('database_host', $host);
 
-    echo "Using: PostgreSQL database for tests\n";
+    error_log("Using: PostgreSQL database for tests\n");
 }
 
 if (getenv('DATABASE') == 'mysql') {
@@ -76,7 +76,7 @@ if (getenv('DATABASE') == 'mysql') {
     Config::write('database_password', $config['password']);
     Config::write('database_name', 'postfixadmin');
 
-    echo "Using: MySQL database for tests\n";
+    error_log("Using: MySQL database for tests");
 }
 
 try {