From 8701e979c6ff45ea4d185e58847d67523df60c26 Mon Sep 17 00:00:00 2001 From: Simon Leary Date: Tue, 3 Jun 2025 13:55:46 -0400 Subject: [PATCH 1/3] catch edge cases --- resources/lib/UnityGroup.php | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/resources/lib/UnityGroup.php b/resources/lib/UnityGroup.php index 5d75c66df..2362dc1aa 100644 --- a/resources/lib/UnityGroup.php +++ b/resources/lib/UnityGroup.php @@ -40,7 +40,7 @@ public function __toString(): string public function requestGroup(bool $send_mail_to_admins, bool $send_mail = true): void { if ($this->exists()) { - return; + throw new Exception("requested to create group '$this' that already exists!"); } if ($this->SQL->accDeletionRequestExists($this->getOwner()->uid)) { return; @@ -70,7 +70,10 @@ public function approveGroup(bool $send_mail = true): void $uid = $this->getOwner()->uid; $request = $this->SQL->getRequest($uid, UnitySQL::REQUEST_BECOME_PI); if ($this->exists()) { - return; + throw new Exception("group '$this' exists"); + } + if ($this->SQL->accDeletionRequestExists($this->getOwner()->getUID())) { + throw new Exception("group owner '{$this->getOwner()->getUID()}' requested account deletion"); } \ensure($this->getOwner()->exists()); $this->init(); @@ -91,7 +94,7 @@ public function denyGroup(bool $send_mail = true): void $request = $this->SQL->getRequest($this->getOwner()->uid, UnitySQL::REQUEST_BECOME_PI); $this->SQL->removeRequest($this->getOwner()->uid, UnitySQL::REQUEST_BECOME_PI); if ($this->exists()) { - return; + throw new Exception("group '$this' creation request cannot be denied, it already exists!"); } $this->SQL->addLog("denied_group", $this->getOwner()->uid); if ($send_mail) { @@ -102,6 +105,7 @@ public function denyGroup(bool $send_mail = true): void public function cancelGroupRequest(bool $send_mail = true): void { if (!$this->SQL->requestExists($this->getOwner()->uid, UnitySQL::REQUEST_BECOME_PI)) { + UnitySite::errorLog("warning", "attempt to cancel nonexistent group creation request ($this)"); return; } $this->SQL->removeRequest($this->getOwner()->uid, UnitySQL::REQUEST_BECOME_PI); @@ -115,6 +119,7 @@ public function cancelGroupRequest(bool $send_mail = true): void public function cancelGroupJoinRequest(UnityUser $user, bool $send_mail = true): void { if (!$this->requestExists($user)) { + UnitySite::errorLog("warning", "attempt to cancel nonexistent group join request ($this)"); return; } $this->SQL->removeRequest($user->uid, $this->gid); From 3baa00009800b084f93509e1561f787e749c0a4c Mon Sep 17 00:00:00 2001 From: Simon Leary Date: Mon, 5 Jan 2026 15:32:41 -0500 Subject: [PATCH 2/3] catch edge cases --- resources/lib/UnityGroup.php | 30 +++++++++++++++++-------- test/functional/PiBecomeRequestTest.php | 3 +-- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/resources/lib/UnityGroup.php b/resources/lib/UnityGroup.php index 2362dc1aa..4eccf08a7 100644 --- a/resources/lib/UnityGroup.php +++ b/resources/lib/UnityGroup.php @@ -43,7 +43,7 @@ public function requestGroup(bool $send_mail_to_admins, bool $send_mail = true): throw new Exception("requested to create group '$this' that already exists!"); } if ($this->SQL->accDeletionRequestExists($this->getOwner()->uid)) { - return; + throw new Exception("group owner '$this' requested account deletion"); } $context = [ "user" => $this->getOwner()->uid, @@ -72,8 +72,9 @@ public function approveGroup(bool $send_mail = true): void if ($this->exists()) { throw new Exception("group '$this' exists"); } - if ($this->SQL->accDeletionRequestExists($this->getOwner()->getUID())) { - throw new Exception("group owner '{$this->getOwner()->getUID()}' requested account deletion"); + $owner_uid = $this->getOwner()->uid; + if ($this->SQL->accDeletionRequestExists($owner_uid)) { + throw new Exception("group owner '$owner_uid' requested account deletion"); } \ensure($this->getOwner()->exists()); $this->init(); @@ -94,7 +95,9 @@ public function denyGroup(bool $send_mail = true): void $request = $this->SQL->getRequest($this->getOwner()->uid, UnitySQL::REQUEST_BECOME_PI); $this->SQL->removeRequest($this->getOwner()->uid, UnitySQL::REQUEST_BECOME_PI); if ($this->exists()) { - throw new Exception("group '$this' creation request cannot be denied, it already exists!"); + throw new Exception( + "group '$this' creation request cannot be denied, it already exists!", + ); } $this->SQL->addLog("denied_group", $this->getOwner()->uid); if ($send_mail) { @@ -105,7 +108,10 @@ public function denyGroup(bool $send_mail = true): void public function cancelGroupRequest(bool $send_mail = true): void { if (!$this->SQL->requestExists($this->getOwner()->uid, UnitySQL::REQUEST_BECOME_PI)) { - UnitySite::errorLog("warning", "attempt to cancel nonexistent group creation request ($this)"); + UnityHTTPD::errorLog( + "warning", + "attempt to cancel nonexistent group creation request ($this)", + ); return; } $this->SQL->removeRequest($this->getOwner()->uid, UnitySQL::REQUEST_BECOME_PI); @@ -119,7 +125,10 @@ public function cancelGroupRequest(bool $send_mail = true): void public function cancelGroupJoinRequest(UnityUser $user, bool $send_mail = true): void { if (!$this->requestExists($user)) { - UnitySite::errorLog("warning", "attempt to cancel nonexistent group join request ($this)"); + UnityHTTPD::errorLog( + "warning", + "attempt to cancel nonexistent group join request ($this)", + ); return; } $this->SQL->removeRequest($user->uid, $this->gid); @@ -222,6 +231,10 @@ public function denyUser(UnityUser $new_user, bool $send_mail = true): void public function removeUser(UnityUser $new_user, bool $send_mail = true): void { if (!$this->memberUIDExists($new_user->uid)) { + UnityHTTPD::errorLog( + "warning", + "attempted to delete absent user '$new_user' from group '$this'", + ); return; } if ($new_user->uid == $this->getOwner()->uid) { @@ -250,11 +263,10 @@ public function removeUser(UnityUser $new_user, bool $send_mail = true): void public function newUserRequest(UnityUser $new_user, bool $send_mail = true): void { if ($this->memberUIDExists($new_user->uid)) { - UnityHTTPD::errorLog("warning", "user '$new_user' already in group"); - return; + throw new Exception("user '$new_user' already in group"); } if ($this->requestExists($new_user)) { - UnityHTTPD::errorLog("warning", "user '$new_user' already requested group membership"); + throw new Exception("user '$new_user' already requested group membership"); return; } if ($this->SQL->accDeletionRequestExists($new_user->uid)) { diff --git a/test/functional/PiBecomeRequestTest.php b/test/functional/PiBecomeRequestTest.php index 34933c777..133da5762 100644 --- a/test/functional/PiBecomeRequestTest.php +++ b/test/functional/PiBecomeRequestTest.php @@ -43,15 +43,14 @@ public function testRequestBecomePiUserRequestedAccountDeletion() { global $USER, $SQL; $this->switchUser("Blank"); - $this->assertNumberPiBecomeRequests(0); try { $USER->requestAccountDeletion(); + $this->expectException(Exception::class); http_post(__DIR__ . "/../../webroot/panel/account.php", [ "form_type" => "pi_request", "tos" => "agree", "account_policy" => "agree", ]); - $this->assertNumberPiBecomeRequests(0); } finally { if ($SQL->requestExists($USER, UnitySQL::REQUEST_BECOME_PI)) { $SQL->removeRequest($USER->uid, UnitySQL::REQUEST_BECOME_PI); From b8bbf7404659001b4ef028444dee89ef98d58edc Mon Sep 17 00:00:00 2001 From: Simon Leary Date: Mon, 5 Jan 2026 15:34:38 -0500 Subject: [PATCH 3/3] remove dead code --- resources/lib/UnityGroup.php | 1 - 1 file changed, 1 deletion(-) diff --git a/resources/lib/UnityGroup.php b/resources/lib/UnityGroup.php index 4eccf08a7..d1d709f5b 100644 --- a/resources/lib/UnityGroup.php +++ b/resources/lib/UnityGroup.php @@ -267,7 +267,6 @@ public function newUserRequest(UnityUser $new_user, bool $send_mail = true): voi } if ($this->requestExists($new_user)) { throw new Exception("user '$new_user' already requested group membership"); - return; } if ($this->SQL->accDeletionRequestExists($new_user->uid)) { throw new Exception("user '$new_user' requested account deletion");