Skip to content

Commit dea93a2

Browse files
general purpose function for strictly fetching array keys (#311)
* setup arrayGet * move arrayGet from UnitySite to utils * Update resources/lib/UnitySite.php Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fail to read file is not bad request * take advantage of new badRequest --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 73ea4c4 commit dea93a2

File tree

9 files changed

+126
-97
lines changed

9 files changed

+126
-97
lines changed

resources/autoload.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,10 @@
2424
require_once __DIR__ . "/lib/UnityWebhook.php";
2525
require_once __DIR__ . "/lib/UnityRedis.php";
2626
require_once __DIR__ . "/lib/UnityGithub.php";
27+
require_once __DIR__ . "/lib/utils.php";
28+
require_once __DIR__ . "/lib/exceptions/NoDieException.php";
2729
require_once __DIR__ . "/lib/exceptions/SSOException.php";
30+
require_once __DIR__ . "/lib/exceptions/ArrayKeyException.php";
2831

2932
require_once __DIR__ . "/config.php";
3033
require __DIR__ . "/init.php";

resources/lib/UnitySite.php

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use phpseclib3\Crypt\PublicKeyLoader;
66
use UnityWebPortal\lib\exceptions\NoDieException;
7+
use UnityWebPortal\lib\exceptions\ArrayKeyException;
78

89
class UnitySite
910
{
@@ -138,18 +139,30 @@ public static function shutdown()
138139
self::internalServerError("An internal server error has occurred.", data: ["error" => $e]);
139140
}
140141

141-
public static function arrayGetOrBadRequest(array $array, ...$keys)
142+
public static function getPostData(...$keys)
142143
{
143-
$cursor = $array;
144-
$keysTraversed = [];
145-
foreach ($keys as $key) {
146-
array_push($keysTraversed, $key);
147-
if (!isset($cursor[$key])) {
148-
self::badRequest("array key not found: " . json_encode($keysTraversed));
149-
}
150-
$cursor = $cursor[$key];
144+
try {
145+
return \arrayGet($_POST, ...$keys);
146+
} catch (ArrayKeyException $e) {
147+
self::badRequest('failed to get $_POST data', $e, ['$_POST' => $_POST]);
148+
}
149+
}
150+
151+
public static function getUploadedFileContents($filename, $do_delete_tmpfile_after_read = true)
152+
{
153+
try {
154+
$tmpfile_path = \arrayGet($_FILES, $filename, "tmp_name");
155+
} catch (ArrayKeyException $e) {
156+
self::badRequest('no such uploaded file', $e, ['$_FILES' => $_FILES]);
157+
}
158+
$contents = file_get_contents($tmpfile_path);
159+
if ($contents === false) {
160+
throw new \Exception("Failed to read file: " . $tmpfile_path);
161+
}
162+
if ($do_delete_tmpfile_after_read) {
163+
unlink($tmpfile_path);
151164
}
152-
return $cursor;
165+
return $contents;
153166
}
154167

155168
// in firefox, the user can disable alert/confirm/prompt after the 2nd or 3rd popup
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<?php
2+
3+
namespace UnityWebPortal\lib\exceptions;
4+
5+
class ArrayKeyException extends \Exception
6+
{
7+
}

resources/lib/utils.php

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<?php
2+
3+
use UnityWebPortal\lib\exceptions\ArrayKeyException;
4+
5+
function arrayGet($array, ...$keys)
6+
{
7+
$cursor = $array;
8+
$keysTraversed = [];
9+
foreach ($keys as $key) {
10+
array_push($keysTraversed, $key);
11+
if (!isset($cursor[$key])) {
12+
throw new ArrayKeyException(
13+
"key not found: \$array" .
14+
// [1, 2, "foo"] => [1][2]["foo"]
15+
implode("", array_map(fn($x) => json_encode([$x]), $keysTraversed))
16+
);
17+
}
18+
$cursor = $cursor[$key];
19+
}
20+
return $cursor;
21+
}

test/functional/SSHKeyAddTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ private function addSshKeysImport(array $keys): void {
3131
__DIR__ . "/../../webroot/panel/account.php",
3232
["form_type" => "addKey", "add_type" => "import"]
3333
);
34+
$this->assertFalse(file_exists($tmp_path));
3435
} finally {
35-
unlink($tmp_path);
3636
unset($_FILES["keyfile"]);
3737
}
3838
}

test/phpunit-bootstrap.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,10 @@
1515
require_once __DIR__ . "/../resources/lib/UnityWebhook.php";
1616
require_once __DIR__ . "/../resources/lib/UnityRedis.php";
1717
require_once __DIR__ . "/../resources/lib/UnityGithub.php";
18+
require_once __DIR__ . "/../resources/lib/utils.php";
1819
require_once __DIR__ . "/../resources/lib/exceptions/NoDieException.php";
1920
require_once __DIR__ . "/../resources/lib/exceptions/SSOException.php";
21+
require_once __DIR__ . "/../resources/lib/exceptions/ArrayKeyException.php";
2022

2123
$_SERVER["HTTP_HOST"] = "phpunit"; // used for config override
2224
require_once __DIR__ . "/../resources/config.php";

test/unit/UnitySiteTest.php

Lines changed: 0 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
use PHPUnit\Framework\TestCase;
66
use PHPUnit\Framework\Attributes\DataProvider;
7-
use UnityWebPortal\lib\exceptions\NoDieException;
87
// use PHPUnit\Framework\Attributes\BackupGlobals;
98
// use PHPUnit\Framework\Attributes\RunTestsInSeparateProcess;
109

@@ -80,82 +79,4 @@ public function testTestValidSSHKey(bool $expected, string $key)
8079
{
8180
$this->assertEquals($expected, UnitySite::testValidSSHKey($key));
8281
}
83-
84-
public function testArrayGetOrBadRequestReturnsValueWhenKeyExists()
85-
{
86-
$array = [
87-
"a" => [
88-
"b" => [
89-
"c" => 123
90-
]
91-
]
92-
];
93-
$result = UnitySite::arrayGetOrBadRequest($array, "a", "b", "c");
94-
$this->assertSame(123, $result);
95-
}
96-
97-
public function testArrayGetOrBadRequestReturnsArrayWhenTraversingPartially()
98-
{
99-
$array = [
100-
"foo" => [
101-
"bar" => "baz"
102-
]
103-
];
104-
$result = UnitySite::arrayGetOrBadRequest($array, "foo");
105-
$this->assertSame(["bar" => "baz"], $result);
106-
}
107-
108-
public function testArrayGetOrBadRequestThrowsOnMissingKeyFirstLevel()
109-
{
110-
$array = ["x" => 1];
111-
$this->expectException(NoDieException::class);
112-
$this->expectExceptionMessage('["y"]');
113-
UnitySite::arrayGetOrBadRequest($array, "y");
114-
}
115-
116-
public function testArrayGetOrBadRequestThrowsOnMissingKeyNested()
117-
{
118-
$array = ["a" => []];
119-
$this->expectException(NoDieException::class);
120-
// Should include both levels
121-
$this->expectExceptionMessage('["a","b"]');
122-
UnitySite::arrayGetOrBadRequest($array, "a", "b");
123-
}
124-
125-
public function testArrayGetOrBadRequestThrowsWhenValueIsNullButKeyNotSet()
126-
{
127-
$array = ["a" => null];
128-
$this->expectException(NoDieException::class);
129-
$this->expectExceptionMessage('["a"]');
130-
UnitySite::arrayGetOrBadRequest($array, "a");
131-
}
132-
133-
public function testArrayGetOrBadRequestReturnsValueWhenValueIsFalsyButSet()
134-
{
135-
$array = ["a" => 0];
136-
$result = UnitySite::arrayGetOrBadRequest($array, "a");
137-
$this->assertSame(0, $result);
138-
}
139-
140-
// I suspect that this test could have unexpected interactions with other tests.
141-
// even with RunTestsInSeparateProcess and BackupGlobalState, http_response_code()
142-
// still persists to the next test. header("HTTP/1.1 false") puts it back to its
143-
// initial value, but this is a hack and does not inspire confidence.
144-
// #[BackupGlobals(true)]
145-
// #[RunTestsInSeparateProcess]
146-
// public function testHeaderResponseCode()
147-
// {
148-
// $this->assertEquals(false, http_response_code());
149-
// $this->assertArrayNotHasKey("SERVER_PROTOCOL", $_SERVER);
150-
// try {
151-
// $_SERVER["SERVER_PROTOCOL"] = "HTTP/1.1";
152-
// UnitySite::headerResponseCode(400);
153-
// $this->assertEquals(400, http_response_code());
154-
// UnitySite::headerResponseCode(401);
155-
// $this->assertEquals(401, http_response_code());
156-
// } finally {
157-
// unset($_SERVER["SERVER_PROTOCOL"]);
158-
// header("HTTP/1.1 false");
159-
// }
160-
// }
16182
}

test/unit/UtilsTest.php

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
<?php
2+
3+
use UnityWebPortal\lib\exceptions\ArrayKeyException;
4+
use PHPUnit\Framework\TestCase;
5+
6+
class UtilsTest extends TestCase
7+
{
8+
public function testArrayGetReturnsValueWhenKeyExists()
9+
{
10+
$array = [
11+
"a" => [
12+
"b" => [
13+
"c" => 123
14+
]
15+
]
16+
];
17+
$result = \arrayGet($array, "a", "b", "c");
18+
$this->assertSame(123, $result);
19+
}
20+
21+
public function testArrayGetReturnsArrayWhenTraversingPartially()
22+
{
23+
$array = [
24+
"foo" => [
25+
"bar" => "baz"
26+
]
27+
];
28+
$result = \arrayGet($array, "foo");
29+
$this->assertSame(["bar" => "baz"], $result);
30+
}
31+
32+
public function testArrayGetThrowsOnMissingKeyFirstLevel()
33+
{
34+
$array = ["x" => 1];
35+
$this->expectException(ArrayKeyException::class);
36+
$this->expectExceptionMessage('$array["y"]');
37+
\arrayGet($array, "y");
38+
}
39+
40+
public function testArrayGetThrowsOnMissingKeyNested()
41+
{
42+
$array = ["a" => []];
43+
$this->expectException(ArrayKeyException::class);
44+
// Should include both levels
45+
$this->expectExceptionMessage('$array["a"]["b"]');
46+
\arrayGet($array, "a", "b");
47+
}
48+
49+
public function testArrayGetThrowsWhenValueIsNullButKeyNotSet()
50+
{
51+
$array = ["a" => null];
52+
$this->expectException(ArrayKeyException::class);
53+
$this->expectExceptionMessage('$array["a"]');
54+
\arrayGet($array, "a");
55+
}
56+
57+
public function testArrayGetReturnsValueWhenValueIsFalsyButSet()
58+
{
59+
$array = ["a" => 0];
60+
$result = \arrayGet($array, "a");
61+
$this->assertSame(0, $result);
62+
}
63+
}

webroot/panel/account.php

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,23 +7,22 @@
77
$hasGroups = count($USER->getPIGroupGIDs()) > 0;
88

99
if ($_SERVER['REQUEST_METHOD'] == "POST") {
10-
switch (UnitySite::arrayGetOrBadRequest($_POST, "form_type")) {
10+
switch (UnitySite::getPostData("form_type")) {
1111
case "addKey":
1212
$keys = array();
13-
switch (UnitySite::arrayGetOrBadRequest($_POST, "add_type")) {
13+
switch (UnitySite::getPostData("add_type")) {
1414
case "paste":
15-
array_push($keys, UnitySite::arrayGetOrBadRequest($_POST, "key"));
15+
array_push($keys, UnitySite::getPostData("key"));
1616
break;
1717
case "import":
18-
$keyPath = UnitySite::arrayGetOrBadRequest($_FILES, "keyfile", "tmp_name");
19-
$key = file_get_contents($keyPath);
18+
$key = UnitySite::getUploadedFileContents("keyfile");
2019
array_push($keys, $key);
2120
break;
2221
case "generate":
23-
array_push($keys, UnitySite::arrayGetOrBadRequest($_POST, "gen_key"));
22+
array_push($keys, UnitySite::getPostData("gen_key"));
2423
break;
2524
case "github":
26-
$githubUsername = UnitySite::arrayGetOrBadRequest($_POST, "gh_user");
25+
$githubUsername = UnitySite::getPostData("gh_user");
2726
$githubKeys = $GITHUB->getSshPublicKeys($githubUsername);
2827
$keys = array_merge($keys, $githubKeys);
2928
break;

0 commit comments

Comments
 (0)