From 2b51c814fabed11dc8b5719153f9edf075cc3b0a Mon Sep 17 00:00:00 2001 From: Simon Leary Date: Fri, 19 Sep 2025 17:23:48 -0400 Subject: [PATCH 1/5] setup arrayGet --- resources/autoload.php | 2 + resources/lib/UnitySite.php | 32 +++++++++++++++- .../lib/exceptions/ArrayKeyException.php | 7 ++++ test/functional/SSHKeyAddTest.php | 2 +- test/phpunit-bootstrap.php | 1 + test/unit/UnitySiteTest.php | 38 +++++++++---------- webroot/panel/account.php | 13 +++---- 7 files changed, 66 insertions(+), 29 deletions(-) create mode 100644 resources/lib/exceptions/ArrayKeyException.php diff --git a/resources/autoload.php b/resources/autoload.php index 2eb533a1..bec10073 100644 --- a/resources/autoload.php +++ b/resources/autoload.php @@ -24,7 +24,9 @@ require_once __DIR__ . "/lib/UnityWebhook.php"; require_once __DIR__ . "/lib/UnityRedis.php"; require_once __DIR__ . "/lib/UnityGithub.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..1c9f21ca 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,20 +139,47 @@ 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 arrayGet($array, ...$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)); + throw new ArrayKeyException( + "key not found: \$array" . + // [1, 2, "foo"] => [1][2]["foo"] + implode("", array_map(fn($x) => json_encode([$x]), $keysTraversed)) + ); } $cursor = $cursor[$key]; } return $cursor; } + public static function getPostData(...$keys) + { + try { + return self::arrayGet($_POST, ...$keys); + } catch (ArrayKeyException $e) { + self::badRequest(strval($e)); + } + } + + public static function getUploadedFileContents($filename, $do_delete_tmpfile_after_read = true) + { + try { + $tmpfile_path = self::arrayGet($_FILES, $filename, "tmp_name"); + } catch (ArrayKeyException $e) { + self::badRequest(strval($e)); + } + $contents = file_get_contents($tmpfile_path); + if ($do_delete_tmpfile_after_read) { + unlink($tmpfile_path); + } + return $contents; + } + // in firefox, the user can disable alert/confirm/prompt after the 2nd or 3rd popup // after I disable alerts, if I quit and reopen my browser, the alerts come back public static function alert(string $message) 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 @@ + "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..d0c00265 100644 --- a/test/phpunit-bootstrap.php +++ b/test/phpunit-bootstrap.php @@ -17,6 +17,7 @@ require_once __DIR__ . "/../resources/lib/UnityGithub.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..4dd6d2f9 100644 --- a/test/unit/UnitySiteTest.php +++ b/test/unit/UnitySiteTest.php @@ -4,7 +4,7 @@ use PHPUnit\Framework\TestCase; use PHPUnit\Framework\Attributes\DataProvider; -use UnityWebPortal\lib\exceptions\NoDieException; +use UnityWebPortal\lib\exceptions\ArrayKeyException; // use PHPUnit\Framework\Attributes\BackupGlobals; // use PHPUnit\Framework\Attributes\RunTestsInSeparateProcess; @@ -81,7 +81,7 @@ public function testTestValidSSHKey(bool $expected, string $key) $this->assertEquals($expected, UnitySite::testValidSSHKey($key)); } - public function testArrayGetOrBadRequestReturnsValueWhenKeyExists() + public function testArrayGetReturnsValueWhenKeyExists() { $array = [ "a" => [ @@ -90,50 +90,50 @@ public function testArrayGetOrBadRequestReturnsValueWhenKeyExists() ] ] ]; - $result = UnitySite::arrayGetOrBadRequest($array, "a", "b", "c"); + $result = UnitySite::arrayGet($array, "a", "b", "c"); $this->assertSame(123, $result); } - public function testArrayGetOrBadRequestReturnsArrayWhenTraversingPartially() + public function testArrayGetReturnsArrayWhenTraversingPartially() { $array = [ "foo" => [ "bar" => "baz" ] ]; - $result = UnitySite::arrayGetOrBadRequest($array, "foo"); + $result = UnitySite::arrayGet($array, "foo"); $this->assertSame(["bar" => "baz"], $result); } - public function testArrayGetOrBadRequestThrowsOnMissingKeyFirstLevel() + public function testArrayGetThrowsOnMissingKeyFirstLevel() { $array = ["x" => 1]; - $this->expectException(NoDieException::class); - $this->expectExceptionMessage('["y"]'); - UnitySite::arrayGetOrBadRequest($array, "y"); + $this->expectException(ArrayKeyException::class); + $this->expectExceptionMessage('$array["y"]'); + UnitySite::arrayGet($array, "y"); } - public function testArrayGetOrBadRequestThrowsOnMissingKeyNested() + public function testArrayGetThrowsOnMissingKeyNested() { $array = ["a" => []]; - $this->expectException(NoDieException::class); + $this->expectException(ArrayKeyException::class); // Should include both levels - $this->expectExceptionMessage('["a","b"]'); - UnitySite::arrayGetOrBadRequest($array, "a", "b"); + $this->expectExceptionMessage('$array["a"]["b"]'); + UnitySite::arrayGet($array, "a", "b"); } - public function testArrayGetOrBadRequestThrowsWhenValueIsNullButKeyNotSet() + public function testArrayGetThrowsWhenValueIsNullButKeyNotSet() { $array = ["a" => null]; - $this->expectException(NoDieException::class); - $this->expectExceptionMessage('["a"]'); - UnitySite::arrayGetOrBadRequest($array, "a"); + $this->expectException(ArrayKeyException::class); + $this->expectExceptionMessage('$array["a"]'); + UnitySite::arrayGet($array, "a"); } - public function testArrayGetOrBadRequestReturnsValueWhenValueIsFalsyButSet() + public function testArrayGetReturnsValueWhenValueIsFalsyButSet() { $array = ["a" => 0]; - $result = UnitySite::arrayGetOrBadRequest($array, "a"); + $result = UnitySite::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; From 6894b4150fb5a3f9d8695cf654a1f13553acbf9b Mon Sep 17 00:00:00 2001 From: Simon Leary Date: Fri, 19 Sep 2025 17:31:10 -0400 Subject: [PATCH 2/5] move arrayGet from UnitySite to utils --- resources/autoload.php | 1 + resources/lib/UnitySite.php | 22 +---------- resources/lib/utils.php | 21 ++++++++++ test/phpunit-bootstrap.php | 1 + test/unit/UnitySiteTest.php | 79 ------------------------------------- test/unit/UtilsTest.php | 63 +++++++++++++++++++++++++++++ 6 files changed, 88 insertions(+), 99 deletions(-) create mode 100644 resources/lib/utils.php create mode 100644 test/unit/UtilsTest.php diff --git a/resources/autoload.php b/resources/autoload.php index bec10073..48965f77 100644 --- a/resources/autoload.php +++ b/resources/autoload.php @@ -24,6 +24,7 @@ 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"; diff --git a/resources/lib/UnitySite.php b/resources/lib/UnitySite.php index 1c9f21ca..b3a65a78 100644 --- a/resources/lib/UnitySite.php +++ b/resources/lib/UnitySite.php @@ -139,28 +139,10 @@ public static function shutdown() self::internalServerError("An internal server error has occurred.", data: ["error" => $e]); } - public static function arrayGet($array, ...$keys) - { - $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) => json_encode([$x]), $keysTraversed)) - ); - } - $cursor = $cursor[$key]; - } - return $cursor; - } - public static function getPostData(...$keys) { try { - return self::arrayGet($_POST, ...$keys); + return \arrayGet($_POST, ...$keys); } catch (ArrayKeyException $e) { self::badRequest(strval($e)); } @@ -169,7 +151,7 @@ public static function getPostData(...$keys) public static function getUploadedFileContents($filename, $do_delete_tmpfile_after_read = true) { try { - $tmpfile_path = self::arrayGet($_FILES, $filename, "tmp_name"); + $tmpfile_path = \arrayGet($_FILES, $filename, "tmp_name"); } catch (ArrayKeyException $e) { self::badRequest(strval($e)); } diff --git a/resources/lib/utils.php b/resources/lib/utils.php new file mode 100644 index 00000000..2243317a --- /dev/null +++ b/resources/lib/utils.php @@ -0,0 +1,21 @@ + [1][2]["foo"] + implode("", array_map(fn($x) => json_encode([$x]), $keysTraversed)) + ); + } + $cursor = $cursor[$key]; + } + return $cursor; +} diff --git a/test/phpunit-bootstrap.php b/test/phpunit-bootstrap.php index d0c00265..a9acc17f 100644 --- a/test/phpunit-bootstrap.php +++ b/test/phpunit-bootstrap.php @@ -15,6 +15,7 @@ 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"; diff --git a/test/unit/UnitySiteTest.php b/test/unit/UnitySiteTest.php index 4dd6d2f9..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\ArrayKeyException; // 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 testArrayGetReturnsValueWhenKeyExists() - { - $array = [ - "a" => [ - "b" => [ - "c" => 123 - ] - ] - ]; - $result = UnitySite::arrayGet($array, "a", "b", "c"); - $this->assertSame(123, $result); - } - - public function testArrayGetReturnsArrayWhenTraversingPartially() - { - $array = [ - "foo" => [ - "bar" => "baz" - ] - ]; - $result = UnitySite::arrayGet($array, "foo"); - $this->assertSame(["bar" => "baz"], $result); - } - - public function testArrayGetThrowsOnMissingKeyFirstLevel() - { - $array = ["x" => 1]; - $this->expectException(ArrayKeyException::class); - $this->expectExceptionMessage('$array["y"]'); - UnitySite::arrayGet($array, "y"); - } - - public function testArrayGetThrowsOnMissingKeyNested() - { - $array = ["a" => []]; - $this->expectException(ArrayKeyException::class); - // Should include both levels - $this->expectExceptionMessage('$array["a"]["b"]'); - UnitySite::arrayGet($array, "a", "b"); - } - - public function testArrayGetThrowsWhenValueIsNullButKeyNotSet() - { - $array = ["a" => null]; - $this->expectException(ArrayKeyException::class); - $this->expectExceptionMessage('$array["a"]'); - UnitySite::arrayGet($array, "a"); - } - - public function testArrayGetReturnsValueWhenValueIsFalsyButSet() - { - $array = ["a" => 0]; - $result = UnitySite::arrayGet($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); + } +} From 10b0b7fa6b59170b6289171faf28020c8203d4c5 Mon Sep 17 00:00:00 2001 From: simonLeary42 <71396965+simonLeary42@users.noreply.github.com> Date: Fri, 19 Sep 2025 17:36:12 -0400 Subject: [PATCH 3/5] Update resources/lib/UnitySite.php Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- resources/lib/UnitySite.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/resources/lib/UnitySite.php b/resources/lib/UnitySite.php index b3a65a78..c00cda91 100644 --- a/resources/lib/UnitySite.php +++ b/resources/lib/UnitySite.php @@ -156,6 +156,9 @@ public static function getUploadedFileContents($filename, $do_delete_tmpfile_aft self::badRequest(strval($e)); } $contents = file_get_contents($tmpfile_path); + if ($contents === false) { + self::badRequest("Failed to read uploaded file: " . $tmpfile_path); + } if ($do_delete_tmpfile_after_read) { unlink($tmpfile_path); } From 87159a897ede05224baeec13e6933e452d37b3c5 Mon Sep 17 00:00:00 2001 From: Simon Leary Date: Fri, 19 Sep 2025 17:37:18 -0400 Subject: [PATCH 4/5] fail to read file is not bad request --- resources/lib/UnitySite.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/lib/UnitySite.php b/resources/lib/UnitySite.php index c00cda91..f36592d1 100644 --- a/resources/lib/UnitySite.php +++ b/resources/lib/UnitySite.php @@ -157,7 +157,7 @@ public static function getUploadedFileContents($filename, $do_delete_tmpfile_aft } $contents = file_get_contents($tmpfile_path); if ($contents === false) { - self::badRequest("Failed to read uploaded file: " . $tmpfile_path); + throw new \Exception("Failed to read file: " . $tmpfile_path); } if ($do_delete_tmpfile_after_read) { unlink($tmpfile_path); From 56b8e12f71df0e7a910c34fef27b945d1aabff95 Mon Sep 17 00:00:00 2001 From: Simon Leary Date: Tue, 23 Sep 2025 08:20:38 -0400 Subject: [PATCH 5/5] take advantage of new badRequest --- resources/lib/UnitySite.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/resources/lib/UnitySite.php b/resources/lib/UnitySite.php index f36592d1..6f9972b4 100644 --- a/resources/lib/UnitySite.php +++ b/resources/lib/UnitySite.php @@ -144,7 +144,7 @@ public static function getPostData(...$keys) try { return \arrayGet($_POST, ...$keys); } catch (ArrayKeyException $e) { - self::badRequest(strval($e)); + self::badRequest('failed to get $_POST data', $e, ['$_POST' => $_POST]); } } @@ -153,7 +153,7 @@ public static function getUploadedFileContents($filename, $do_delete_tmpfile_aft try { $tmpfile_path = \arrayGet($_FILES, $filename, "tmp_name"); } catch (ArrayKeyException $e) { - self::badRequest(strval($e)); + self::badRequest('no such uploaded file', $e, ['$_FILES' => $_FILES]); } $contents = file_get_contents($tmpfile_path); if ($contents === false) {