diff --git a/resources/autoload.php b/resources/autoload.php index 2eb533a1..48965f77 100644 --- a/resources/autoload.php +++ b/resources/autoload.php @@ -24,7 +24,10 @@ require_once __DIR__ . "/lib/UnityWebhook.php"; require_once __DIR__ . "/lib/UnityRedis.php"; require_once __DIR__ . "/lib/UnityGithub.php"; +require_once __DIR__ . "/lib/utils.php"; +require_once __DIR__ . "/lib/exceptions/NoDieException.php"; require_once __DIR__ . "/lib/exceptions/SSOException.php"; +require_once __DIR__ . "/lib/exceptions/ArrayKeyException.php"; require_once __DIR__ . "/config.php"; require __DIR__ . "/init.php"; diff --git a/resources/lib/UnitySite.php b/resources/lib/UnitySite.php index ebc5e839..6f9972b4 100644 --- a/resources/lib/UnitySite.php +++ b/resources/lib/UnitySite.php @@ -4,6 +4,7 @@ use phpseclib3\Crypt\PublicKeyLoader; use UnityWebPortal\lib\exceptions\NoDieException; +use UnityWebPortal\lib\exceptions\ArrayKeyException; class UnitySite { @@ -138,18 +139,30 @@ public static function shutdown() self::internalServerError("An internal server error has occurred.", data: ["error" => $e]); } - public static function arrayGetOrBadRequest(array $array, ...$keys) + public static function getPostData(...$keys) { - $cursor = $array; - $keysTraversed = []; - foreach ($keys as $key) { - array_push($keysTraversed, $key); - if (!isset($cursor[$key])) { - self::badRequest("array key not found: " . json_encode($keysTraversed)); - } - $cursor = $cursor[$key]; + try { + return \arrayGet($_POST, ...$keys); + } catch (ArrayKeyException $e) { + self::badRequest('failed to get $_POST data', $e, ['$_POST' => $_POST]); + } + } + + public static function getUploadedFileContents($filename, $do_delete_tmpfile_after_read = true) + { + try { + $tmpfile_path = \arrayGet($_FILES, $filename, "tmp_name"); + } catch (ArrayKeyException $e) { + self::badRequest('no such uploaded file', $e, ['$_FILES' => $_FILES]); + } + $contents = file_get_contents($tmpfile_path); + if ($contents === false) { + throw new \Exception("Failed to read file: " . $tmpfile_path); + } + if ($do_delete_tmpfile_after_read) { + unlink($tmpfile_path); } - return $cursor; + return $contents; } // in firefox, the user can disable alert/confirm/prompt after the 2nd or 3rd popup diff --git a/resources/lib/exceptions/ArrayKeyException.php b/resources/lib/exceptions/ArrayKeyException.php new file mode 100644 index 00000000..7b591ac8 --- /dev/null +++ b/resources/lib/exceptions/ArrayKeyException.php @@ -0,0 +1,7 @@ + [1][2]["foo"] + implode("", array_map(fn($x) => json_encode([$x]), $keysTraversed)) + ); + } + $cursor = $cursor[$key]; + } + return $cursor; +} diff --git a/test/functional/SSHKeyAddTest.php b/test/functional/SSHKeyAddTest.php index 78f64bc9..bd4abebb 100644 --- a/test/functional/SSHKeyAddTest.php +++ b/test/functional/SSHKeyAddTest.php @@ -31,8 +31,8 @@ private function addSshKeysImport(array $keys): void { __DIR__ . "/../../webroot/panel/account.php", ["form_type" => "addKey", "add_type" => "import"] ); + $this->assertFalse(file_exists($tmp_path)); } finally { - unlink($tmp_path); unset($_FILES["keyfile"]); } } diff --git a/test/phpunit-bootstrap.php b/test/phpunit-bootstrap.php index 262f5e05..a9acc17f 100644 --- a/test/phpunit-bootstrap.php +++ b/test/phpunit-bootstrap.php @@ -15,8 +15,10 @@ require_once __DIR__ . "/../resources/lib/UnityWebhook.php"; require_once __DIR__ . "/../resources/lib/UnityRedis.php"; require_once __DIR__ . "/../resources/lib/UnityGithub.php"; +require_once __DIR__ . "/../resources/lib/utils.php"; require_once __DIR__ . "/../resources/lib/exceptions/NoDieException.php"; require_once __DIR__ . "/../resources/lib/exceptions/SSOException.php"; +require_once __DIR__ . "/../resources/lib/exceptions/ArrayKeyException.php"; $_SERVER["HTTP_HOST"] = "phpunit"; // used for config override require_once __DIR__ . "/../resources/config.php"; diff --git a/test/unit/UnitySiteTest.php b/test/unit/UnitySiteTest.php index 91fbc346..1abaf43c 100644 --- a/test/unit/UnitySiteTest.php +++ b/test/unit/UnitySiteTest.php @@ -4,7 +4,6 @@ use PHPUnit\Framework\TestCase; use PHPUnit\Framework\Attributes\DataProvider; -use UnityWebPortal\lib\exceptions\NoDieException; // use PHPUnit\Framework\Attributes\BackupGlobals; // use PHPUnit\Framework\Attributes\RunTestsInSeparateProcess; @@ -80,82 +79,4 @@ public function testTestValidSSHKey(bool $expected, string $key) { $this->assertEquals($expected, UnitySite::testValidSSHKey($key)); } - - public function testArrayGetOrBadRequestReturnsValueWhenKeyExists() - { - $array = [ - "a" => [ - "b" => [ - "c" => 123 - ] - ] - ]; - $result = UnitySite::arrayGetOrBadRequest($array, "a", "b", "c"); - $this->assertSame(123, $result); - } - - public function testArrayGetOrBadRequestReturnsArrayWhenTraversingPartially() - { - $array = [ - "foo" => [ - "bar" => "baz" - ] - ]; - $result = UnitySite::arrayGetOrBadRequest($array, "foo"); - $this->assertSame(["bar" => "baz"], $result); - } - - public function testArrayGetOrBadRequestThrowsOnMissingKeyFirstLevel() - { - $array = ["x" => 1]; - $this->expectException(NoDieException::class); - $this->expectExceptionMessage('["y"]'); - UnitySite::arrayGetOrBadRequest($array, "y"); - } - - public function testArrayGetOrBadRequestThrowsOnMissingKeyNested() - { - $array = ["a" => []]; - $this->expectException(NoDieException::class); - // Should include both levels - $this->expectExceptionMessage('["a","b"]'); - UnitySite::arrayGetOrBadRequest($array, "a", "b"); - } - - public function testArrayGetOrBadRequestThrowsWhenValueIsNullButKeyNotSet() - { - $array = ["a" => null]; - $this->expectException(NoDieException::class); - $this->expectExceptionMessage('["a"]'); - UnitySite::arrayGetOrBadRequest($array, "a"); - } - - public function testArrayGetOrBadRequestReturnsValueWhenValueIsFalsyButSet() - { - $array = ["a" => 0]; - $result = UnitySite::arrayGetOrBadRequest($array, "a"); - $this->assertSame(0, $result); - } - - // I suspect that this test could have unexpected interactions with other tests. - // even with RunTestsInSeparateProcess and BackupGlobalState, http_response_code() - // still persists to the next test. header("HTTP/1.1 false") puts it back to its - // initial value, but this is a hack and does not inspire confidence. - // #[BackupGlobals(true)] - // #[RunTestsInSeparateProcess] - // public function testHeaderResponseCode() - // { - // $this->assertEquals(false, http_response_code()); - // $this->assertArrayNotHasKey("SERVER_PROTOCOL", $_SERVER); - // try { - // $_SERVER["SERVER_PROTOCOL"] = "HTTP/1.1"; - // UnitySite::headerResponseCode(400); - // $this->assertEquals(400, http_response_code()); - // UnitySite::headerResponseCode(401); - // $this->assertEquals(401, http_response_code()); - // } finally { - // unset($_SERVER["SERVER_PROTOCOL"]); - // header("HTTP/1.1 false"); - // } - // } } diff --git a/test/unit/UtilsTest.php b/test/unit/UtilsTest.php new file mode 100644 index 00000000..e2914ea3 --- /dev/null +++ b/test/unit/UtilsTest.php @@ -0,0 +1,63 @@ + [ + "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); + } +} diff --git a/webroot/panel/account.php b/webroot/panel/account.php index d8a51c93..2f6719f1 100644 --- a/webroot/panel/account.php +++ b/webroot/panel/account.php @@ -7,23 +7,22 @@ $hasGroups = count($USER->getPIGroupGIDs()) > 0; if ($_SERVER['REQUEST_METHOD'] == "POST") { - switch (UnitySite::arrayGetOrBadRequest($_POST, "form_type")) { + switch (UnitySite::getPostData("form_type")) { case "addKey": $keys = array(); - switch (UnitySite::arrayGetOrBadRequest($_POST, "add_type")) { + switch (UnitySite::getPostData("add_type")) { case "paste": - array_push($keys, UnitySite::arrayGetOrBadRequest($_POST, "key")); + array_push($keys, UnitySite::getPostData("key")); break; case "import": - $keyPath = UnitySite::arrayGetOrBadRequest($_FILES, "keyfile", "tmp_name"); - $key = file_get_contents($keyPath); + $key = UnitySite::getUploadedFileContents("keyfile"); array_push($keys, $key); break; case "generate": - array_push($keys, UnitySite::arrayGetOrBadRequest($_POST, "gen_key")); + array_push($keys, UnitySite::getPostData("gen_key")); break; case "github": - $githubUsername = UnitySite::arrayGetOrBadRequest($_POST, "gh_user"); + $githubUsername = UnitySite::getPostData("gh_user"); $githubKeys = $GITHUB->getSshPublicKeys($githubUsername); $keys = array_merge($keys, $githubKeys); break;