diff --git a/core/basepage.php b/core/basepage.php index 22756cee..a31aad5d 100644 --- a/core/basepage.php +++ b/core/basepage.php @@ -303,7 +303,7 @@ class BasePage if (!is_null($this->filename)) { header('Content-Disposition: ' . $this->disposition . '; filename=' . $this->filename); } - assert($this->file, "file should not be null with PageMode::FILE"); + assert(!is_null($this->file), "file should not be null with PageMode::FILE"); // https://gist.github.com/codler/3906826 $size = \Safe\filesize($this->file); // File size diff --git a/core/config.php b/core/config.php index 80c8171f..1b323211 100644 --- a/core/config.php +++ b/core/config.php @@ -112,6 +112,10 @@ interface Config /** * Pick a value out of the table by name, cast to the appropriate data type. + * + * @template T of string|null + * @param T $default + * @return T|string */ public function get_string(string $name, ?string $default = null): ?string; diff --git a/core/database.php b/core/database.php index f9c40591..bd8799d6 100644 --- a/core/database.php +++ b/core/database.php @@ -71,8 +71,10 @@ class Database if (is_null($this->db)) { $this->db = new PDO($this->dsn); $this->connect_engine(); + assert(!is_null($this->db)); $this->get_engine()->init($this->db); $this->begin_transaction(); + assert(!is_null($this->db)); } return $this->db; } @@ -155,6 +157,7 @@ class Database { if (is_null($this->engine)) { $this->connect_engine(); + assert(!is_null($this->engine)); } return $this->engine; } @@ -182,7 +185,8 @@ class Database global $_tracer, $tracer_enabled; $dur = ftime() - $start; // trim whitespace - $query = preg_replace('/[\n\t ]+/m', ' ', $query); + $query = \Safe\preg_replace('/[\n\t ]+/m', ' ', $query); + assert(is_string($query)); $query = trim($query); if ($tracer_enabled) { $_tracer->complete($start * 1000000, $dur * 1000000, "DB Query", ["query" => $query, "args" => $args, "method" => $method]); diff --git a/core/extension.php b/core/extension.php index 9215a858..cc042c84 100644 --- a/core/extension.php +++ b/core/extension.php @@ -181,14 +181,16 @@ abstract class ExtensionInfo { if ($this->supported === null) { $this->check_support(); + assert(!is_null($this->supported)); } return $this->supported; } public function get_support_info(): string { - if ($this->supported === null) { + if ($this->support_info === null) { $this->check_support(); + assert(!is_null($this->support_info)); } return $this->support_info; } @@ -373,12 +375,10 @@ abstract class DataHandlerExtension extends Extension // If everything is OK, then move the file to the archive $filename = warehouse_path(Image::IMAGE_DIR, $event->hash); - if (!@copy($event->tmpname, $filename)) { - $errors = error_get_last(); - throw new UploadException( - "Failed to copy file from uploads ({$event->tmpname}) to archive ($filename): ". - "{$errors['type']} / {$errors['message']}" - ); + try { + \Safe\copy($event->tmpname, $filename); + } catch (\Exception $e) { + throw new UploadException("Failed to copy file from uploads ({$event->tmpname}) to archive ($filename): ".$e->getMessage()); } $event->images[] = $iae->image; diff --git a/core/polyfills.php b/core/polyfills.php index 0086023a..8c71e523 100644 --- a/core/polyfills.php +++ b/core/polyfills.php @@ -693,7 +693,9 @@ function validate_input(array $inputs): array */ function sanitize_path(string $path): string { - return preg_replace('|[\\\\/]+|S', DIRECTORY_SEPARATOR, $path); + $r = \Safe\preg_replace('|[\\\\/]+|S', DIRECTORY_SEPARATOR, $path); + assert(is_string($r)); + return $r; } /** diff --git a/core/testcase.php b/core/testcase.php index f70ec7bf..3bee4163 100644 --- a/core/testcase.php +++ b/core/testcase.php @@ -44,7 +44,7 @@ if (class_exists("\\PHPUnit\\Framework\\TestCase")) { $database->execute("SAVEPOINT test_start"); self::log_out(); foreach ($database->get_col("SELECT id FROM images") as $image_id) { - send_event(new ImageDeletionEvent(Image::by_id((int)$image_id), true)); + send_event(new ImageDeletionEvent(Image::by_id_ex((int)$image_id), true)); } $_tracer->end(); # setUp @@ -223,38 +223,36 @@ if (class_exists("\\PHPUnit\\Framework\\TestCase")) { $this->assertEquals($results, $ids); } - protected function assertException(string $type, callable $function): \Exception|null + protected function assertException(string $type, callable $function): \Exception { - $exception = null; try { call_user_func($function); - } catch (\Exception $e) { - $exception = $e; + self::fail("Expected exception of type $type, but none was thrown"); + } catch (\Exception $exception) { + self::assertThat( + $exception, + new \PHPUnit\Framework\Constraint\Exception($type), + "Expected exception of type $type, but got " . get_class($exception) + ); + return $exception; } - - self::assertThat( - $exception, - new \PHPUnit\Framework\Constraint\Exception($type), - "Expected exception of type $type, but got " . ($exception ? get_class($exception) : "none") - ); - return $exception; } // user things protected static function log_in_as_admin(): void { - send_event(new UserLoginEvent(User::by_name(self::$admin_name))); + send_event(new UserLoginEvent(User::by_name_ex(self::$admin_name))); } protected static function log_in_as_user(): void { - send_event(new UserLoginEvent(User::by_name(self::$user_name))); + send_event(new UserLoginEvent(User::by_name_ex(self::$user_name))); } protected static function log_out(): void { global $config; - send_event(new UserLoginEvent(User::by_id($config->get_int("anon_id", 0)))); + send_event(new UserLoginEvent(User::by_id_ex($config->get_int("anon_id", 0)))); } // post things diff --git a/core/urls.php b/core/urls.php index 5e436029..4875bd26 100644 --- a/core/urls.php +++ b/core/urls.php @@ -6,10 +6,10 @@ namespace Shimmie2; class Link { - public ?string $page; + public string $page; public ?string $query; - public function __construct(?string $page = null, ?string $query = null) + public function __construct(string $page, ?string $query = null) { $this->page = $page; $this->query = $query; @@ -61,8 +61,12 @@ function make_link(?string $page = null, ?string $query = null, ?string $fragmen $parts['path'] = "$install_dir/index.php"; $query = empty($query) ? "q=$page" : "q=$page&$query"; } - $parts['query'] = $query; // http_build_query($query); - $parts['fragment'] = $fragment; // http_build_query($hash); + if (!is_null($query)) { + $parts['query'] = $query; // http_build_query($query); + } + if (!is_null($fragment)) { + $parts['fragment'] = $fragment; // http_build_query($hash); + } return unparse_url($parts); } diff --git a/core/user.php b/core/user.php index 8c015667..26d86acb 100644 --- a/core/user.php +++ b/core/user.php @@ -86,6 +86,9 @@ class User $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)) { + return null; + } if ($user_by_name->get_session_id() === $session) { $user = $user_by_name; } @@ -110,6 +113,16 @@ class User 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)) { + throw new UserNotFound("Can't find any user with ID $id"); + } else { + return $u; + } + } + #[Query(name: "user")] public static function by_name(string $name): ?User { @@ -118,6 +131,16 @@ class User 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)) { + throw new UserNotFound("Can't find any user named $name"); + } else { + return $u; + } + } + public static function name_to_id(string $name): int { $u = User::by_name($name); @@ -142,6 +165,7 @@ class User 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;