From d3bca6eb478e852f15f704e5ac1f12e8956fd8e3 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Thu, 20 Jul 2017 22:40:09 -0400 Subject: [PATCH] More code coverage accommodation --- .gitignore | 1 + lib/Database.php | 268 +++++++++++++++++------ lib/Db/AbstractDriver.php | 16 +- lib/Db/AbstractStatement.php | 24 +- lib/Db/SQLite3/Statement.php | 8 +- lib/Feed.php | 80 +++++-- lib/Lang.php | 55 +++-- lib/Misc/Context.php | 20 +- lib/Misc/Date.php | 8 +- lib/Misc/Query.php | 8 +- lib/REST.php | 8 +- lib/REST/AbstractHandler.php | 24 +- lib/REST/NextCloudNews/V1_2.php | 90 ++++++-- lib/REST/Request.php | 20 +- lib/Service.php | 4 +- lib/Service/Curl/Driver.php | 2 +- lib/User.php | 160 ++++++++++---- lib/User/Internal/Driver.php | 4 +- lib/User/Internal/InternalFunctions.php | 31 +-- tests/Misc/TestContext.php | 8 +- tests/REST/NextCloudNews/TestNCNV1_2.php | 2 +- tests/User/TestAuthorization.php | 28 ++- 22 files changed, 640 insertions(+), 229 deletions(-) diff --git a/.gitignore b/.gitignore index be6a2d52..51ec49a3 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,7 @@ vendor/ #temp files documentation/ +tests/coverage arsse.db* # Windows image file caches diff --git a/lib/Database.php b/lib/Database.php index 7c74eebd..77305ca3 100644 --- a/lib/Database.php +++ b/lib/Database.php @@ -42,7 +42,9 @@ class Database { } public function driverSchemaUpdate(): bool { - if($this->db->schemaVersion() < self::SCHEMA_VERSION) return $this->db->schemaUpdate(self::SCHEMA_VERSION); + if($this->db->schemaVersion() < self::SCHEMA_VERSION) { + return $this->db->schemaUpdate(self::SCHEMA_VERSION); + } return false; } @@ -53,7 +55,9 @@ class Database { [], // binding values ]; foreach($valid as $prop => $type) { - if(!array_key_exists($prop, $props)) continue; + if(!array_key_exists($prop, $props)) { + continue; + } $out[0][] = "$prop = ?"; $out[1][] = $type; $out[2][] = $props[$prop]; @@ -95,37 +99,54 @@ class Database { } public function userExists(string $user): bool { - if(!Arsse::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + if(!Arsse::$user->authorize($user, __FUNCTION__)) { + throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + } return (bool) $this->db->prepare("SELECT count(*) from arsse_users where id is ?", "str")->run($user)->getValue(); } public function userAdd(string $user, string $password = null): string { - if(!Arsse::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); - if($this->userExists($user)) throw new User\Exception("alreadyExists", ["action" => __FUNCTION__, "user" => $user]); - if($password===null) $password = (new PassGen)->length(Arsse::$conf->userTempPasswordLength)->get(); + if(!Arsse::$user->authorize($user, __FUNCTION__)) { + throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + } else if($this->userExists($user)) { + throw new User\Exception("alreadyExists", ["action" => __FUNCTION__, "user" => $user]); + } + if($password===null) { + $password = (new PassGen)->length(Arsse::$conf->userTempPasswordLength)->get(); + } $hash = ""; - if(strlen($password) > 0) $hash = password_hash($password, \PASSWORD_DEFAULT); + if(strlen($password) > 0) { + $hash = password_hash($password, \PASSWORD_DEFAULT); + } $this->db->prepare("INSERT INTO arsse_users(id,password) values(?,?)", "str", "str")->runArray([$user,$hash]); return $password; } public function userRemove(string $user): bool { - if(!Arsse::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); - if($this->db->prepare("DELETE from arsse_users where id is ?", "str")->run($user)->changes() < 1) throw new User\Exception("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); + if(!Arsse::$user->authorize($user, __FUNCTION__)) { + throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + } + if($this->db->prepare("DELETE from arsse_users where id is ?", "str")->run($user)->changes() < 1) { + throw new User\Exception("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); + } return true; } public function userList(string $domain = null): array { $out = []; if($domain !== null) { - if(!Arsse::$user->authorize("@".$domain, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $domain]); + if(!Arsse::$user->authorize("@".$domain, __FUNCTION__)) { + throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $domain]); + } $domain = str_replace(["\\","%","_"],["\\\\", "\\%", "\\_"], $domain); $domain = "%@".$domain; foreach($this->db->prepare("SELECT id from arsse_users where id like ?", "str")->run($domain) as $user) { $out[] = $user['id']; } } else { - if(!Arsse::$user->authorize("", __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => "global"]); + if(!Arsse::$user->authorize("", __FUNCTION__)) { + throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => "global"]); + } foreach($this->db->prepare("SELECT id from arsse_users")->run() as $user) { $out[] = $user['id']; } @@ -134,31 +155,48 @@ class Database { } public function userPasswordGet(string $user): string { - if(!Arsse::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); - if(!$this->userExists($user)) throw new User\Exception("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); + if(!Arsse::$user->authorize($user, __FUNCTION__)) { + throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + } else if(!$this->userExists($user)) { + throw new User\Exception("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); + } return (string) $this->db->prepare("SELECT password from arsse_users where id is ?", "str")->run($user)->getValue(); } public function userPasswordSet(string $user, string $password = null): string { - if(!Arsse::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); - if(!$this->userExists($user)) throw new User\Exception("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); - if($password===null) $password = (new PassGen)->length(Arsse::$conf->userTempPasswordLength)->get(); + if(!Arsse::$user->authorize($user, __FUNCTION__)) { + throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + } else if(!$this->userExists($user)) { + throw new User\Exception("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); + } + if($password===null) { + $password = (new PassGen)->length(Arsse::$conf->userTempPasswordLength)->get(); + } $hash = ""; - if(strlen($password) > 0) $hash = password_hash($password, \PASSWORD_DEFAULT); + if(strlen($password) > 0) { + $hash = password_hash($password, \PASSWORD_DEFAULT); + } $this->db->prepare("UPDATE arsse_users set password = ? where id is ?", "str", "str")->run($hash, $user); return $password; } public function userPropertiesGet(string $user): array { - if(!Arsse::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + if(!Arsse::$user->authorize($user, __FUNCTION__)) { + throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + } $prop = $this->db->prepare("SELECT name,rights from arsse_users where id is ?", "str")->run($user)->getRow(); - if(!$prop) throw new User\Exception("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); + if(!$prop) { + throw new User\Exception("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); + } return $prop; } public function userPropertiesSet(string $user, array $properties): array { - if(!Arsse::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); - if(!$this->userExists($user)) throw new User\Exception("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); + if(!Arsse::$user->authorize($user, __FUNCTION__)) { + throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + } else if(!$this->userExists($user)) { + throw new User\Exception("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); + } $valid = [ // FIXME: add future properties "name" => "str", ]; @@ -168,13 +206,18 @@ class Database { } public function userRightsGet(string $user): int { - if(!Arsse::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + if(!Arsse::$user->authorize($user, __FUNCTION__)) { + throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + } return (int) $this->db->prepare("SELECT rights from arsse_users where id is ?", "str")->run($user)->getValue(); } public function userRightsSet(string $user, int $rights): bool { - if(!Arsse::$user->authorize($user, __FUNCTION__, $rights)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); - if(!$this->userExists($user)) throw new User\Exception("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); + if(!Arsse::$user->authorize($user, __FUNCTION__, $rights)) { + throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + } else if(!$this->userExists($user)) { + throw new User\Exception("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); + } $this->db->prepare("UPDATE arsse_users set rights = ? where id is ?", "int", "str")->run($rights, $user); return true; } @@ -198,7 +241,9 @@ class Database { } else { // if a parent is specified, make sure it exists and belongs to the user; get its root (first-level) folder if it's a nested folder $p = $this->db->prepare("SELECT id from arsse_folders where owner is ? and id is ?", "str", "int")->run($user, $parent)->getValue(); - if(!$p) throw new Db\ExceptionInput("idMissing", ["action" => __FUNCTION__, "field" => "parent", 'id' => $parent]); + if(!$p) { + throw new Db\ExceptionInput("idMissing", ["action" => __FUNCTION__, "field" => "parent", 'id' => $parent]); + } } // check if a folder by the same name already exists, because nulls are wonky in SQL // FIXME: How should folder name be compared? Should a Unicode normalization be applied before comparison and insertion? @@ -232,24 +277,36 @@ class Database { } public function folderRemove(string $user, int $id): bool { - if(!Arsse::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + if(!Arsse::$user->authorize($user, __FUNCTION__)) { + throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + } $changes = $this->db->prepare("DELETE FROM arsse_folders where owner is ? and id is ?", "str", "int")->run($user, $id)->changes(); - if(!$changes) throw new Db\ExceptionInput("subjectMissing", ["action" => __FUNCTION__, "field" => "folder", 'id' => $id]); + if(!$changes) { + throw new Db\ExceptionInput("subjectMissing", ["action" => __FUNCTION__, "field" => "folder", 'id' => $id]); + } return true; } public function folderPropertiesGet(string $user, int $id): array { - if(!Arsse::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + if(!Arsse::$user->authorize($user, __FUNCTION__)) { + throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + } $props = $this->db->prepare("SELECT id,name,parent from arsse_folders where owner is ? and id is ?", "str", "int")->run($user, $id)->getRow(); - if(!$props) throw new Db\ExceptionInput("subjectMissing", ["action" => __FUNCTION__, "field" => "folder", 'id' => $id]); + if(!$props) { + throw new Db\ExceptionInput("subjectMissing", ["action" => __FUNCTION__, "field" => "folder", 'id' => $id]); + } return $props; } public function folderPropertiesSet(string $user, int $id, array $data): bool { - if(!Arsse::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + if(!Arsse::$user->authorize($user, __FUNCTION__)) { + throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + } // validate the folder ID and, if specified, the parent to move it to $parent = null; - if(array_key_exists("parent", $data)) $parent = $data['parent']; + if(array_key_exists("parent", $data)) { + $parent = $data['parent']; + } $f = $this->folderValidateId($user, $id, $parent, true); // if a new name is specified, validate it if(array_key_exists("name", $data)) { @@ -279,7 +336,9 @@ class Database { } // check whether the folder exists and is owned by the user $f = $this->db->prepare("SELECT name,parent from arsse_folders where owner is ? and id is ?", "str", "int")->run($user, $id)->getRow(); - if(!$f) throw new Db\ExceptionInput($subject ? "subjectMissing" : "idMissing", ["action" => $this->caller(), "field" => "folder", 'id' => $parent]); + if(!$f) { + throw new Db\ExceptionInput($subject ? "subjectMissing" : "idMissing", ["action" => $this->caller(), "field" => "folder", 'id' => $parent]); + } // if we're moving a folder to a new parent, check that the parent is valid if(!is_null($parent)) { // make sure both that the parent exists, and that the parent is not either the folder itself or one of its children (a circular dependence) @@ -293,7 +352,9 @@ class Database { throw new Db\ExceptionInput("idMissing", ["action" => $this->caller(), "field" => "parent", 'id' => $parent]); } else { // if using the desired parent would create a circular dependence, throw a different exception - if(!$p['valid']) throw new Db\ExceptionInput("circularDependence", ["action" => $this->caller(), "field" => "parent", 'id' => $parent]); + if(!$p['valid']) { + throw new Db\ExceptionInput("circularDependence", ["action" => $this->caller(), "field" => "parent", 'id' => $parent]); + } } } return $f; @@ -311,7 +372,9 @@ class Database { } public function subscriptionAdd(string $user, string $url, string $fetchUser = "", string $fetchPassword = ""): int { - if(!Arsse::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + if(!Arsse::$user->authorize($user, __FUNCTION__)) { + throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + } // check to see if the feed exists $feedID = $this->db->prepare("SELECT id from arsse_feeds where url is ? and username is ? and password is ?", "str", "str", "str")->run($url, $fetchUser, $fetchPassword)->getValue(); if(is_null($feedID)) { @@ -331,7 +394,9 @@ class Database { } public function subscriptionList(string $user, int $folder = null, int $id = null): Db\Result { - if(!Arsse::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + if(!Arsse::$user->authorize($user, __FUNCTION__)) { + throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + } // create a complex query $q = new Query( "SELECT @@ -366,24 +431,34 @@ class Database { } public function subscriptionRemove(string $user, int $id): bool { - if(!Arsse::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + if(!Arsse::$user->authorize($user, __FUNCTION__)) { + throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + } $changes = $this->db->prepare("DELETE from arsse_subscriptions where owner is ? and id is ?", "str", "int")->run($user, $id)->changes(); - if(!$changes) throw new Db\ExceptionInput("subjectMissing", ["action" => __FUNCTION__, "field" => "folder", 'id' => $id]); + if(!$changes) { + throw new Db\ExceptionInput("subjectMissing", ["action" => __FUNCTION__, "field" => "folder", 'id' => $id]); + } return true; } public function subscriptionPropertiesGet(string $user, int $id): array { - if(!Arsse::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + if(!Arsse::$user->authorize($user, __FUNCTION__)) { + throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + } // disable authorization checks for the list call Arsse::$user->authorizationEnabled(false); $sub = $this->subscriptionList($user, null, $id)->getRow(); Arsse::$user->authorizationEnabled(true); - if(!$sub) throw new Db\ExceptionInput("subjectMissing", ["action" => __FUNCTION__, "field" => "feed", 'id' => $id]); + if(!$sub) { + throw new Db\ExceptionInput("subjectMissing", ["action" => __FUNCTION__, "field" => "feed", 'id' => $id]); + } return $sub; } public function subscriptionPropertiesSet(string $user, int $id, array $data): bool { - if(!Arsse::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + if(!Arsse::$user->authorize($user, __FUNCTION__)) { + throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + } $tr = $this->db->begin(); if(!$this->db->prepare("SELECT count(*) from arsse_subscriptions where owner is ? and id is ?", "str", "int")->run($user, $id)->getValue()) { // if the ID doesn't exist or doesn't belong to the user, throw an exception @@ -397,8 +472,11 @@ class Database { // if the title is null, this signals intended use of the default title; otherwise make sure it's not effectively an empty string if(!is_null($data['title'])) { $title = (string) $data['title']; - if(!strlen($title)) throw new Db\ExceptionInput("missing", ["action" => __FUNCTION__, "field" => "title"]); - if(!strlen(trim($title))) throw new Db\ExceptionInput("whitespace", ["action" => __FUNCTION__, "field" => "title"]); + if(!strlen($title)) { + throw new Db\ExceptionInput("missing", ["action" => __FUNCTION__, "field" => "title"]); + } else if(!strlen(trim($title))) { + throw new Db\ExceptionInput("whitespace", ["action" => __FUNCTION__, "field" => "title"]); + } $data['title'] = $title; } } @@ -416,7 +494,9 @@ class Database { protected function subscriptionValidateId(string $user, int $id): array { $out = $this->db->prepare("SELECT feed from arsse_subscriptions where id is ? and owner is ?", "int", "str")->run($id, $user)->getRow(); - if(!$out) throw new Db\ExceptionInput("idMissing", ["action" => $this->caller(), "field" => "subscription", 'id' => $id]); + if(!$out) { + throw new Db\ExceptionInput("idMissing", ["action" => $this->caller(), "field" => "subscription", 'id' => $id]); + } return $out; } @@ -429,7 +509,9 @@ class Database { $tr = $this->db->begin(); // check to make sure the feed exists $f = $this->db->prepare("SELECT url, username, password, modified, etag, err_count, scrape FROM arsse_feeds where id is ?", "int")->run($feedID)->getRow(); - if(!$f) throw new Db\ExceptionInput("subjectMissing", ["action" => __FUNCTION__, "field" => "feed", 'id' => $feedID]); + if(!$f) { + throw new Db\ExceptionInput("subjectMissing", ["action" => __FUNCTION__, "field" => "feed", 'id' => $feedID]); + } // determine whether the feed's items should be scraped for full content from the source Web site $scrape = (Arsse::$conf->fetchEnableScraping && $f['scrape']); // the Feed object throws an exception when there are problems, but that isn't ideal @@ -450,7 +532,9 @@ class Database { 'datetime', 'str', 'int' )->run(Feed::nextFetchOnError($f['err_count']), $e->getMessage(),$feedID); $tr->commit(); - if($throwError) throw $e; + if($throwError) { + throw $e; + } return false; } //prepare the necessary statements to perform the update @@ -561,7 +645,9 @@ class Database { } public function articleStarredCount(string $user, array $context = []): int { - if(!Arsse::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + if(!Arsse::$user->authorize($user, __FUNCTION__)) { + throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + } return $this->db->prepare( "WITH RECURSIVE user(user) as (SELECT ?), @@ -576,8 +662,12 @@ class Database { } public function editionLatest(string $user, Context $context = null): int { - if(!Arsse::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); - if(!$context) $context = new Context; + if(!Arsse::$user->authorize($user, __FUNCTION__)) { + throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + } + if(!$context) { + $context = new Context; + } $q = new Query("SELECT max(arsse_editions.id) from arsse_editions left join arsse_articles on article is arsse_articles.id left join arsse_feeds on arsse_articles.feed is arsse_feeds.id"); if($context->subscription()) { // if a subscription is specified, make sure it exists @@ -592,8 +682,12 @@ class Database { } public function articleList(string $user, Context $context = null): Db\Result { - if(!Arsse::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); - if(!$context) $context = new Context; + if(!Arsse::$user->authorize($user, __FUNCTION__)) { + throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + } + if(!$context) { + $context = new Context; + } $q = new Query( "SELECT arsse_articles.id as id, @@ -637,21 +731,37 @@ class Database { $q->setCTE("subscribed_feeds(id,sub)", "SELECT feed,id from arsse_subscriptions join user on user is owner"); } // filter based on edition offset - if($context->oldestEdition()) $q->setWhere("edition >= ?", "int", $context->oldestEdition); - if($context->latestEdition()) $q->setWhere("edition <= ?", "int", $context->latestEdition); + if($context->oldestEdition()) { + $q->setWhere("edition >= ?", "int", $context->oldestEdition); + } + if($context->latestEdition()) { + $q->setWhere("edition <= ?", "int", $context->latestEdition); + } // filter based on lastmod time - if($context->modifiedSince()) $q->setWhere("modified_date >= ?", "datetime", $context->modifiedSince); - if($context->notModifiedSince()) $q->setWhere("modified_date <= ?", "datetime", $context->notModifiedSince); + if($context->modifiedSince()) { + $q->setWhere("modified_date >= ?", "datetime", $context->modifiedSince); + } + if($context->notModifiedSince()) { + $q->setWhere("modified_date <= ?", "datetime", $context->notModifiedSince); + } // filter for un/read and un/starred status if specified - if($context->unread()) $q->setWhere("unread is ?", "bool", $context->unread); - if($context->starred()) $q->setWhere("starred is ?", "bool", $context->starred); + if($context->unread()) { + $q->setWhere("unread is ?", "bool", $context->unread); + } + if($context->starred()) { + $q->setWhere("starred is ?", "bool", $context->starred); + } // perform the query and return results return $this->db->prepare($q->getQuery(), $q->getTypes())->run($q->getValues()); } public function articleMark(string $user, array $data, Context $context = null): bool { - if(!Arsse::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); - if(!$context) $context = new Context; + if(!Arsse::$user->authorize($user, __FUNCTION__)) { + throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + } + if(!$context) { + $context = new Context; + } // sanitize input $values = [ isset($data['read']) ? $data['read'] : null, @@ -683,7 +793,9 @@ class Database { // make sure the edition exists $edition = $this->articleValidateEdition($user, $context->edition); // if the edition is not the latest, do not mark the read flag - if(!$edition['current']) $values[0] = null; + if(!$edition['current']) { + $values[0] = null; + } } else if($context->article()) { // otherwise if an article context is specified, make sure it's valid $this->articleValidateId($user, $context->article); @@ -731,8 +843,11 @@ class Database { } if($context->editions()) { // if multiple specific editions have been requested, prepare a CTE to list them and their articles - if(!$context->editions) throw new Db\ExceptionInput("tooShort", ['field' => "editions", 'action' => __FUNCTION__, 'min' => 1]); // must have at least one array element - if(sizeof($context->editions) > 50) throw new Db\ExceptionInput("tooLong", ['field' => "editions", 'action' => __FUNCTION__, 'max' => 50]); // must not have more than 50 array elements + if(!$context->editions) { + throw new Db\ExceptionInput("tooShort", ['field' => "editions", 'action' => __FUNCTION__, 'min' => 1]); // must have at least one array element + } else if(sizeof($context->editions) > 50) { + throw new Db\ExceptionInput("tooLong", ['field' => "editions", 'action' => __FUNCTION__, 'max' => 50]); // must not have more than 50 array elements + } list($inParams, $inTypes) = $this->generateIn($context->editions, "int"); $q->setCTE("requested_articles(id,edition)", "SELECT article,id as edition from arsse_editions where edition in ($inParams)", @@ -742,8 +857,11 @@ class Database { $q->setWhere("arsse_articles.id in (select id from requested_articles)"); } else if($context->articles()) { // if multiple specific articles have been requested, prepare a CTE to list them and their articles - if(!$context->articles) throw new Db\ExceptionInput("tooShort", ['field' => "articles", 'action' => __FUNCTION__, 'min' => 1]); // must have at least one array element - if(sizeof($context->articles) > 50) throw new Db\ExceptionInput("tooLong", ['field' => "articles", 'action' => __FUNCTION__, 'max' => 50]); // must not have more than 50 array elements + if(!$context->articles) { + throw new Db\ExceptionInput("tooShort", ['field' => "articles", 'action' => __FUNCTION__, 'min' => 1]); // must have at least one array element + } else if(sizeof($context->articles) > 50) { + throw new Db\ExceptionInput("tooLong", ['field' => "articles", 'action' => __FUNCTION__, 'max' => 50]); // must not have more than 50 array elements + } list($inParams, $inTypes) = $this->generateIn($context->articles, "int"); $q->setCTE("requested_articles(id,edition)", "SELECT id,(select max(id) from arsse_editions where article is arsse_articles.id) as edition from arsse_articles where arsse_articles.id in ($inParams)", @@ -756,11 +874,19 @@ class Database { $q->setCTE("requested_articles(id,edition)", "SELECT 'empty','table' where 1 is 0"); } // filter based on edition offset - if($context->oldestEdition()) $q->setWhere("edition >= ?", "int", $context->oldestEdition); - if($context->latestEdition()) $q->setWhere("edition <= ?", "int", $context->latestEdition); + if($context->oldestEdition()) { + $q->setWhere("edition >= ?", "int", $context->oldestEdition); + } + if($context->latestEdition()) { + $q->setWhere("edition <= ?", "int", $context->latestEdition); + } // filter based on lastmod time - if($context->modifiedSince()) $q->setWhere("modified_date >= ?", "datetime", $context->modifiedSince); - if($context->notModifiedSince()) $q->setWhere("modified_date <= ?", "datetime", $context->notModifiedSince); + if($context->modifiedSince()) { + $q->setWhere("modified_date >= ?", "datetime", $context->modifiedSince); + } + if($context->notModifiedSince()) { + $q->setWhere("modified_date <= ?", "datetime", $context->notModifiedSince); + } // push the current query onto the CTE stack and execute the query we're actually interested in $q->pushCTE("target_articles(id,edition,modified_date,to_insert,honour_read,honour_star)"); $q->setBody($query); @@ -783,7 +909,9 @@ class Database { arsse_articles.id is ? and arsse_subscriptions.owner is ?", "int", "str" )->run($id, $user)->getRow(); - if(!$out) throw new Db\ExceptionInput("subjectMissing", ["action" => $this->caller(), "field" => "article", 'id' => $id]); + if(!$out) { + throw new Db\ExceptionInput("subjectMissing", ["action" => $this->caller(), "field" => "article", 'id' => $id]); + } return $out; } @@ -801,7 +929,9 @@ class Database { edition is ? and arsse_subscriptions.owner is ?", "int", "str" )->run($id, $user)->getRow(); - if(!$out) throw new Db\ExceptionInput("subjectMissing", ["action" => $this->caller(), "field" => "edition", 'id' => $id]); + if(!$out) { + throw new Db\ExceptionInput("subjectMissing", ["action" => $this->caller(), "field" => "edition", 'id' => $id]); + } return $out; } } \ No newline at end of file diff --git a/lib/Db/AbstractDriver.php b/lib/Db/AbstractDriver.php index 71cf368e..03e907b1 100644 --- a/lib/Db/AbstractDriver.php +++ b/lib/Db/AbstractDriver.php @@ -35,7 +35,9 @@ abstract class AbstractDriver implements Driver { } public function savepointRelease(int $index = null): bool { - if(is_null($index)) $index = $this->transDepth; + if(is_null($index)) { + $index = $this->transDepth; + } if(array_key_exists($index, $this->transStatus)) { switch($this->transStatus[$index]) { case self::TR_PEND: @@ -43,7 +45,9 @@ abstract class AbstractDriver implements Driver { $this->transStatus[$index] = self::TR_COMMIT; $a = $index; while(++$a && $a <= $this->transDepth) { - if($this->transStatus[$a] <= self::TR_PEND) $this->transStatus[$a] = self::TR_PEND_COMMIT; + if($this->transStatus[$a] <= self::TR_PEND) { + $this->transStatus[$a] = self::TR_PEND_COMMIT; + } } $out = true; break; @@ -78,7 +82,9 @@ abstract class AbstractDriver implements Driver { } public function savepointUndo(int $index = null): bool { - if(is_null($index)) $index = $this->transDepth; + if(is_null($index)) { + $index = $this->transDepth; + } if(array_key_exists($index, $this->transStatus)) { switch($this->transStatus[$index]) { case self::TR_PEND: @@ -87,7 +93,9 @@ abstract class AbstractDriver implements Driver { $this->transStatus[$index] = self::TR_ROLLBACK; $a = $index; while(++$a && $a <= $this->transDepth) { - if($this->transStatus[$a] <= self::TR_PEND) $this->transStatus[$a] = self::TR_PEND_ROLLBACK; + if($this->transStatus[$a] <= self::TR_PEND) { + $this->transStatus[$a] = self::TR_PEND_ROLLBACK; + } } $out = true; break; diff --git a/lib/Db/AbstractStatement.php b/lib/Db/AbstractStatement.php index 8f40aee5..3e3f52c2 100644 --- a/lib/Db/AbstractStatement.php +++ b/lib/Db/AbstractStatement.php @@ -20,7 +20,9 @@ abstract class AbstractStatement implements Statement { } public function rebindArray(array $bindings, bool $append = false): bool { - if(!$append) $this->types = []; + if(!$append) { + $this->types = []; + } foreach($bindings as $binding) { if(is_array($binding)) { // recursively flatten any arrays, which may be provided for SET or IN() clauses @@ -34,7 +36,9 @@ abstract class AbstractStatement implements Statement { } else { $this->isNullable[] = true; } - if(!array_key_exists($binding, self::TYPES)) throw new Exception("paramTypeInvalid", $binding); + if(!array_key_exists($binding, self::TYPES)) { + throw new Exception("paramTypeInvalid", $binding); + } $this->types[] = self::TYPES[$binding]; } } @@ -44,13 +48,19 @@ abstract class AbstractStatement implements Statement { protected function cast($v, string $t, bool $nullable) { switch($t) { case "date": - if(is_null($v) && !$nullable) $v = 0; + if(is_null($v) && !$nullable) { + $v = 0; + } return Date::transform($v, "date"); case "time": - if(is_null($v) && !$nullable) $v = 0; + if(is_null($v) && !$nullable) { + $v = 0; + } return Date::transform($v, "time"); case "datetime": - if(is_null($v) && !$nullable) $v = 0; + if(is_null($v) && !$nullable) { + $v = 0; + } return Date::transform($v, "sql"); case "null": case "integer": @@ -58,7 +68,9 @@ abstract class AbstractStatement implements Statement { case "binary": case "string": case "boolean": - if($t=="binary") $t = "string"; + if($t=="binary") { + $t = "string"; + } if($v instanceof \DateTimeInterface) { if($t=="string") { return Date::transform($v, "sql"); diff --git a/lib/Db/SQLite3/Statement.php b/lib/Db/SQLite3/Statement.php index 2c39798d..05db69d0 100644 --- a/lib/Db/SQLite3/Statement.php +++ b/lib/Db/SQLite3/Statement.php @@ -59,7 +59,9 @@ class Statement extends \JKingWeb\Arsse\Db\AbstractStatement { $a += $this->bindValues($value, $a); } else if(array_key_exists($a,$this->types)) { // if the parameter type is something other than the known values, this is an error - if(!array_key_exists($this->types[$a], self::BINDINGS)) throw new Exception("paramTypeUnknown", $this->types[$a]); + if(!array_key_exists($this->types[$a], self::BINDINGS)) { + throw new Exception("paramTypeUnknown", $this->types[$a]); + } // if the parameter type is null or the value is null (and the type is nullable), just bind null if($this->types[$a]=="null" || ($this->isNullable[$a] && is_null($value))) { $this->st->bindValue($a+1, null, \SQLITE3_NULL); @@ -68,7 +70,9 @@ class Statement extends \JKingWeb\Arsse\Db\AbstractStatement { $type = self::BINDINGS[$this->types[$a]]; $value = $this->cast($value, $this->types[$a], $this->isNullable[$a]); // re-adjust for null casts - if($value===null) $type = \SQLITE3_NULL; + if($value===null) { + $type = \SQLITE3_NULL; + } // perform binding $this->st->bindValue($a+1, $value, $type); } diff --git a/lib/Feed.php b/lib/Feed.php index 3a6e2a03..847185e7 100644 --- a/lib/Feed.php +++ b/lib/Feed.php @@ -42,11 +42,17 @@ class Feed { // ascertain whether there are any articles not in the database $this->matchToDatabase($feedID); // if caching header fields are not sent by the server, try to ascertain a last-modified date from the feed contents - if(!$this->lastModified) $this->lastModified = $this->computeLastModified(); + if(!$this->lastModified) { + $this->lastModified = $this->computeLastModified(); + } // we only really care if articles have been modified; if there are no new articles, act as if the feed is unchanged - if(!sizeof($this->newItems) && !sizeof($this->changedItems)) $this->modified = false; + if(!sizeof($this->newItems) && !sizeof($this->changedItems)) { + $this->modified = false; + } // if requested, scrape full content for any new and changed items - if($scrape) $this->scrape(); + if($scrape) { + $this->scrape(); + } } // compute the time at which the feed should next be fetched $this->nextFetch = $this->computeNextFetch(); @@ -116,11 +122,17 @@ class Feed { // prefer an Atom ID as the item's ID $id = (string) $f->xml->children('http://www.w3.org/2005/Atom')->id; // otherwise use the RSS2 guid element - if(!strlen($id)) $id = (string) $f->xml->guid; + if(!strlen($id)) { + $id = (string) $f->xml->guid; + } // otherwise use the Dublin Core identifier element - if(!strlen($id)) $id = (string) $f->xml->children('http://purl.org/dc/elements/1.1/')->identifier; + if(!strlen($id)) { + $id = (string) $f->xml->children('http://purl.org/dc/elements/1.1/')->identifier; + } // otherwise there is no ID; if there is one, hash it - if(strlen($id)) $f->id = hash('sha256', $id); + if(strlen($id)) { + $f->id = hash('sha256', $id); + } // PicoFeed also doesn't gather up categories, so we do this as well $f->categories = []; @@ -129,19 +141,27 @@ class Feed { // if the category has a label, use that $name = (string) $c->attributes()->label; // otherwise use the term - if(!strlen($name)) $name = (string) $c->attributes()->term; + if(!strlen($name)) { + $name = (string) $c->attributes()->term; + } // ... assuming it has that much - if(strlen($name)) $f->categories[] = $name; + if(strlen($name)) { + $f->categories[] = $name; + } } // next add RSS2 categories foreach($f->xml->children()->category as $c) { $name = (string) $c; - if(strlen($name)) $f->categories[] = $name; + if(strlen($name)) { + $f->categories[] = $name; + } } // and finally try Dublin Core subjects foreach($f->xml->children('http://purl.org/dc/elements/1.1/')->subject as $c) { $name = (string) $c; - if(strlen($name)) $f->categories[] = $name; + if(strlen($name)) { + $f->categories[] = $name; + } } //sort the results sort($f->categories); @@ -161,7 +181,9 @@ class Feed { foreach($items as $item) { foreach($out as $index => $check) { // if the two items both have IDs and they differ, they do not match, regardless of hashes - if($item->id && $check->id && $item->id != $check->id) continue; + if($item->id && $check->id && $item->id != $check->id) { + continue; + } // if the two items have the same ID or any one hash matches, they are two versions of the same item if( ($item->id && $check->id && $item->id == $check->id) || @@ -208,10 +230,18 @@ class Feed { // if we need to, perform a second pass on the database looking specifically for IDs and hashes of the new items $ids = $hashesUT = $hashesUC = $hashesTC = []; foreach($this->newItems as $i) { - if($i->id) $ids[] = $i->id; - if($i->urlTitleHash) $hashesUT[] = $i->urlTitleHash; - if($i->urlContentHash) $hashesUC[] = $i->urlContentHash; - if($i->titleContentHash) $hashesTC[] = $i->titleContentHash; + if($i->id) { + $ids[] = $i->id; + } + if($i->urlTitleHash) { + $hashesUT[] = $i->urlTitleHash; + } + if($i->urlContentHash) { + $hashesUC[] = $i->urlContentHash; + } + if($i->titleContentHash) { + $hashesTC[] = $i->titleContentHash; + } } $articles = Arsse::$db->feedMatchIds($feedID, $ids, $hashesUT, $hashesUC, $hashesTC)->getAll(); list($this->newItems, $changed) = $this->matchItems($this->newItems, $articles); @@ -228,7 +258,9 @@ class Feed { $found = false; foreach($articles as $a) { // if the item has an ID and it doesn't match the article ID, the two don't match, regardless of hashes - if($i->id && $i->id !== $a['guid']) continue; + if($i->id && $i->id !== $a['guid']) { + continue; + } if( // the item matches if the GUID matches... ($i->id && $i->id === $a['guid']) || @@ -255,7 +287,9 @@ class Feed { } } } - if(!$found) $new[] = $i; + if(!$found) { + $new[] = $i; + } } return [$new, $edited]; } @@ -319,7 +353,9 @@ class Feed { } protected function computeLastModified() { - if(!$this->modified) return $this->lastModified; + if(!$this->modified) { + return $this->lastModified; + } $dates = $this->gatherDates(); if(sizeof($dates)) { return Date::normalize($dates[0]); @@ -331,8 +367,12 @@ class Feed { protected function gatherDates(): array { $dates = []; foreach($this->data->items as $item) { - if($item->updatedDate) $dates[] = $item->updatedDate->getTimestamp(); - if($item->publishedDate) $dates[] = $item->publishedDate->getTimestamp(); + if($item->updatedDate) { + $dates[] = $item->updatedDate->getTimestamp(); + } + if($item->publishedDate) { + $dates[] = $item->publishedDate->getTimestamp(); + } } $dates = array_unique($dates, \SORT_NUMERIC); rsort($dates); diff --git a/lib/Lang.php b/lib/Lang.php index 5b57813f..e64a182d 100644 --- a/lib/Lang.php +++ b/lib/Lang.php @@ -29,24 +29,32 @@ class Lang { public function set(string $locale, bool $immediate = false): string { // make sure the Intl extension is loaded - if(!static::$requirementsMet) static::checkRequirements(); + if(!static::$requirementsMet) { + static::checkRequirements(); + } // if requesting the same locale as already wanted, just return (but load first if we've requested an immediate load) if($locale==$this->wanted) { - if($immediate && !$this->synched) $this->load(); + if($immediate && !$this->synched) { + $this->load(); + } return $locale; } // if we've requested a locale other than the null locale, fetch the list of available files and find the closest match e.g. en_ca_somedialect -> en_ca if($locale != "") { $list = $this->listFiles(); // if the default locale is unavailable, this is (for now) an error - if(!in_array(self::DEFAULT, $list)) throw new Lang\Exception("defaultFileMissing", self::DEFAULT); + if(!in_array(self::DEFAULT, $list)) { + throw new Lang\Exception("defaultFileMissing", self::DEFAULT); + } $this->wanted = $this->match($locale, $list); } else { $this->wanted = ""; } $this->synched = false; // load right now if asked to, otherwise load later when actually required - if($immediate) $this->load(); + if($immediate) { + $this->load(); + } return $this->wanted; } @@ -74,7 +82,9 @@ class Lang { } // if the requested message is not present in any of the currently loaded language files, throw an exception // note that this is indicative of a programming error since the default locale should have all strings - if(!array_key_exists($msgID, $this->strings)) throw new Lang\Exception("stringMissing", ['msgID' => $msgID, 'fileList' => implode(", ",$this->loaded)]); + if(!array_key_exists($msgID, $this->strings)) { + throw new Lang\Exception("stringMissing", ['msgID' => $msgID, 'fileList' => implode(", ",$this->loaded)]); + } $msg = $this->strings[$msgID]; // variables fed to MessageFormatter must be contained in an array if($vars===null) { @@ -84,7 +94,9 @@ class Lang { $vars = [$vars]; } $msg = \MessageFormatter::formatMessage($this->locale, $msg, $vars); - if($msg===false) throw new Lang\Exception("stringInvalid", ['msgID' => $msgID, 'fileList' => implode(", ",$this->loaded)]); + if($msg===false) { + throw new Lang\Exception("stringInvalid", ['msgID' => $msgID, 'fileList' => implode(", ",$this->loaded)]); + } return $msg; } @@ -98,13 +110,17 @@ class Lang { } public function match(string $locale, array $list = null): string { - if($list===null) $list = $this->listFiles(); + if($list===null) { + $list = $this->listFiles(); + } $default = ($this->locale=="") ? self::DEFAULT : $this->locale; return \Locale::lookup($list,$locale, true, $default); } static protected function checkRequirements(): bool { - if(!extension_loaded("intl")) throw new ExceptionFatal("The \"Intl\" extension is required, but not loaded"); + if(!extension_loaded("intl")) { + throw new ExceptionFatal("The \"Intl\" extension is required, but not loaded"); + } static::$requirementsMet = true; return true; } @@ -128,7 +144,9 @@ class Lang { } protected function load(): bool { - if(!self::$requirementsMet) self::checkRequirements(); + if(!self::$requirementsMet) { + self::checkRequirements(); + } // if we've requested no locale (""), just load the fallback strings and return if($this->wanted=="") { $this->strings = self::REQUIRED; @@ -144,13 +162,17 @@ class Lang { $tag = array_pop($tags); } // include the default locale as the base if the most general locale requested is not the default - if($tag != self::DEFAULT) $files[] = self::DEFAULT; + if($tag != self::DEFAULT) { + $files[] = self::DEFAULT; + } // save the list of files to be loaded for later reference $loaded = $files; // reduce the list of files to be loaded to the minimum necessary (e.g. if we go from "fr" to "fr_ca", we don't need to load "fr" or "en") $files = []; foreach($loaded as $file) { - if($file==$this->locale) break; + if($file==$this->locale) { + break; + } $files[] = $file; } // if we need to load all files, start with the fallback strings @@ -164,8 +186,11 @@ class Lang { // read files in reverse order $files = array_reverse($files); foreach($files as $file) { - if(!file_exists($this->path."$file.php")) throw new Lang\Exception("fileMissing", $file); - if(!is_readable($this->path."$file.php")) throw new Lang\Exception("fileUnreadable", $file); + if(!file_exists($this->path."$file.php")) { + throw new Lang\Exception("fileMissing", $file); + } else if(!is_readable($this->path."$file.php")) { + throw new Lang\Exception("fileUnreadable", $file); + } try { // we use output buffering in case the language file is corrupted ob_start(); @@ -175,7 +200,9 @@ class Lang { } finally { ob_end_clean(); } - if(!is_array($arr)) throw new Lang\Exception("fileCorrupt", $file); + if(!is_array($arr)) { + throw new Lang\Exception("fileCorrupt", $file); + } $strings[] = $arr; } // apply the results and return diff --git a/lib/Misc/Context.php b/lib/Misc/Context.php index 4f7847a7..a52105ef 100644 --- a/lib/Misc/Context.php +++ b/lib/Misc/Context.php @@ -36,15 +36,17 @@ class Context { $spec = array_values($spec); for($a = 0; $a < sizeof($spec); $a++) { $id = $spec[$a]; - if(is_int($id) && $id > -1) continue; - if(is_float($id) && !fmod($id, 1) && $id >= 0) { + if(is_int($id) && $id > -1) { + continue; + } else if(is_float($id) && !fmod($id, 1) && $id >= 0) { $spec[$a] = (int) $id; continue; - } - if(is_string($id)) { + } else if(is_string($id)) { $ch1 = strval(@intval($id)); $ch2 = strval($id); - if($ch1 !== $ch2 || $id < 1) $id = 0; + if($ch1 !== $ch2 || $id < 1) { + $id = 0; + } } else { $id = 0; } @@ -108,12 +110,16 @@ class Context { } function editions(array $spec = null) { - if($spec) $spec = $this->cleanArray($spec); + if($spec) { + $spec = $this->cleanArray($spec); + } return $this->act(__FUNCTION__, func_num_args(), $spec); } function articles(array $spec = null) { - if($spec) $spec = $this->cleanArray($spec); + if($spec) { + $spec = $this->cleanArray($spec); + } return $this->act(__FUNCTION__, func_num_args(), $spec); } } \ No newline at end of file diff --git a/lib/Misc/Date.php b/lib/Misc/Date.php index 4c9abe94..1bb20443 100644 --- a/lib/Misc/Date.php +++ b/lib/Misc/Date.php @@ -6,9 +6,13 @@ class Date { static function transform($date, string $outFormat = null, string $inFormat = null, bool $inLocal = false) { $date = self::normalize($date, $inFormat, $inLocal); - if(is_null($date) || is_null($outFormat)) return $date; + if(is_null($date) || is_null($outFormat)) { + return $date; + } $outFormat = strtolower($outFormat); - if($outFormat=="unix") return $date->getTimestamp(); + if($outFormat=="unix") { + return $date->getTimestamp(); + } switch ($outFormat) { case 'http': $f = "D, d M Y H:i:s \G\M\T"; break; case 'iso8601': $f = "Y-m-d\TH:i:s"; break; diff --git a/lib/Misc/Query.php b/lib/Misc/Query.php index a25d195d..1bd3dd16 100644 --- a/lib/Misc/Query.php +++ b/lib/Misc/Query.php @@ -37,7 +37,9 @@ class Query { $this->tCTE[] = $types; $this->vCTE[] = $values; } - if(strlen($join)) $this->jCTE[] = $join; // the CTE might only participate in subqueries rather than a join on the main query + if(strlen($join)) { // the CTE might only participate in subqueries rather than a join on the main query + $this->jCTE[] = $join; + } return true; } @@ -77,7 +79,9 @@ class Query { $this->vWhere = []; $this->order = []; $this->setLimit(0,0); - if(strlen($join)) $this->jCTE[] = $join; + if(strlen($join)) { + $this->jCTE[] = $join; + } return true; } diff --git a/lib/REST.php b/lib/REST.php index 8c2457c0..764f9e23 100644 --- a/lib/REST.php +++ b/lib/REST.php @@ -31,7 +31,9 @@ class REST { } function dispatch(REST\Request $req = null): bool { - if($req===null) $req = new REST\Request(); + if($req===null) { + $req = new REST\Request(); + } $api = $this->apiMatch($req->url, $this->apis); $req->url = substr($req->url,strlen($this->apis[$api]['strip'])); $req->refreshURL(); @@ -62,7 +64,9 @@ class REST { uasort($map, function($a, $b) {return (strlen($a['match']) <=> strlen($b['match'])) * -1;}); // find a match foreach($map as $id => $api) { - if(strpos($url, $api['match'])===0) return $id; + if(strpos($url, $api['match'])===0) { + return $id; + } } // or throw an exception otherwise throw new REST\Exception501(); diff --git a/lib/REST/AbstractHandler.php b/lib/REST/AbstractHandler.php index df91c487..a39bbb69 100644 --- a/lib/REST/AbstractHandler.php +++ b/lib/REST/AbstractHandler.php @@ -50,7 +50,9 @@ abstract class AbstractHandler implements Handler { } switch($types[$key]) { case "int": - if($this->validateInt($value)) $out[$key] = (int) $value; + if($this->validateInt($value)) { + $out[$key] = (int) $value; + } break; case "string": $out[$key] = (string) $value; @@ -60,19 +62,29 @@ abstract class AbstractHandler implements Handler { $out[$key] = $value; } else if($this->validateInt($value)) { $value = (int) $value; - if($value > -1 && $value < 2) $out[$key] = $value; + if($value > -1 && $value < 2) { + $out[$key] = $value; + } } else if(is_string($value)) { $value = trim(strtolower($value)); - if($value=="false") $out[$key] = false; - if($value=="true") $out[$key] = true; + if($value=="false") { + $out[$key] = false; + } + if($value=="true") { + $out[$key] = true; + } } break; case "float": - if(is_numeric($value)) $out[$key] = (float) $value; + if(is_numeric($value)) { + $out[$key] = (float) $value; + } break; case "datetime": $t = Date::normalize($value, $dateFormat); - if($t) $out[$key] = $t; + if($t) { + $out[$key] = $t; + } break; default: throw new Exception("typeUnknown", $types[$key]); diff --git a/lib/REST/NextCloudNews/V1_2.php b/lib/REST/NextCloudNews/V1_2.php index 5015aedb..55504c2c 100644 --- a/lib/REST/NextCloudNews/V1_2.php +++ b/lib/REST/NextCloudNews/V1_2.php @@ -40,13 +40,15 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { function dispatch(\JKingWeb\Arsse\REST\Request $req): Response { // try to authenticate - if(!Arsse::$user->authHTTP()) return new Response(401, "", "", ['WWW-Authenticate: Basic realm="'.self::REALM.'"']); - // only accept GET, POST, PUT, or DELETE - if(!in_array($req->method, ["GET", "POST", "PUT", "DELETE"])) return new Response(405, "", "", ['Allow: GET, POST, PUT, DELETE']); + if(!Arsse::$user->authHTTP()) { + return new Response(401, "", "", ['WWW-Authenticate: Basic realm="'.self::REALM.'"']); + } // normalize the input if($req->body) { // if the entity body is not JSON according to content type, return "415 Unsupported Media Type" - if(!preg_match("<^application/json\b|^$>", $req->type)) return new Response(415, "", "", ['Accept: application/json']); + if(!preg_match("<^application/json\b|^$>", $req->type)) { + return new Response(415, "", "", ['Accept: application/json']); + } $data = @json_decode($req->body, true); if(json_last_error() != \JSON_ERROR_NONE) { // if the body could not be parsed as JSON, return "400 Bad Request" @@ -68,7 +70,9 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { } catch(Exception405 $e) { return new Response(405, "", "", ["Allow: ".$e->getMessage()]); } - if(!method_exists($this, $func)) return new Response(501); + if(!method_exists($this, $func)) { + return new Response(501); + } // dispatch try { return $this->$func($req->paths, $data); @@ -129,12 +133,16 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { $scope = $url[0]; // any URL components which are only digits should be replaced with "0", for easier comparison (integer segments are IDs, and we don't care about the specific ID) for($a = 0; $a < sizeof($url); $a++) { - if($this->validateInt($url[$a])) $url[$a] = "0"; + if($this->validateInt($url[$a])) { + $url[$a] = "0"; + } } // normalize the HTTP method to uppercase $method = strtoupper($method); // if the scope is not supported, return 501 - if(!array_key_exists($scope, $choices)) throw new Exception501(); + if(!array_key_exists($scope, $choices)) { + throw new Exception501(); + } // we now evaluate the supplied URL against every supported path for the selected scope // the URL is evaluated as an array so as to avoid decoded escapes turning invalid URLs into valid ones foreach($choices[$scope] as $path => $funcs) { @@ -251,7 +259,9 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { // rename a folder (also supports moving nesting folders, but this is not a feature of the API) protected function folderRename(array $url, array $data): Response { // there must be some change to be made - if(!sizeof($data)) return new Response(422); + if(!sizeof($data)) { + return new Response(422); + } // perform the edit try { Arsse::$db->folderPropertiesSet(Arsse::$user->id, (int) $url[1], $data); @@ -296,7 +306,9 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { // return list of feeds which should be refreshed protected function feedListStale(array $url, array $data): Response { // function requires admin rights per spec - if(Arsse::$user->rightsGet(Arsse::$user->id)==User::RIGHTS_NONE) return new Response(403); + if(Arsse::$user->rightsGet(Arsse::$user->id)==User::RIGHTS_NONE) { + return new Response(403); + } // list stale feeds which should be checked for updates $feeds = Arsse::$db->feedListStale(); $out = []; @@ -310,9 +322,13 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { // refresh a feed protected function feedUpdate(array $url, array $data): Response { // function requires admin rights per spec - if(Arsse::$user->rightsGet(Arsse::$user->id)==User::RIGHTS_NONE) return new Response(403); + if(Arsse::$user->rightsGet(Arsse::$user->id)==User::RIGHTS_NONE) { + return new Response(403); + } // perform an update of a single feed - if(!isset($data['feedId'])) return new Response(422); + if(!isset($data['feedId'])) { + return new Response(422); + } try { Arsse::$db->feedUpdate($data['feedId']); } catch(ExceptionInput $e) { @@ -324,7 +340,9 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { // add a new feed protected function subscriptionAdd(array $url, array $data): Response { // normalize the feed URL - if(!isset($data['url'])) return new Response(422); + if(!isset($data['url'])) { + return new Response(422); + } // normalize the folder ID, if specified; zero should be transformed to null $folder = (isset($data['folderId']) && $data['folderId']) ? $data['folderId'] : null; // try to add the feed @@ -350,7 +368,9 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { $feed = $this->feedTranslate($feed); $out = ['feeds' => [$feed]]; $newest = Arsse::$db->editionLatest(Arsse::$user->id, (new Context)->subscription($id)); - if($newest) $out['newestItemId'] = $newest; + if($newest) { + $out['newestItemId'] = $newest; + } return new Response(200, $out); } @@ -364,7 +384,9 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { $out = ['feeds' => $out]; $out['starredCount'] = Arsse::$db->articleStarredCount(Arsse::$user->id); $newest = Arsse::$db->editionLatest(Arsse::$user->id); - if($newest) $out['newestItemId'] = $newest; + if($newest) { + $out['newestItemId'] = $newest; + } return new Response(200, $out); } @@ -456,7 +478,9 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { // set the context options supplied by the client $c = new Context; // set the batch size - if(isset($data['batchSize']) && $data['batchSize'] > 0) $c->limit($data['batchSize']); + if(isset($data['batchSize']) && $data['batchSize'] > 0) { + $c->limit($data['batchSize']); + } // set the order of returned items if(isset($data['oldestFirst']) && $data['oldestFirst']) { $c->reverse(false); @@ -472,15 +496,23 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { } } // set whether to only return unread - if(isset($data['getRead']) && !$data['getRead']) $c->unread(true); + if(isset($data['getRead']) && !$data['getRead']) { + $c->unread(true); + } // if no type is specified assume 3 (All) - if(!isset($data['type'])) $data['type'] = 3; + if(!isset($data['type'])) { + $data['type'] = 3; + } switch($data['type']) { case 0: // feed - if(isset($data['id'])) $c->subscription($data['id']); + if(isset($data['id'])) { + $c->subscription($data['id']); + } break; case 1: // folder - if(isset($data['id'])) $c->folder($data['id']); + if(isset($data['id'])) { + $c->folder($data['id']); + } break; case 2: // starred $c->starred(true); @@ -489,7 +521,9 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { // return all items } // whether to return only updated items - if(isset($data['lastModified'])) $c->modifiedSince($data['lastModified']); + if(isset($data['lastModified'])) { + $c->modifiedSince($data['lastModified']); + } // perform the fetch try { $items = Arsse::$db->articleList(Arsse::$user->id, $c); @@ -557,7 +591,9 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { // determine whether to mark read or unread $set = ($url[1]=="read"); // if the input data is not at all valid, return an error - if(!isset($data['items']) || !is_array($data['items'])) return new Response(422); + if(!isset($data['items']) || !is_array($data['items'])) { + return new Response(422); + } // start a transaction and loop through the items $t = Arsse::$db->begin(); $in = array_chunk($data['items'], 50); @@ -578,7 +614,9 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { // determine whether to mark starred or unstarred $set = ($url[1]=="star"); // if the input data is not at all valid, return an error - if(!isset($data['items']) || !is_array($data['items'])) return new Response(422); + if(!isset($data['items']) || !is_array($data['items'])) { + return new Response(422); + } // start a transaction and loop through the items $t = Arsse::$db->begin(); $in = array_chunk(array_column($data['items'], "guidHash"), 50); @@ -617,14 +655,18 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { protected function cleanupBefore(array $url, array $data): Response { // function requires admin rights per spec - if(Arsse::$user->rightsGet(Arsse::$user->id)==User::RIGHTS_NONE) return new Response(403); + if(Arsse::$user->rightsGet(Arsse::$user->id)==User::RIGHTS_NONE) { + return new Response(403); + } // FIXME: stub return new Response(204); } protected function cleanupAfter(array $url, array $data): Response { // function requires admin rights per spec - if(Arsse::$user->rightsGet(Arsse::$user->id)==User::RIGHTS_NONE) return new Response(403); + if(Arsse::$user->rightsGet(Arsse::$user->id)==User::RIGHTS_NONE) { + return new Response(403); + } // FIXME: stub return new Response(204); } diff --git a/lib/REST/Request.php b/lib/REST/Request.php index 7a129591..2904e0f1 100644 --- a/lib/REST/Request.php +++ b/lib/REST/Request.php @@ -12,9 +12,15 @@ class Request { public $body = ""; function __construct(string $method = null, string $url = null, string $body = null, string $contentType = null) { - if(is_null($method)) $method = $_SERVER['REQUEST_METHOD']; - if(is_null($url)) $url = $_SERVER['REQUEST_URI']; - if(is_null($body)) $body = file_get_contents("php://input"); + if(is_null($method)) { + $method = $_SERVER['REQUEST_METHOD']; + } + if(is_null($url)) { + $url = $_SERVER['REQUEST_URI']; + } + if(is_null($body)) { + $body = file_get_contents("php://input"); + } if(is_null($contentType)) { if(isset($_SERVER['HTTP_CONTENT_TYPE'])) { $contentType = $_SERVER['HTTP_CONTENT_TYPE']; @@ -63,8 +69,12 @@ class Request { if(!in_array($out['path'],["/",""])) { $paths = explode("/", $out['path']); // remove the first and last empty elements, if present (they are artefacts of the splitting; others should remain) - if(!strlen($paths[0])) array_shift($paths); - if(!strlen($paths[sizeof($paths)-1])) array_pop($paths); + if(!strlen($paths[0])) { + array_shift($paths); + } + if(!strlen($paths[sizeof($paths)-1])) { + array_pop($paths); + } // %-decode each path element $paths = array_map(function($v){return rawurldecode($v);}, $paths); $out['paths'] = $paths; diff --git a/lib/Service.php b/lib/Service.php index f5f0aab1..191bf140 100644 --- a/lib/Service.php +++ b/lib/Service.php @@ -66,7 +66,9 @@ class Service { static function hasCheckedIn(): bool { $checkin = Arsse::$db->metaGet("service_last_checkin"); // if the service has never checked in, return false - if(!$checkin) return false; + if(!$checkin) { + return false; + } // convert the check-in timestamp to a DateTime instance $checkin = Date::normalize($checkin, "sql"); // get the checking interval diff --git a/lib/Service/Curl/Driver.php b/lib/Service/Curl/Driver.php index b8cdd847..b41c9bef 100644 --- a/lib/Service/Curl/Driver.php +++ b/lib/Service/Curl/Driver.php @@ -42,7 +42,7 @@ class Driver implements \JKingWeb\Arsse\Service\Driver { curl_multi_setopt($this->queue, \CURLMOPT_PIPELINING, 1); } - function qeueue(int ...$feeds): int { + function queue(int ...$feeds): int { foreach($feeds as $id) { $h = curl_init(); curl_setopt($h, \CURLOPT_POSTFIELDS, json_encode(['userId' => "", 'feedId' => $id])); diff --git a/lib/User.php b/lib/User.php index b8a89f59..64968153 100644 --- a/lib/User.php +++ b/lib/User.php @@ -37,54 +37,86 @@ class User { } public function __toString() { - if($this->id===null) $this->credentials(); + if($this->id===null) { + $this->credentials(); + } return (string) $this->id; } // checks whether the logged in user is authorized to act for the affected user (used especially when granting rights) function authorize(string $affectedUser, string $action, int $newRightsLevel = 0): bool { // if authorization checks are disabled (either because we're running the installer or the background updater) just return true - if(!$this->authorizationEnabled()) return true; + if(!$this->authorizationEnabled()) { + return true; + } // if we don't have a logged-in user, fetch credentials - if($this->id===null) $this->credentials(); + if($this->id===null) { + $this->credentials(); + } // if the affected user is the actor and the actor is not trying to grant themselves rights, accept the request - if($affectedUser==Arsse::$user->id && $action != "userRightsSet") return true; + if($affectedUser==Arsse::$user->id && $action != "userRightsSet") { + return true; + } // if we're authorizing something other than a user function and the affected user is not the actor, make sure the affected user exists $this->authorizationEnabled(false); - if(Arsse::$user->id != $affectedUser && strpos($action, "user")!==0 && !$this->exists($affectedUser)) throw new User\Exception("doesNotExist", ["action" => $action, "user" => $affectedUser]); + if(Arsse::$user->id != $affectedUser && strpos($action, "user")!==0 && !$this->exists($affectedUser)) { + throw new User\Exception("doesNotExist", ["action" => $action, "user" => $affectedUser]); + } $this->authorizationEnabled(true); // get properties of actor if not already available - if(!sizeof($this->actor)) $this->actor = $this->propertiesGet(Arsse::$user->id); + if(!sizeof($this->actor)) { + $this->actor = $this->propertiesGet(Arsse::$user->id); + } $rights = $this->actor["rights"]; // if actor is a global admin, accept the request - if($rights==User\Driver::RIGHTS_GLOBAL_ADMIN) return true; + if($rights==User\Driver::RIGHTS_GLOBAL_ADMIN) { + return true; + } // if actor is a common user, deny the request - if($rights==User\Driver::RIGHTS_NONE) return false; + if($rights==User\Driver::RIGHTS_NONE) { + return false; + } // if actor is not some other sort of admin, deny the request - if(!in_array($rights,[User\Driver::RIGHTS_GLOBAL_MANAGER,User\Driver::RIGHTS_DOMAIN_MANAGER,User\Driver::RIGHTS_DOMAIN_ADMIN],true)) return false; + if(!in_array($rights,[User\Driver::RIGHTS_GLOBAL_MANAGER,User\Driver::RIGHTS_DOMAIN_MANAGER,User\Driver::RIGHTS_DOMAIN_ADMIN],true)) { + return false; + } // if actor is a domain admin/manager and domains don't match, deny the request if(Arsse::$conf->userComposeNames && $this->actor["domain"] && $rights != User\Driver::RIGHTS_GLOBAL_MANAGER) { $test = "@".$this->actor["domain"]; - if(substr($affectedUser,-1*strlen($test)) != $test) return false; + if(substr($affectedUser,-1*strlen($test)) != $test) { + return false; + } } // certain actions shouldn't check affected user's rights - if(in_array($action, ["userRightsGet","userExists","userList"], true)) return true; + if(in_array($action, ["userRightsGet","userExists","userList"], true)) { + return true; + } if($action=="userRightsSet") { // setting rights above your own is not allowed - if($newRightsLevel > $rights) return false; + if($newRightsLevel > $rights) { + return false; + } // setting yourself to rights you already have is harmless and can be allowed - if($this->id==$affectedUser && $newRightsLevel==$rights) return true; + if($this->id==$affectedUser && $newRightsLevel==$rights) { + return true; + } // managers can only set their own rights, and only to normal user if(in_array($rights, [User\Driver::RIGHTS_DOMAIN_MANAGER, User\Driver::RIGHTS_GLOBAL_MANAGER])) { - if($this->id != $affectedUser || $newRightsLevel != User\Driver::RIGHTS_NONE) return false; + if($this->id != $affectedUser || $newRightsLevel != User\Driver::RIGHTS_NONE) { + return false; + } return true; } } $affectedRights = $this->rightsGet($affectedUser); // managers can only act on themselves (checked above) or regular users - if(in_array($rights,[User\Driver::RIGHTS_GLOBAL_MANAGER,User\Driver::RIGHTS_DOMAIN_MANAGER]) && $affectedRights != User\Driver::RIGHTS_NONE) return false; + if(in_array($rights,[User\Driver::RIGHTS_GLOBAL_MANAGER,User\Driver::RIGHTS_DOMAIN_MANAGER]) && $affectedRights != User\Driver::RIGHTS_NONE) { + return false; + } // domain admins canot act above themselves - if(!in_array($affectedRights,[User\Driver::RIGHTS_NONE,User\Driver::RIGHTS_DOMAIN_MANAGER,User\Driver::RIGHTS_DOMAIN_ADMIN])) return false; + if(!in_array($affectedRights,[User\Driver::RIGHTS_NONE,User\Driver::RIGHTS_DOMAIN_MANAGER,User\Driver::RIGHTS_DOMAIN_ADMIN])) { + return false; + } return true; } @@ -116,11 +148,15 @@ class User { } else { $out = $this->u->auth($user, $password); } - if($out && !Arsse::$db->userExists($user)) $this->autoProvision($user, $password); + if($out && !Arsse::$db->userExists($user)) { + $this->autoProvision($user, $password); + } return $out; case User\Driver::FUNC_INTERNAL: if(Arsse::$conf->userPreAuth) { - if(!Arsse::$db->userExists($user)) $this->autoProvision($user, $password); + if(!Arsse::$db->userExists($user)) { + $this->autoProvision($user, $password); + } return true; } else { return $this->u->auth($user, $password); @@ -133,7 +169,9 @@ class User { public function authHTTP(): bool { $cred = $this->credentials(); - if(!$cred["user"]) return false; + if(!$cred["user"]) { + return false; + } return $this->auth($cred["user"], $cred["password"]); } @@ -147,9 +185,13 @@ class User { case User\Driver::FUNC_EXTERNAL: // we handle authorization checks for external drivers if($domain===null) { - if(!$this->authorize("@".$domain, $func)) throw new User\ExceptionAuthz("notAuthorized", ["action" => $func, "user" => $domain]); + if(!$this->authorize("@".$domain, $func)) { + throw new User\ExceptionAuthz("notAuthorized", ["action" => $func, "user" => $domain]); + } } else { - if(!$this->authorize("", $func)) throw new User\ExceptionAuthz("notAuthorized", ["action" => $func, "user" => "all users"]); + if(!$this->authorize("", $func)) { + throw new User\ExceptionAuthz("notAuthorized", ["action" => $func, "user" => "all users"]); + } } case User\Driver::FUNC_INTERNAL: // internal functions handle their own authorization @@ -160,9 +202,13 @@ class User { } public function authorizationEnabled(bool $setting = null): bool { - if(is_null($setting)) return !$this->authz; + if(is_null($setting)) { + return !$this->authz; + } $this->authz += ($setting ? -1 : 1); - if($this->authz < 0) $this->authz = 0; + if($this->authz < 0) { + $this->authz = 0; + } return !$this->authz; } @@ -171,9 +217,13 @@ class User { switch($this->u->driverFunctions($func)) { case User\Driver::FUNC_EXTERNAL: // we handle authorization checks for external drivers - if(!$this->authorize($user, $func)) throw new User\ExceptionAuthz("notAuthorized", ["action" => $func, "user" => $user]); + if(!$this->authorize($user, $func)) { + throw new User\ExceptionAuthz("notAuthorized", ["action" => $func, "user" => $user]); + } $out = $this->u->userExists($user); - if($out && !Arsse::$db->userExists($user)) $this->autoProvision($user, ""); + if($out && !Arsse::$db->userExists($user)) { + $this->autoProvision($user, ""); + } return $out; case User\Driver::FUNC_INTERNAL: // internal functions handle their own authorization @@ -189,10 +239,14 @@ class User { switch($this->u->driverFunctions($func)) { case User\Driver::FUNC_EXTERNAL: // we handle authorization checks for external drivers - if(!$this->authorize($user, $func)) throw new User\ExceptionAuthz("notAuthorized", ["action" => $func, "user" => $user]); + if(!$this->authorize($user, $func)) { + throw new User\ExceptionAuthz("notAuthorized", ["action" => $func, "user" => $user]); + } $newPassword = $this->u->userAdd($user, $password); // if there was no exception and we don't have the user in the internal database, add it - if(!Arsse::$db->userExists($user)) $this->autoProvision($user, $newPassword); + if(!Arsse::$db->userExists($user)) { + $this->autoProvision($user, $newPassword); + } return $newPassword; case User\Driver::FUNC_INTERNAL: // internal functions handle their own authorization @@ -207,11 +261,15 @@ class User { switch($this->u->driverFunctions($func)) { case User\Driver::FUNC_EXTERNAL: // we handle authorization checks for external drivers - if(!$this->authorize($user, $func)) throw new User\ExceptionAuthz("notAuthorized", ["action" => $func, "user" => $user]); + if(!$this->authorize($user, $func)) { + throw new User\ExceptionAuthz("notAuthorized", ["action" => $func, "user" => $user]); + } $out = $this->u->userRemove($user); if($out && Arsse::$db->userExists($user)) { // if the user was removed and we have it in our data, remove it there - if(!Arsse::$db->userExists($user)) Arsse::$db->userRemove($user); + if(!Arsse::$db->userExists($user)) { + Arsse::$db->userRemove($user); + } } return $out; case User\Driver::FUNC_INTERNAL: @@ -227,7 +285,9 @@ class User { switch($this->u->driverFunctions($func)) { case User\Driver::FUNC_EXTERNAL: // we handle authorization checks for external drivers - if(!$this->authorize($user, $func)) throw new User\ExceptionAuthz("notAuthorized", ["action" => $func, "user" => $user]); + if(!$this->authorize($user, $func)) { + throw new User\ExceptionAuthz("notAuthorized", ["action" => $func, "user" => $user]); + } $out = $this->u->userPasswordSet($user, $newPassword, $oldPassword); if(Arsse::$db->userExists($user)) { // if the password change was successful and the user exists, set the internal password to the same value @@ -248,7 +308,9 @@ class User { public function propertiesGet(string $user, bool $withAvatar = false): array { // prepare default values $domain = null; - if(Arsse::$conf->userComposeNames) $domain = substr($user,strrpos($user,"@")+1); + if(Arsse::$conf->userComposeNames) { + $domain = substr($user,strrpos($user,"@")+1); + } $init = [ "id" => $user, "name" => $user, @@ -259,12 +321,18 @@ class User { switch($this->u->driverFunctions($func)) { case User\Driver::FUNC_EXTERNAL: // we handle authorization checks for external drivers - if(!$this->authorize($user, $func)) throw new User\ExceptionAuthz("notAuthorized", ["action" => $func, "user" => $user]); + if(!$this->authorize($user, $func)) { + throw new User\ExceptionAuthz("notAuthorized", ["action" => $func, "user" => $user]); + } $out = array_merge($init, $this->u->userPropertiesGet($user)); // remove password if it is return (not exhaustive, but...) - if(array_key_exists('password', $out)) unset($out['password']); + if(array_key_exists('password', $out)) { + unset($out['password']); + } // if the user does not exist in the internal database, add it - if(!Arsse::$db->userExists($user)) $this->autoProvision($user, "", $out); + if(!Arsse::$db->userExists($user)) { + $this->autoProvision($user, "", $out); + } return $out; case User\Driver::FUNC_INTERNAL: // internal functions handle their own authorization @@ -278,13 +346,17 @@ class User { public function propertiesSet(string $user, array $properties): array { // remove from the array any values which should be set specially foreach(['id', 'domain', 'password', 'rights'] as $key) { - if(array_key_exists($key, $properties)) unset($properties[$key]); + if(array_key_exists($key, $properties)) { + unset($properties[$key]); + } } $func = "userPropertiesSet"; switch($this->u->driverFunctions($func)) { case User\Driver::FUNC_EXTERNAL: // we handle authorization checks for external drivers - if(!$this->authorize($user, $func)) throw new User\ExceptionAuthz("notAuthorized", ["action" => $func, "user" => $user]); + if(!$this->authorize($user, $func)) { + throw new User\ExceptionAuthz("notAuthorized", ["action" => $func, "user" => $user]); + } $out = $this->u->userPropertiesSet($user, $properties); if(Arsse::$db->userExists($user)) { // if the property change was successful and the user exists, set the internal properties to the same values @@ -307,10 +379,14 @@ class User { switch($this->u->driverFunctions($func)) { case User\Driver::FUNC_EXTERNAL: // we handle authorization checks for external drivers - if(!$this->authorize($user, $func)) throw new User\ExceptionAuthz("notAuthorized", ["action" => $func, "user" => $user]); + if(!$this->authorize($user, $func)) { + throw new User\ExceptionAuthz("notAuthorized", ["action" => $func, "user" => $user]); + } $out = $this->u->userRightsGet($user); // if the user does not exist in the internal database, add it - if(!Arsse::$db->userExists($user)) $this->autoProvision($user, "", null, $out); + if(!Arsse::$db->userExists($user)) { + $this->autoProvision($user, "", null, $out); + } return $out; case User\Driver::FUNC_INTERNAL: // internal functions handle their own authorization @@ -326,7 +402,9 @@ class User { switch($this->u->driverFunctions($func)) { case User\Driver::FUNC_EXTERNAL: // we handle authorization checks for external drivers - if(!$this->authorize($user, $func)) throw new User\ExceptionAuthz("notAuthorized", ["action" => $func, "user" => $user]); + if(!$this->authorize($user, $func)) { + throw new User\ExceptionAuthz("notAuthorized", ["action" => $func, "user" => $user]); + } $out = $this->u->userRightsSet($user, $level); // if the user does not exist in the internal database, add it if($out && Arsse::$db->userExists($user)) { @@ -365,7 +443,9 @@ class User { if($properties===null) { // if nothing is provided but the driver uses an external function, try to get the current values from the external source try { - if($this->u->driverFunctions("userPropertiesGet")==User\Driver::FUNC_EXTERNAL) Arsse::$db->userPropertiesSet($user, $this->u->userPropertiesGet($user)); + if($this->u->driverFunctions("userPropertiesGet")==User\Driver::FUNC_EXTERNAL) { + Arsse::$db->userPropertiesSet($user, $this->u->userPropertiesGet($user)); + } } catch(\Throwable $e) {} } else { // otherwise if values are provided, use those diff --git a/lib/User/Internal/Driver.php b/lib/User/Internal/Driver.php index 3f6cba6f..bdd5c36a 100644 --- a/lib/User/Internal/Driver.php +++ b/lib/User/Internal/Driver.php @@ -24,7 +24,9 @@ final class Driver implements \JKingWeb\Arsse\User\Driver { } public function driverFunctions(string $function = null) { - if($function===null) return $this->functions; + if($function===null) { + return $this->functions; + } if(array_key_exists($function, $this->functions)) { return $this->functions[$function]; } else { diff --git a/lib/User/Internal/InternalFunctions.php b/lib/User/Internal/InternalFunctions.php index f031a5c2..9e8862e6 100644 --- a/lib/User/Internal/InternalFunctions.php +++ b/lib/User/Internal/InternalFunctions.php @@ -2,54 +2,59 @@ declare(strict_types=1); namespace JKingWeb\Arsse\User\Internal; use JKingWeb\Arsse\Arsse; +use JKingWeb\Arsse\User\Exception; trait InternalFunctions { protected $actor = []; public function __construct() { - $this->db = Arsse::$db; } function auth(string $user, string $password): bool { - if(!Arsse::$user->exists($user)) return false; - $hash = $this->db->userPasswordGet($user); - if($password==="" && $hash==="") return true; + try { + $hash = Arsse::$db->userPasswordGet($user); + } catch(Exception $e) { + return false; + } + if($password==="" && $hash==="") { + return true; + } return password_verify($password, $hash); } function userExists(string $user): bool { - return $this->db->userExists($user); + return Arsse::$db->userExists($user); } function userAdd(string $user, string $password = null): string { - return $this->db->userAdd($user, $password); + return Arsse::$db->userAdd($user, $password); } function userRemove(string $user): bool { - return $this->db->userRemove($user); + return Arsse::$db->userRemove($user); } function userList(string $domain = null): array { - return $this->db->userList($domain); + return Arsse::$db->userList($domain); } function userPasswordSet(string $user, string $newPassword = null, string $oldPassword = null): string { - return $this->db->userPasswordSet($user, $newPassword); + return Arsse::$db->userPasswordSet($user, $newPassword); } function userPropertiesGet(string $user): array { - return $this->db->userPropertiesGet($user); + return Arsse::$db->userPropertiesGet($user); } function userPropertiesSet(string $user, array $properties): array { - return $this->db->userPropertiesSet($user, $properties); + return Arsse::$db->userPropertiesSet($user, $properties); } function userRightsGet(string $user): int { - return $this->db->userRightsGet($user); + return Arsse::$db->userRightsGet($user); } function userRightsSet(string $user, int $level): bool { - return $this->db->userRightsSet($user, $level); + return Arsse::$db->userRightsSet($user, $level); } } \ No newline at end of file diff --git a/tests/Misc/TestContext.php b/tests/Misc/TestContext.php index 9d20bc75..d1615131 100644 --- a/tests/Misc/TestContext.php +++ b/tests/Misc/TestContext.php @@ -9,7 +9,9 @@ class TestContext extends Test\AbstractTest { function testVerifyInitialState() { $c = new Context; foreach((new \ReflectionObject($c))->getMethods(\ReflectionMethod::IS_PUBLIC) as $m) { - if($m->isConstructor() || $m->isStatic()) continue; + if($m->isConstructor() || $m->isStatic()) { + continue; + } $method = $m->name; $this->assertFalse($c->$method(), "Context method $method did not initially return false"); $this->assertEquals(null, $c->$method, "Context property $method is not initially falsy"); @@ -37,7 +39,9 @@ class TestContext extends Test\AbstractTest { $times = ['modifiedSince','notModifiedSince']; $c = new Context; foreach((new \ReflectionObject($c))->getMethods(\ReflectionMethod::IS_PUBLIC) as $m) { - if($m->isConstructor() || $m->isStatic()) continue; + if($m->isConstructor() || $m->isStatic()) { + continue; + } $method = $m->name; $this->assertArrayHasKey($method, $v, "Context method $method not included in test"); $this->assertInstanceOf(Context::class, $c->$method($v[$method])); diff --git a/tests/REST/NextCloudNews/TestNCNV1_2.php b/tests/REST/NextCloudNews/TestNCNV1_2.php index 82c80c78..d58a47ad 100644 --- a/tests/REST/NextCloudNews/TestNCNV1_2.php +++ b/tests/REST/NextCloudNews/TestNCNV1_2.php @@ -10,7 +10,7 @@ use JKingWeb\Arsse\Db\ExceptionInput; use JKingWeb\Arsse\Db\Transaction; use Phake; -/** @covers \JKingWeb\Arsse\REST\NextCloudNews\V1_2 */ +/** @covers \JKingWeb\Arsse\REST\NextCloudNews\V1_2 */ class TestNCNV1_2 extends Test\AbstractTest { protected $h; protected $feeds = [ // expected sample output of a feed list from the database, and the resultant expected transformation by the REST handler diff --git a/tests/User/TestAuthorization.php b/tests/User/TestAuthorization.php index 2841707b..b443cf54 100644 --- a/tests/User/TestAuthorization.php +++ b/tests/User/TestAuthorization.php @@ -87,7 +87,9 @@ class TestAuthorization extends Test\AbstractTest { function testRegularUserLogic() { foreach(self::USERS as $actor => $rights) { - if($rights != User\Driver::RIGHTS_NONE) continue; + if($rights != User\Driver::RIGHTS_NONE) { + continue; + } Arsse::$user->auth($actor, ""); foreach(array_keys(self::USERS) as $affected) { // regular users should only be able to act for themselves @@ -112,7 +114,9 @@ class TestAuthorization extends Test\AbstractTest { function testDomainManagerLogic() { foreach(self::USERS as $actor => $actorRights) { - if($actorRights != User\Driver::RIGHTS_DOMAIN_MANAGER) continue; + if($actorRights != User\Driver::RIGHTS_DOMAIN_MANAGER) { + continue; + } $actorDomain = substr($actor,strrpos($actor,"@")+1); Arsse::$user->auth($actor, ""); foreach(self::USERS as $affected => $affectedRights) { @@ -151,7 +155,9 @@ class TestAuthorization extends Test\AbstractTest { function testDomainAdministratorLogic() { foreach(self::USERS as $actor => $actorRights) { - if($actorRights != User\Driver::RIGHTS_DOMAIN_ADMIN) continue; + if($actorRights != User\Driver::RIGHTS_DOMAIN_ADMIN) { + continue; + } $actorDomain = substr($actor,strrpos($actor,"@")+1); Arsse::$user->auth($actor, ""); $allowed = [User\Driver::RIGHTS_NONE,User\Driver::RIGHTS_DOMAIN_MANAGER,User\Driver::RIGHTS_DOMAIN_ADMIN]; @@ -191,7 +197,9 @@ class TestAuthorization extends Test\AbstractTest { function testGlobalManagerLogic() { foreach(self::USERS as $actor => $actorRights) { - if($actorRights != User\Driver::RIGHTS_GLOBAL_MANAGER) continue; + if($actorRights != User\Driver::RIGHTS_GLOBAL_MANAGER) { + continue; + } $actorDomain = substr($actor,strrpos($actor,"@")+1); Arsse::$user->auth($actor, ""); foreach(self::USERS as $affected => $affectedRights) { @@ -222,7 +230,9 @@ class TestAuthorization extends Test\AbstractTest { function testGlobalAdministratorLogic() { foreach(self::USERS as $actor => $actorRights) { - if($actorRights != User\Driver::RIGHTS_GLOBAL_ADMIN) continue; + if($actorRights != User\Driver::RIGHTS_GLOBAL_ADMIN) { + continue; + } Arsse::$user->auth($actor, ""); // global admins can do anything foreach(self::USERS as $affected => $affectedRights) { @@ -240,7 +250,9 @@ class TestAuthorization extends Test\AbstractTest { function testInvalidLevelLogic() { foreach(self::USERS as $actor => $rights) { - if(in_array($rights, self::LEVELS)) continue; + if(in_array($rights, self::LEVELS)) { + continue; + } Arsse::$user->auth($actor, ""); foreach(array_keys(self::USERS) as $affected) { // users with unknown/invalid rights should be treated just like regular users and only be able to act for themselves @@ -297,7 +309,9 @@ class TestAuthorization extends Test\AbstractTest { $err = []; foreach($tests as $func => $args) { // list method does not take an affected user, so do not unshift for that one - if($func != "list") array_unshift($args, $user); + if($func != "list") { + array_unshift($args, $user); + } try { call_user_func_array(array(Arsse::$user, $func), $args); } catch(User\ExceptionAuthz $e) {