[core] Make User::by_name / User::by_id not-null

Nearly everywhere that these functions are called, the result is assumed to be not-null, and a null will break things

core/cli_app.php

@@ -38,11 +38,7 @@ class CliApp extends \Symfony\Component\Console\Application
         if ($input->hasParameterOption(['--user', '-u'])) {
             $name = $input->getParameterOption(['--user', '-u']);
             $user = User::by_name($name);
-            if (is_null($user)) {
-                die("Unknown user '$name'\n");
-            } else {
-                send_event(new UserLoginEvent($user));
-            }
+            send_event(new UserLoginEvent($user));
         }
 
         $log_level = SCORE_LOG_WARNING;

core/imageboard/image.php

@@ -265,7 +265,6 @@ class Image implements \ArrayAccess
     public function get_owner(): User
     {
         $user = User::by_id($this->owner_id);
-        assert(!is_null($user));
         return $user;
     }
 

core/polyfills.php

@@ -628,7 +628,9 @@ function validate_input(array $inputs): array
         if (in_array('user_id', $flags)) {
             $id = int_escape($value);
             if (in_array('exists', $flags)) {
-                if (is_null(User::by_id($id))) {
+                try {
+                    User::by_id($id);
+                } catch (UserNotFound $e) {
                     throw new InvalidInput("User #$id does not exist");
                 }
             }

core/testcase.php

@@ -241,18 +241,18 @@ if (class_exists("\\PHPUnit\\Framework\\TestCase")) {
         // user things
         protected static function log_in_as_admin(): void
         {
-            send_event(new UserLoginEvent(User::by_name_ex(self::$admin_name)));
+            send_event(new UserLoginEvent(User::by_name(self::$admin_name)));
         }
 
         protected static function log_in_as_user(): void
         {
-            send_event(new UserLoginEvent(User::by_name_ex(self::$user_name)));
+            send_event(new UserLoginEvent(User::by_name(self::$user_name)));
         }
 
         protected static function log_out(): void
         {
             global $config;
-            send_event(new UserLoginEvent(User::by_id_ex($config->get_int("anon_id", 0))));
+            send_event(new UserLoginEvent(User::by_id($config->get_int("anon_id", 0))));
         }
 
         // post things

core/user.php

@@ -85,8 +85,9 @@ class User
         global $cache, $config, $database;
         $user = $cache->get("user-session-obj:$name-$session");
         if (is_null($user)) {
-            $user_by_name = User::by_name($name);
-            if (is_null($user_by_name)) {
+            try {
+                $user_by_name = User::by_name($name);
+            } catch (UserNotFound $e) {
                 return null;
             }
             if ($user_by_name->get_session_id() === $session) {
@@ -97,7 +98,7 @@ class User
         return $user;
     }
 
-    public static function by_id(int $id): ?User
+    public static function by_id(int $id): User
     {
         global $cache, $database;
         if ($id === 1) {
@@ -110,72 +111,55 @@ class User
         if ($id === 1) {
             $cache->set('user-id:'.$id, $row, 600);
         }
-        return is_null($row) ? null : new User($row);
-    }
-
-    public static function by_id_ex(int $id): User
-    {
-        $u = User::by_id($id);
-        if (is_null($u)) {
+        if (is_null($row)) {
             throw new UserNotFound("Can't find any user with ID $id");
-        } else {
-            return $u;
         }
+        return new User($row);
     }
 
     #[Query(name: "user")]
-    public static function by_name(string $name): ?User
+    public static function by_name(string $name): User
     {
         global $database;
         $row = $database->get_row("SELECT * FROM users WHERE LOWER(name) = LOWER(:name)", ["name" => $name]);
-        return is_null($row) ? null : new User($row);
-    }
-
-    public static function by_name_ex(string $name): User
-    {
-        $u = User::by_name($name);
-        if (is_null($u)) {
+        if (is_null($row)) {
             throw new UserNotFound("Can't find any user named $name");
         } else {
-            return $u;
+            return new User($row);
         }
     }
 
     public static function name_to_id(string $name): int
     {
-        $u = User::by_name($name);
-        if (is_null($u)) {
-            throw new UserNotFound("Can't find any user named $name");
-        } else {
-            return $u->id;
-        }
+        return User::by_name($name)->id;
     }
 
-    public static function by_name_and_pass(string $name, string $pass): ?User
+    public static function by_name_and_pass(string $name, string $pass): User
     {
-        $my_user = User::by_name($name);
-
-        // If user tried to log in as "foo bar" and failed, try "foo_bar"
-        if (!$my_user && str_contains($name, " ")) {
-            $my_user = User::by_name(str_replace(" ", "_", $name));
+        try {
+            $my_user = User::by_name($name);
+        } catch (UserNotFound $e) {
+            // If user tried to log in as "foo bar" and failed, try "foo_bar"
+            try {
+                $my_user = User::by_name(str_replace(" ", "_", $name));
+            } catch (UserNotFound $e) {
+                log_warning("core-user", "Failed to log in as $name (Invalid username)");
+                throw $e;
+            }
         }
 
-        if ($my_user) {
-            if ($my_user->passhash == md5(strtolower($name) . $pass)) {
-                log_info("core-user", "Migrating from md5 to bcrypt for $name");
-                $my_user->set_password($pass);
-            }
-            assert(!is_null($my_user->passhash));
-            if (password_verify($pass, $my_user->passhash)) {
-                log_info("core-user", "Logged in as $name ({$my_user->class->name})");
-                return $my_user;
-            } else {
-                log_warning("core-user", "Failed to log in as $name (Invalid password)");
-            }
+        if ($my_user->passhash == md5(strtolower($name) . $pass)) {
+            log_info("core-user", "Migrating from md5 to bcrypt for $name");
+            $my_user->set_password($pass);
+        }
+        assert(!is_null($my_user->passhash));
+        if (password_verify($pass, $my_user->passhash)) {
+            log_info("core-user", "Logged in as $name ({$my_user->class->name})");
+            return $my_user;
         } else {
-            log_warning("core-user", "Failed to log in as $name (Invalid username)");
+            log_warning("core-user", "Failed to log in as $name (Invalid password)");
+            throw new UserNotFound("Can't find anybody with that username and password");
         }
-        return null;
     }
 
 
@@ -203,8 +187,11 @@ class User
     public function set_name(string $name): void
     {
         global $database;
-        if (User::by_name($name)) {
+        try {
+            User::by_name($name);
             throw new InvalidInput("Desired username is already in use");
+        } catch (UserNotFound $e) {
+            // if user is not found, we're good
         }
         $old_name = $this->name;
         $this->name = $name;

core/userclass.php

@@ -17,7 +17,7 @@ class UserClass
     public static array $known_classes = [];
 
     #[Field]
-    public ?string $name = null;
+    public string $name;
     public ?UserClass $parent = null;
 
     /** @var array<string, bool> */

core/util.php

@@ -747,7 +747,6 @@ function _get_user(): User
     if (is_null($my_user)) {
         $my_user = User::by_id($config->get_int("anon_id", 0));
     }
-    assert(!is_null($my_user));
 
     return $my_user;
 }

ext/alias_editor/main.php

@@ -175,6 +175,8 @@ class AliasEditor extends Extension
         foreach (explode("\n", $csv) as $line) {
             $parts = str_getcsv($line);
             if (count($parts) == 2) {
+                assert(is_string($parts[0]));
+                assert(is_string($parts[1]));
                 send_event(new AddAliasEvent($parts[0], $parts[1]));
                 $i++;
             }

ext/auto_tagger/main.php

@@ -187,6 +187,8 @@ class AutoTagger extends Extension
         foreach (explode("\n", $csv) as $line) {
             $parts = str_getcsv($line);
             if (count($parts) == 2) {
+                assert(is_string($parts[0]));
+                assert(is_string($parts[1]));
                 send_event(new AddAutoTagEvent($parts[0], $parts[1]));
                 $i++;
             }

ext/danbooru_api/main.php

@@ -80,14 +80,12 @@ class DanbooruApi extends Extension
         if (isset($_REQUEST['login']) && isset($_REQUEST['password'])) {
             // Get this user from the db, if it fails the user becomes anonymous
             // Code borrowed from /ext/user
-            $name = $_REQUEST['login'];
-            $pass = $_REQUEST['password'];
-            $duser = User::by_name_and_pass($name, $pass);
-            if (!is_null($duser)) {
-                $user = $duser;
-            } else {
+            try {
+                $name = $_REQUEST['login'];
+                $pass = $_REQUEST['password'];
+                $user = User::by_name_and_pass($name, $pass);
+            } catch (UserNotFound $e) {
                 $user = User::by_id($config->get_int("anon_id", 0));
-                assert(!is_null($user));
             }
             send_event(new UserLoginEvent($user));
         }

ext/numeric_score/main.php

@@ -327,22 +327,12 @@ class NumericScore extends Extension
             $event->add_querylet(new Querylet("numeric_score $cmp $score"));
         } elseif (preg_match("/^upvoted_by[=|:](.*)$/i", $event->term, $matches)) {
             $duser = User::by_name($matches[1]);
-            if (is_null($duser)) {
-                throw new SearchTermParseException(
-                    "Can't find the user named ".html_escape($matches[1])
-                );
-            }
             $event->add_querylet(new Querylet(
                 "images.id in (SELECT image_id FROM numeric_score_votes WHERE user_id=:ns_user_id AND score=1)",
                 ["ns_user_id" => $duser->id]
             ));
         } elseif (preg_match("/^downvoted_by[=|:](.*)$/i", $event->term, $matches)) {
             $duser = User::by_name($matches[1]);
-            if (is_null($duser)) {
-                throw new SearchTermParseException(
-                    "Can't find the user named ".html_escape($matches[1])
-                );
-            }
             $event->add_querylet(new Querylet(
                 "images.id in (SELECT image_id FROM numeric_score_votes WHERE user_id=:ns_user_id AND score=-1)",
                 ["ns_user_id" => $duser->id]

ext/numeric_score/test.php

@@ -50,10 +50,10 @@ class NumericScoreTest extends ShimmiePHPUnitTestCase
         $this->assertEquals(404, $page->code);
 
         # test errors
-        $this->assertException(SearchTermParseException::class, function () {
+        $this->assertException(UserNotFound::class, function () {
             $this->get_page("post/list/upvoted_by=asdfasdf/1");
         });
-        $this->assertException(SearchTermParseException::class, function () {
+        $this->assertException(UserNotFound::class, function () {
             $this->get_page("post/list/downvoted_by=asdfasdf/1");
         });
 

ext/post_owner/main.php

@@ -28,11 +28,7 @@ class PostOwner extends Extension
         $owner = $event->get_param('owner');
         if ($user->can(Permissions::EDIT_IMAGE_OWNER) && !is_null($owner)) {
             $owner_ob = User::by_name($owner);
-            if (!is_null($owner_ob)) {
-                send_event(new OwnerSetEvent($event->image, $owner_ob));
-            } else {
-                throw new UserNotFound("Error: No user with that name was found.");
-            }
+            send_event(new OwnerSetEvent($event->image, $owner_ob));
         }
     }
 

ext/replace_file/main.php

@@ -67,12 +67,10 @@ class ReplaceFile extends Extension
         $image->remove_image_only(); // Actually delete the old image file from disk
 
         $target = warehouse_path(Image::IMAGE_DIR, $event->new_hash);
-        if (!@copy($event->tmp_filename, $target)) {
-            $errors = error_get_last();
-            throw new ImageReplaceException(
-                "Failed to copy file from uploads ({$event->tmp_filename}) to archive ($target): ".
-                "{$errors['type']} / {$errors['message']}"
-            );
+        try {
+            \Safe\copy($event->tmp_filename, $target);
+        } catch (\Exception $e) {
+            throw new ImageReplaceException("Failed to copy file from uploads ({$event->tmp_filename}) to archive ($target): {$e->getMessage()}");
         }
         unlink($event->tmp_filename);
 

ext/setup/main.php

@@ -239,7 +239,7 @@ class SetupBlock extends Block
 
     public function add_shorthand_int_option(string $name, string $label = null, bool $table_row = false): void
     {
-        $val = to_shorthand_int($this->config->get_int($name));
+        $val = to_shorthand_int($this->config->get_int($name, 0));
         $html = "<input type='text' id='$name' name='_config_$name' value='$val' size='6'>\n";
         $html .= "<input type='hidden' name='_type_$name' value='int'>\n";
 

ext/source_history/main.php

@@ -267,13 +267,8 @@ class SourceHistory extends Extension
 
         if (!is_null($name)) {
             $duser = User::by_name($name);
-            if (is_null($duser)) {
-                $this->theme->add_status($name, "user not found");
-                return;
-            } else {
-                $select_code[] = 'user_id = :user_id';
-                $select_args['user_id'] = $duser->id;
-            }
+            $select_code[] = 'user_id = :user_id';
+            $select_args['user_id'] = $duser->id;
         }
 
         if (!is_null($ip)) {

ext/tag_history/main.php

@@ -313,13 +313,8 @@ class TagHistory extends Extension
 
         if (!is_null($name)) {
             $duser = User::by_name($name);
-            if (is_null($duser)) {
-                $this->theme->add_status($name, "user not found");
-                return;
-            } else {
-                $select_code[] = 'user_id = :user_id';
-                $select_args['user_id'] = $duser->id;
-            }
+            $select_code[] = 'user_id = :user_id';
+            $select_args['user_id'] = $duser->id;
         }
 
         if (!is_null($ip)) {

ext/user/main.php

@@ -90,17 +90,16 @@ class LoginResult
     public static function login(string $username, string $password): LoginResult
     {
         global $config;
-        $duser = User::by_name_and_pass($username, $password);
-        if (!is_null($duser)) {
+        try {
+            $duser = User::by_name_and_pass($username, $password);
             return new LoginResult(
                 $duser,
                 $duser->get_session_id(),
                 null
             );
-        } else {
-            $anon = User::by_id($config->get_int("anon_id", 0));
+        } catch (UserNotFound $ex) {
             return new LoginResult(
-                $anon,
+                User::by_id($config->get_int("anon_id", 0)),
                 null,
                 "No user found"
             );
@@ -310,11 +309,14 @@ class UserPage extends Extension
         }
 
         if ($event->page_matches("user/{name}")) {
-            $display_user = User::by_name($event->get_arg('name'));
-            if (!is_null($display_user) && ($display_user->id != $config->get_int("anon_id"))) {
+            try {
+                $display_user = User::by_name($event->get_arg('name'));
+                if ($display_user->id == $config->get_int("anon_id")) {
+                    throw new UserNotFound("No such user");
+                }
                 $e = send_event(new UserPageBuildingEvent($display_user));
                 $this->display_stats($e);
-            } else {
+            } catch (UserNotFound $ex) {
                 $this->theme->display_error(
                     404,
                     "No Such User",
@@ -519,8 +521,11 @@ class UserPage extends Extension
                 "letters, numbers, dash, and underscore"
             );
         }
-        if (User::by_name($name)) {
+        try {
+            User::by_name($name);
             throw new UserCreationException("That username is already taken");
+        } catch (UserNotFound $ex) {
+            // user not found is good
         }
         if (!captcha_check()) {
             throw new UserCreationException("Error in captcha");
@@ -588,11 +593,6 @@ class UserPage extends Extension
         $matches = [];
         if (preg_match(self::USER_SEARCH_REGEX, $event->term, $matches)) {
             $duser = User::by_name($matches[2]);
-            if (is_null($duser)) {
-                throw new SearchTermParseException(
-                    "Can't find the user named " . html_escape($matches[2])
-                );
-            }
             $event->add_querylet(new Querylet("images.owner_id {$matches[1]}= {$duser->id}"));
         } elseif (preg_match(self::USER_ID_SEARCH_REGEX, $event->term, $matches)) {
             $user_id = int_escape($matches[2]);
@@ -630,19 +630,15 @@ class UserPage extends Extension
         global $config, $page;
 
         $duser = User::by_name_and_pass($name, $pass);
-        if (!is_null($duser)) {
-            send_event(new UserLoginEvent($duser));
-            $duser->set_login_cookie();
-            $page->set_mode(PageMode::REDIRECT);
+        send_event(new UserLoginEvent($duser));
+        $duser->set_login_cookie();
+        $page->set_mode(PageMode::REDIRECT);
 
-            // Try returning to previous page
-            if ($config->get_int("user_loginshowprofile", 0)) {
-                $page->set_redirect(referer_or(make_link(), ["user/"]));
-            } else {
-                $page->set_redirect(make_link("user"));
-            }
+        // Try returning to previous page
+        if ($config->get_int("user_loginshowprofile", 0)) {
+            $page->set_redirect(referer_or(make_link(), ["user/"]));
         } else {
-            $this->theme->display_error(401, "Error", "No user with those details was found");
+            $page->set_redirect(make_link("user"));
         }
     }
 
@@ -669,9 +665,7 @@ class UserPage extends Extension
     private function page_recover(string $username): void
     {
         $my_user = User::by_name($username);
-        if (is_null($my_user)) {
-            $this->theme->display_error(404, "Error", "There's no user with that name");
-        } elseif (is_null($my_user->email)) {
+        if (is_null($my_user->email)) {
             $this->theme->display_error(400, "Error", "That user has no registered email address");
         } else {
             throw new ServerError("Email sending not implemented");

ext/user/test.php

@@ -66,7 +66,7 @@ class UserPageTest extends ShimmiePHPUnitTestCase
                 'email' => '',
             ]);
         });
-        $this->assertNull(User::by_name('testnew'), "Anon can't create others");
+        $this->assertException(UserNotFound::class, function () {User::by_name('testnew');});
 
         $this->log_in_as_admin();
         $this->post_page('user_admin/create_other', [
@@ -76,6 +76,6 @@ class UserPageTest extends ShimmiePHPUnitTestCase
             'email' => '',
         ]);
         $this->assertEquals(302, $page->code);
-        $this->assertNotNull(User::by_name('testnew'), "Admin can create others");
+        $this->assertNotNull(User::by_name('testnew'));
     }
 }

ext/user_config/main.php

@@ -66,7 +66,6 @@ class UserConfig extends Extension
         global $database;
 
         $user = User::by_id($id);
-
         $user_config = new DatabaseConfig($database, "user_config", "user_id", "$id");
         send_event(new InitUserConfigEvent($user, $user_config));
         return $user_config;