setup error handler to make (undefined array key) fatal (#356)

Author: simonLeary42

defaults/config.ini.default

@@ -17,6 +17,7 @@ allow_die = true ; internal use only
 enable_verbose_error_log = true ; internal use only
 enable_error_to_user = true ; internal use only
 enable_exception_handler = true ; internal use only
+enable_error_handler = true ; internal use only
 
 [ldap]
 uri = "ldap://identity"  ; URI of remote LDAP server

deployment/overrides/phpunit/config/config.ini

@@ -6,3 +6,4 @@ allow_die = false
 enable_verbose_error_log = false
 enable_error_to_user = false
 enable_exception_handler = false
+enable_error_handler = false

resources/init.php

@@ -17,6 +17,10 @@
     set_exception_handler(["UnityWebPortal\lib\UnityHTTPD", "exceptionHandler"]);
 }
 
+if (CONFIG["site"]["enable_error_handler"]) {
+    set_error_handler(["UnityWebPortal\lib\UnityHTTPD", "errorHandler"]);
+}
+
 session_start();
 
 if (isset($GLOBALS["ldapconn"])) {

resources/lib/UnityHTTPD.php

@@ -52,17 +52,17 @@ public static function errorLog(
                 $output["data"] = "data could not be JSON encoded: " . $e->getMessage();
             }
         }
-        $output["REMOTE_USER"] = $_SERVER["REMOTE_USER"] ?? null;
-        $output["REMOTE_ADDR"] = $_SERVER["REMOTE_ADDR"] ?? null;
-        if (!is_null($errorid)) {
-            $output["errorid"] = $errorid;
-        }
         if (!is_null($error)) {
             $output["error"] = self::throwableToArray($error);
         } else {
             // newlines are bad for error log, but getTrace() is too verbose
             $output["trace"] = explode("\n", (new \Exception())->getTraceAsString());
         }
+        $output["REMOTE_USER"] = $_SERVER["REMOTE_USER"] ?? null;
+        $output["REMOTE_ADDR"] = $_SERVER["REMOTE_ADDR"] ?? null;
+        if (!is_null($errorid)) {
+            $output["errorid"] = $errorid;
+        }
         error_log("$title: " . \jsonEncode($output));
     }
 
@@ -132,6 +132,11 @@ public static function internalServerError(
         $errorid = uniqid();
         self::errorToUser("An internal server error has occurred.", 500, $errorid);
         self::errorLog("internal server error", $message, $errorid, $error, $data);
+        if (!is_null($error) && ini_get("display_errors") && ini_get("html_errors")) {
+            echo "<table>";
+            echo $error->xdebug_message;
+            echo "</table>";
+        }
         self::die($message);
     }
 
@@ -140,13 +145,20 @@ public static function exceptionHandler(\Throwable $e): void
     {
         ini_set("log_errors", true); // in case something goes wrong and error is not logged
         self::internalServerError("An internal server error has occurred.", error: $e);
-        ini_set("log_errors", false); // error logged successfully
     }
 
-    public static function getPostData(...$keys): mixed
+    public static function errorHandler(int $severity, string $message, string $file, int $line)
+    {
+        if (str_contains($message, "Undefined array key")) {
+            throw new ArrayKeyException($message);
+        }
+        return false;
+    }
+
+    public static function getPostData(string $key): mixed
     {
         try {
-            return \arrayGet($_POST, ...$keys);
+            return $_POST[$key];
         } catch (ArrayKeyException $e) {
             self::badRequest('failed to get $_POST data', $e, [
                 '$_POST' => $_POST,
@@ -160,7 +172,7 @@ public static function getUploadedFileContents(
         string $encoding = "UTF-8",
     ): string {
         try {
-            $tmpfile_path = \arrayGet($_FILES, $filename, "tmp_name");
+            $tmpfile_path = $_FILES[$filename]["tmp_name"];
         } catch (ArrayKeyException $e) {
             self::badRequest("no such uploaded file", $e, [
                 '$_FILES' => $_FILES,

resources/lib/utils.php

@@ -6,24 +6,6 @@
 use UnityWebPortal\lib\exceptions\EncodingConversionException;
 use phpseclib3\Crypt\PublicKeyLoader;
 
-function arrayGet(array $array, mixed ...$keys): mixed
-{
-    $cursor = $array;
-    $keysTraversed = [];
-    foreach ($keys as $key) {
-        array_push($keysTraversed, $key);
-        if (!isset($cursor[$key])) {
-            throw new ArrayKeyException(
-                "key not found: \$array" .
-                    // [1, 2, "foo"] => [1][2]["foo"]
-                    implode("", array_map(fn($x) => jsonEncode([$x]), $keysTraversed)),
-            );
-        }
-        $cursor = $cursor[$key];
-    }
-    return $cursor;
-}
-
 // like assert() but not subject to zend.assertions config
 function ensure(bool $condition, ?string $message = null): void
 {

test/unit/UtilsTest.php

@@ -6,62 +6,6 @@
 
 class UtilsTest extends TestCase
 {
-    public function testArrayGetReturnsValueWhenKeyExists()
-    {
-        $array = [
-            "a" => [
-                "b" => [
-                    "c" => 123,
-                ],
-            ],
-        ];
-        $result = \arrayGet($array, "a", "b", "c");
-        $this->assertSame(123, $result);
-    }
-
-    public function testArrayGetReturnsArrayWhenTraversingPartially()
-    {
-        $array = [
-            "foo" => [
-                "bar" => "baz",
-            ],
-        ];
-        $result = \arrayGet($array, "foo");
-        $this->assertSame(["bar" => "baz"], $result);
-    }
-
-    public function testArrayGetThrowsOnMissingKeyFirstLevel()
-    {
-        $array = ["x" => 1];
-        $this->expectException(ArrayKeyException::class);
-        $this->expectExceptionMessage('$array["y"]');
-        \arrayGet($array, "y");
-    }
-
-    public function testArrayGetThrowsOnMissingKeyNested()
-    {
-        $array = ["a" => []];
-        $this->expectException(ArrayKeyException::class);
-        // Should include both levels
-        $this->expectExceptionMessage('$array["a"]["b"]');
-        \arrayGet($array, "a", "b");
-    }
-
-    public function testArrayGetThrowsWhenValueIsNullButKeyNotSet()
-    {
-        $array = ["a" => null];
-        $this->expectException(ArrayKeyException::class);
-        $this->expectExceptionMessage('$array["a"]');
-        \arrayGet($array, "a");
-    }
-
-    public function testArrayGetReturnsValueWhenValueIsFalsyButSet()
-    {
-        $array = ["a" => 0];
-        $result = \arrayGet($array, "a");
-        $this->assertSame(0, $result);
-    }
-
     public static function SSHKeyProvider()
     {
         global $HTTP_HEADER_TEST_INPUTS;

tools/docker-dev/web/Dockerfile

@@ -29,7 +29,8 @@ RUN sed -i '/zend.assertions/c\zend.assertions = 1' /etc/php/8.3/cli/php.ini
 RUN sed -i '/memory_limit/c\memory_limit = -1' /etc/php/8.3/apache2/php.ini
 RUN sed -i '/zend.exception_ignore_args/c\zend.exception_ignore_args = 0' /etc/php/8.3/apache2/php.ini
 RUN sed -i '/zend.exception_ignore_args/c\zend.exception_ignore_args = 0' /etc/php/8.3/cli/php.ini
-RUN echo 'xdebug.mode=coverage' >> /etc/php/8.3/cli/php.ini
+RUN echo 'xdebug.mode=develop,coverage,debug,gcstats,profile,trace' >> /etc/php/8.3/cli/php.ini
+RUN echo 'xdebug.mode=develop,coverage,debug,gcstats,profile,trace' >> /etc/php/8.3/apache2/php.ini
 
 # Start apache2 server
 EXPOSE 80