From 5488b994f7b3bb424383fb81b3d19a4e74c35e17 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Thu, 28 Sep 2017 10:16:24 -0400 Subject: [PATCH] Merged master; CS fixes --- lib/Conf.php | 2 +- lib/Database.php | 109 ++++++----- lib/Db/SQLite3/Driver.php | 2 +- lib/Misc/Context.php | 2 +- lib/Misc/ValueInfo.php | 63 ++++--- lib/REST/AbstractHandler.php | 37 ++-- lib/REST/NextCloudNews/V1_2.php | 55 +++--- lib/REST/TinyTinyRSS/API.php | 8 +- lib/REST/TinyTinyRSS/Exception.php | 2 +- tests/Misc/TestContext.php | 4 +- tests/Misc/TestValueInfo.php | 204 +++++++++++++++++++++ tests/REST/NextCloudNews/TestNCNV1_2.php | 97 +++++++--- tests/REST/TinyTinyRSS/TestTinyTinyAPI.php | 6 +- tests/lib/AbstractTest.php | 4 +- tests/lib/Database/SeriesCleanup.php | 1 - tests/lib/Database/SeriesFeed.php | 5 + tests/lib/Database/SeriesFolder.php | 21 ++- tests/lib/Database/SeriesSession.php | 1 - tests/lib/Database/SeriesSubscription.php | 17 +- tests/lib/Misc/StrClass.php | 15 ++ tests/phpunit.xml | 7 +- 21 files changed, 507 insertions(+), 155 deletions(-) create mode 100644 tests/Misc/TestValueInfo.php create mode 100644 tests/lib/Misc/StrClass.php diff --git a/lib/Conf.php b/lib/Conf.php index 7ed80d6..c292a77 100644 --- a/lib/Conf.php +++ b/lib/Conf.php @@ -31,7 +31,7 @@ class Conf { /** @var string Period of inactivity after which log-in sessions should be considered invalid, as an ISO 8601 duration (default: 1 hour) * @see https://en.wikipedia.org/wiki/ISO_8601#Durations */ public $userSessionTimeout = "PT1H"; - /** @var string Maximum lifetime of log-in sessions regardless of activity, as an ISO 8601 duration (default: 24 hours); + /** @var string Maximum lifetime of log-in sessions regardless of activity, as an ISO 8601 duration (default: 24 hours); * @see https://en.wikipedia.org/wiki/ISO_8601#Durations */ public $userSessionLifetime = "PT24H"; diff --git a/lib/Database.php b/lib/Database.php index d54d986..bf6ce12 100644 --- a/lib/Database.php +++ b/lib/Database.php @@ -285,23 +285,19 @@ class Database { // normalize folder's parent, if there is one $parent = array_key_exists("parent", $data) ? $this->folderValidateId($user, $data['parent'])['id'] : null; // validate the folder name and parent (if specified); this also checks for duplicates - $name = array_key_exists("name", $data) ? $data['name'] : ""; + $name = array_key_exists("name", $data) ? $data['name'] : ""; $this->folderValidateName($name, true, $parent); // actually perform the insert return $this->db->prepare("INSERT INTO arsse_folders(owner,parent,name) values(?,?,?)", "str", "int", "str")->run($user, $parent, $name)->lastId(); } - public function folderList(string $user, int $parent = null, bool $recursive = true): Db\Result { + public function folderList(string $user, $parent = null, bool $recursive = true): Db\Result { // if the user isn't authorized to perform this action then throw an exception. if (!Arsse::$user->authorize($user, __FUNCTION__)) { throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); } // check to make sure the parent exists, if one is specified - if (!is_null($parent)) { - if (!$this->db->prepare("SELECT count(*) from arsse_folders where owner is ? and id is ?", "str", "int")->run($user, $parent)->getValue()) { - throw new Db\ExceptionInput("idMissing", ["action" => __FUNCTION__, "field" => "parent", 'id' => $parent]); - } - } + $parent = $this->folderValidateId($user, $parent)['id']; // if we're not returning a recursive list we can use a simpler query if (!$recursive) { return $this->db->prepare("SELECT id,name,parent from arsse_folders where owner is ? and parent is ?", "str", "int")->run($user, $parent); @@ -313,10 +309,13 @@ class Database { } } - public function folderRemove(string $user, int $id): bool { + public function folderRemove(string $user, $id): bool { if (!Arsse::$user->authorize($user, __FUNCTION__)) { throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); } + if (!ValueInfo::id($id)) { + throw new Db\ExceptionInput("typeViolation", ["action" => __FUNCTION__, "field" => "folder", 'id' => $id, 'type' => "int > 0"]); + } $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]); @@ -324,10 +323,13 @@ class Database { return true; } - public function folderPropertiesGet(string $user, int $id): array { + public function folderPropertiesGet(string $user, $id): array { if (!Arsse::$user->authorize($user, __FUNCTION__)) { throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); } + if (!ValueInfo::id($id)) { + throw new Db\ExceptionInput("typeViolation", ["action" => __FUNCTION__, "field" => "folder", 'id' => $id, 'type' => "int > 0"]); + } $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]); @@ -335,7 +337,7 @@ class Database { return $props; } - public function folderPropertiesSet(string $user, int $id, array $data): bool { + public function folderPropertiesSet(string $user, $id, array $data): bool { if (!Arsse::$user->authorize($user, __FUNCTION__)) { throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); } @@ -347,14 +349,18 @@ class Database { // if a new name and parent are specified, validate both together $this->folderValidateName($data['name']); $in['name'] = $data['name']; - $in['parent'] = $this->folderValidateMove($user, $id, $data['parent'], $data['name']); + $in['parent'] = $this->folderValidateMove($user, (int) $id, $data['parent'], $data['name']); } elseif ($name) { + // if we're trying to rename the root folder, this simply fails + if (!$id) { + return false; + } // if a new name is specified, validate it $this->folderValidateName($data['name'], true, $in['parent']); $in['name'] = $data['name']; } elseif ($parent) { // if a new parent is specified, validate it - $in['parent'] = $this->folderValidateMove($user, $id, $data['parent']); + $in['parent'] = $this->folderValidateMove($user, (int) $id, $data['parent']); } else { // if neither was specified, do nothing return false; @@ -368,14 +374,13 @@ class Database { } protected function folderValidateId(string $user, $id = null, bool $subject = false): array { - $idInfo = ValueInfo::int($id); - if ($idInfo & (ValueInfo::NULL | ValueInfo::ZERO)) { - // if a null or zero ID is specified this is a no-op - return ['id' => null, 'name' => null, 'parent' => null]; + // if the specified ID is not a non-negative integer (or null), this will always fail + if (!ValueInfo::id($id, true)) { + throw new Db\ExceptionInput("typeViolation", ["action" => $this->caller(), "field" => "folder", 'type' => "int >= 0"]); } - // if a negative integer or non-integer is specified this will always fail - if (!($idInfo & ValueInfo::VALID) || (($idInfo & ValueInfo::NEG))) { - throw new Db\ExceptionInput($subject ? "subjectMissing" : "idMissing", ["action" => $this->caller(), "field" => "folder", 'id' => $id]); + // if a null or zero ID is specified this is a no-op + if (!$id) { + return ['id' => null, 'name' => null, 'parent' => null]; } // check whether the folder exists and is owned by the user $f = $this->db->prepare("SELECT id,name,parent from arsse_folders where owner is ? and id is ?", "str", "int")->run($user, $id)->getRow(); @@ -406,7 +411,9 @@ class Database { if ($id==$parent) { throw new Db\ExceptionInput("circularDependence", $errData); } - // make sure both that the prospective parent exists, and that the it is not one of its children (a circular dependence) + // make sure both that the prospective parent exists, and that the it is not one of its children (a circular dependence); + // also make sure that a folder with the same prospective name and parent does not already exist: if the parent is null, + // SQL will happily accept duplicates (null is not unique), so we must do this check ourselves $p = $this->db->prepare( "WITH RECURSIVE target as (select ? as user, ? as source, ? as dest, ? as rename), @@ -416,7 +423,7 @@ class Database { ((select dest from target) is null or exists(select id from arsse_folders join target on owner is user and id is dest)) as extant, not exists(select id from folders where id is (select dest from target)) as valid, not exists(select id from arsse_folders join target on parent is dest and name is coalesce((select rename from target),(select name from arsse_folders join target on id is source))) as available - ", "str", "int", "int","str" + ", "str", "int", "int", "str" )->run($user, $id, $parent, $name)->getRow(); if (!$p['extant']) { // if the parent doesn't exist or doesn't below to the user, throw an exception @@ -425,6 +432,7 @@ class Database { // if using the desired parent would create a circular dependence, throw a different exception throw new Db\ExceptionInput("circularDependence", $errData); } elseif (!$p['available']) { + // if a folder with the same parent and name already exists, throw another different exception throw new Db\ExceptionInput("constraintViolation", ["action" => $this->caller(), "field" => (is_null($name) ? "parent" : "name")]); } return $parent; @@ -438,7 +446,10 @@ class Database { throw new Db\ExceptionInput("whitespace", ["action" => $this->caller(), "field" => "name"]); } elseif (!($info & ValueInfo::VALID)) { throw new Db\ExceptionInput("typeViolation", ["action" => $this->caller(), "field" => "name", 'type' => "string"]); - } elseif($checkDuplicates) { + } elseif ($checkDuplicates) { + // make sure that a folder with the same prospective name and parent does not already exist: if the parent is null, + // SQL will happily accept duplicates (null is not unique), so we must do this check ourselves + $parent = $parent ? $parent : null; if ($this->db->prepare("SELECT exists(select id from arsse_folders where parent is ? and name is ?)", "int", "str")->run($parent, $name)->getValue()) { throw new Db\ExceptionInput("constraintViolation", ["action" => $this->caller(), "field" => "name"]); } @@ -470,10 +481,12 @@ class Database { return $this->db->prepare('INSERT INTO arsse_subscriptions(owner,feed) values(?,?)', 'str', 'int')->run($user, $feedID)->lastId(); } - public function subscriptionList(string $user, int $folder = null, int $id = null): Db\Result { + public function subscriptionList(string $user, $folder = null, int $id = null): Db\Result { if (!Arsse::$user->authorize($user, __FUNCTION__)) { throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); } + // validate inputs + $folder = $this->folderValidateId($user, $folder)['id']; // create a complex query $q = new Query( "SELECT @@ -492,13 +505,11 @@ class Database { $q->setCTE("user(user)", "SELECT ?", "str", $user); // the subject user; this way we only have to pass it to prepare() once // topmost folders belonging to the user $q->setCTE("topmost(f_id,top)", "SELECT id,id from arsse_folders join user on owner is user where parent is null union select id,top from arsse_folders join topmost on parent=f_id"); - if (!is_null($id)) { + if ($id) { // this condition facilitates the implementation of subscriptionPropertiesGet, which would otherwise have to duplicate the complex query; it takes precedence over a specified folder // if an ID is specified, add a suitable WHERE condition and bindings $q->setWhere("arsse_subscriptions.id is ?", "int", $id); } elseif ($folder) { - // if a folder is specified, make sure it exists - $this->folderValidateId($user, $folder); // if it does exist, add a common table expression to list it and its children so that we select from the entire subtree $q->setCTE("folders(folder)", "SELECT ? union select id from arsse_folders join folders on parent is folder", "int", $folder); // add a suitable WHERE condition @@ -507,24 +518,30 @@ class Database { return $this->db->prepare($q->getQuery(), $q->getTypes())->run($q->getValues()); } - public function subscriptionRemove(string $user, int $id): bool { + public function subscriptionRemove(string $user, $id): bool { if (!Arsse::$user->authorize($user, __FUNCTION__)) { throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); } + if (!ValueInfo::id($id)) { + throw new Db\ExceptionInput("typeViolation", ["action" => __FUNCTION__, "field" => "feed", 'id' => $id, 'type' => "int > 0"]); + } $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]); + throw new Db\ExceptionInput("subjectMissing", ["action" => __FUNCTION__, "field" => "feed", 'id' => $id]); } return true; } - public function subscriptionPropertiesGet(string $user, int $id): array { + public function subscriptionPropertiesGet(string $user, $id): array { if (!Arsse::$user->authorize($user, __FUNCTION__)) { throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); } + if (!ValueInfo::id($id)) { + throw new Db\ExceptionInput("typeViolation", ["action" => __FUNCTION__, "field" => "feed", 'id' => $id, 'type' => "int > 0"]); + } // disable authorization checks for the list call Arsse::$user->authorizationEnabled(false); - $sub = $this->subscriptionList($user, null, $id)->getRow(); + $sub = $this->subscriptionList($user, null, (int) $id)->getRow(); Arsse::$user->authorizationEnabled(true); if (!$sub) { throw new Db\ExceptionInput("subjectMissing", ["action" => __FUNCTION__, "field" => "feed", 'id' => $id]); @@ -532,15 +549,13 @@ class Database { return $sub; } - public function subscriptionPropertiesSet(string $user, int $id, array $data): bool { + public function subscriptionPropertiesSet(string $user, $id, array $data): bool { 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 - throw new Db\ExceptionInput("subjectMissing", ["action" => __FUNCTION__, "field" => "feed", 'id' => $id]); - } + // validate the ID + $id = $this->subscriptionValidateId($user, $id, true)['id']; if (array_key_exists("folder", $data)) { // ensure the target folder exists and belong to the user $data['folder'] = $this->folderValidateId($user, $data['folder'])['id']; @@ -570,10 +585,13 @@ class Database { return $out; } - 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(); + protected function subscriptionValidateId(string $user, $id, bool $subject = false): array { + if (!ValueInfo::id($id)) { + throw new Db\ExceptionInput("typeViolation", ["action" => $this->caller(), "field" => "feed", 'id' => $id, 'type' => "int > 0"]); + } + $out = $this->db->prepare("SELECT id,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]); + throw new Db\ExceptionInput($subject ? "subjectMissing" : "idMissing", ["action" => $this->caller(), "field" => "subscription", 'id' => $id]); } return $out; } @@ -583,9 +601,12 @@ class Database { return array_column($feeds, 'id'); } - public function feedUpdate(int $feedID, bool $throwError = false): bool { + public function feedUpdate($feedID, bool $throwError = false): bool { $tr = $this->db->begin(); // check to make sure the feed exists + if (!ValueInfo::id($feedID)) { + throw new Db\ExceptionInput("typeViolation", ["action" => __FUNCTION__, "field" => "feed", 'id' => $feedID, 'type' => "int > 0"]); + } $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]); @@ -596,7 +617,7 @@ class Database { // here. When an exception is thrown it should update the database with the // error instead of failing; if other exceptions are thrown, we should simply roll back try { - $feed = new Feed($feedID, $f['url'], (string) Date::transform($f['modified'], "http", "sql"), $f['etag'], $f['username'], $f['password'], $scrape); + $feed = new Feed((int) $feedID, $f['url'], (string) Date::transform($f['modified'], "http", "sql"), $f['etag'], $f['username'], $f['password'], $scrape); if (!$feed->modified) { // if the feed hasn't changed, just compute the next fetch time and record it $this->db->prepare("UPDATE arsse_feeds SET updated = CURRENT_TIMESTAMP, next_fetch = ? WHERE id is ?", 'datetime', 'int')->run($feed->nextFetch, $feedID); @@ -1004,7 +1025,10 @@ class Database { return true; } - protected function articleValidateId(string $user, int $id): array { + protected function articleValidateId(string $user, $id): array { + if (!ValueInfo::id($id)) { + throw new Db\ExceptionInput("typeViolation", ["action" => $this->caller(), "field" => "article", 'id' => $id, 'type' => "int > 0"]); // @codeCoverageIgnore + } $out = $this->db->prepare( "SELECT arsse_articles.id as article, @@ -1023,6 +1047,9 @@ class Database { } protected function articleValidateEdition(string $user, int $id): array { + if (!ValueInfo::id($id)) { + throw new Db\ExceptionInput("typeViolation", ["action" => $this->caller(), "field" => "edition", 'id' => $id, 'type' => "int > 0"]); // @codeCoverageIgnore + } $out = $this->db->prepare( "SELECT arsse_editions.id as edition, diff --git a/lib/Db/SQLite3/Driver.php b/lib/Db/SQLite3/Driver.php index b47339e..d65ff2e 100644 --- a/lib/Db/SQLite3/Driver.php +++ b/lib/Db/SQLite3/Driver.php @@ -20,7 +20,7 @@ class Driver extends \JKingWeb\Arsse\Db\AbstractDriver { // check to make sure required extension is loaded if (!class_exists("SQLite3")) { throw new Exception("extMissing", self::driverName()); // @codeCoverageIgnore - } + } // if no database file is specified in the configuration, use a suitable default $dbFile = Arsse::$conf->dbSQLite3File ?? \JKingWeb\Arsse\BASE."arsse.db"; $mode = \SQLITE3_OPEN_READWRITE | \SQLITE3_OPEN_CREATE; diff --git a/lib/Misc/Context.php b/lib/Misc/Context.php index 566d928..ca37b3d 100644 --- a/lib/Misc/Context.php +++ b/lib/Misc/Context.php @@ -37,7 +37,7 @@ class Context { protected function cleanArray(array $spec): array { $spec = array_values($spec); for ($a = 0; $a < sizeof($spec); $a++) { - if(ValueInfo::int($spec[$a])===ValueInfo::VALID) { + if (ValueInfo::id($spec[$a])) { $spec[$a] = (int) $spec[$a]; } else { $spec[$a] = 0; diff --git a/lib/Misc/ValueInfo.php b/lib/Misc/ValueInfo.php index f6f85e9..040243d 100644 --- a/lib/Misc/ValueInfo.php +++ b/lib/Misc/ValueInfo.php @@ -13,33 +13,34 @@ class ValueInfo { const EMPTY = 1 << 2; const WHITE = 1 << 3; - static public function int($value): int { + public static function int($value): int { $out = 0; - // check if the input is null if (is_null($value)) { - $out += self::NULL; - } - // normalize the value to an integer or float if possible - if (is_string($value)) { - if (strval(@intval($value))===$value) { + // check if the input is null + return self::NULL; + } elseif (is_string($value) || (is_object($value) && method_exists($value, "__toString"))) { + $value = (string) $value; + // normalize a string an integer or float if possible + if (!strlen($value)) { + // the empty string is equivalent to null when evaluating an integer + return self::NULL; + } elseif (filter_var($value, \FILTER_VALIDATE_FLOAT) !== false && !fmod((float) $value, 1)) { + // an integral float is acceptable $value = (int) $value; - } elseif (strval(@floatval($value))===$value) { - $value = (float) $value; - } - // the empty string is equivalent to null when evaluating an integer - if (!strlen((string) $value)) { - $out += self::NULL; + } else { + return $out; } - } - // if the value is not an integer or integral float, stop - if (!is_int($value) && (!is_float($value) || fmod($value, 1))) { + } elseif (is_float($value) && !fmod($value, 1)) { + // an integral float is acceptable + $value = (int) $value; + } elseif (!is_int($value)) { + // if the value is not an integer or integral float, stop return $out; } // mark validity - $value = (int) $value; $out += self::VALID; // mark zeroness - if(!$value) { + if ($value==0) { $out += self::ZERO; } // mark negativeness @@ -49,14 +50,17 @@ class ValueInfo { return $out; } - static public function str($value): int { + public static function str($value): int { $out = 0; // check if the input is null if (is_null($value)) { $out += self::NULL; } - // if the value is not scalar, it cannot be valid - if (!is_scalar($value)) { + if (is_object($value) && method_exists($value, "__toString")) { + // if the value is an object which has a __toString method, this is acceptable + $value = (string) $value; + } elseif (!is_scalar($value) || is_bool($value) || (is_float($value) && !is_finite($value))) { + // otherwise if the value is not scalar, is a boolean, or is infinity or NaN, it cannot be valid return $out; } // mark validity @@ -70,4 +74,19 @@ class ValueInfo { } return $out; } -} \ No newline at end of file + + public static function id($value, bool $allowNull = false): bool { + $info = self::int($value); + if ($allowNull && ($info & self::NULL)) { // null (and allowed) + return true; + } elseif (!($info & self::VALID)) { // not an integer + return false; + } elseif ($info & self::NEG) { // negative integer + return false; + } elseif (!$allowNull && ($info & self::ZERO)) { // zero (and not allowed) + return false; + } else { // non-negative integer + return true; + } + } +} diff --git a/lib/REST/AbstractHandler.php b/lib/REST/AbstractHandler.php index 281c7c5..8c0988d 100644 --- a/lib/REST/AbstractHandler.php +++ b/lib/REST/AbstractHandler.php @@ -32,10 +32,6 @@ abstract class AbstractHandler implements Handler { return $data; } - protected function validateInt($id): bool { - return (bool) (ValueInfo::int($id) & ValueInfo::VALID); - } - protected function NormalizeInput(array $data, array $types, string $dateFormat = null): array { $out = []; foreach ($data as $key => $value) { @@ -49,34 +45,29 @@ abstract class AbstractHandler implements Handler { } switch ($types[$key]) { case "int": - if ($this->validateInt($value)) { + if (valueInfo::int($value) & ValueInfo::VALID) { $out[$key] = (int) $value; } break; case "string": - $out[$key] = (string) $value; + if (is_bool($value)) { + $out[$key] = var_export($value, true); + } elseif (!is_scalar($value)) { + break; + } else { + $out[$key] = (string) $value; + } break; case "bool": - if (is_bool($value)) { - $out[$key] = $value; - } elseif ($this->validateInt($value)) { - $value = (int) $value; - if ($value > -1 && $value < 2) { - $out[$key] = $value; - } - } elseif (is_string($value)) { - $value = trim(strtolower($value)); - if ($value=="false") { - $out[$key] = false; - } - if ($value=="true") { - $out[$key] = true; - } + $test = filter_var($value, \FILTER_VALIDATE_BOOLEAN, \FILTER_NULL_ON_FAILURE); + if (!is_null($test)) { + $out[$key] = $test; } break; case "float": - if (is_numeric($value)) { - $out[$key] = (float) $value; + $test = filter_var($value, \FILTER_VALIDATE_FLOAT); + if ($test !== false) { + $out[$key] = $test; } break; case "datetime": diff --git a/lib/REST/NextCloudNews/V1_2.php b/lib/REST/NextCloudNews/V1_2.php index f17a298..8ece274 100644 --- a/lib/REST/NextCloudNews/V1_2.php +++ b/lib/REST/NextCloudNews/V1_2.php @@ -6,6 +6,7 @@ use JKingWeb\Arsse\Arsse; use JKingWeb\Arsse\User; use JKingWeb\Arsse\Service; use JKingWeb\Arsse\Misc\Context; +use JKingWeb\Arsse\Misc\ValueInfo; use JKingWeb\Arsse\AbstractException; use JKingWeb\Arsse\Db\ExceptionInput; use JKingWeb\Arsse\Feed\Exception as FeedException; @@ -78,7 +79,7 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { // dispatch try { return $this->$func($req->paths, $data); - // @codeCoverageIgnoreStart + // @codeCoverageIgnoreStart } catch (Exception $e) { // if there was a REST exception return 400 return new Response(400); @@ -94,15 +95,15 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { 'items' => [], 'folders' => [ '' => ['GET' => "folderList", 'POST' => "folderAdd"], - '0' => ['PUT' => "folderRename", 'DELETE' => "folderRemove"], - '0/read' => ['PUT' => "folderMarkRead"], + '1' => ['PUT' => "folderRename", 'DELETE' => "folderRemove"], + '1/read' => ['PUT' => "folderMarkRead"], ], 'feeds' => [ '' => ['GET' => "subscriptionList", 'POST' => "subscriptionAdd"], - '0' => ['DELETE' => "subscriptionRemove"], - '0/move' => ['PUT' => "subscriptionMove"], - '0/rename' => ['PUT' => "subscriptionRename"], - '0/read' => ['PUT' => "subscriptionMarkRead"], + '1' => ['DELETE' => "subscriptionRemove"], + '1/move' => ['PUT' => "subscriptionMove"], + '1/rename' => ['PUT' => "subscriptionRename"], + '1/read' => ['PUT' => "subscriptionMarkRead"], 'all' => ['GET' => "feedListStale"], 'update' => ['GET' => "feedUpdate"], ], @@ -110,12 +111,12 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { '' => ['GET' => "articleList"], 'updated' => ['GET' => "articleList"], 'read' => ['PUT' => "articleMarkReadAll"], - '0/read' => ['PUT' => "articleMarkRead"], - '0/unread' => ['PUT' => "articleMarkRead"], + '1/read' => ['PUT' => "articleMarkRead"], + '1/unread' => ['PUT' => "articleMarkRead"], 'read/multiple' => ['PUT' => "articleMarkReadMulti"], 'unread/multiple' => ['PUT' => "articleMarkReadMulti"], - '0/0/star' => ['PUT' => "articleMarkStarred"], - '0/0/unstar' => ['PUT' => "articleMarkStarred"], + '1/1/star' => ['PUT' => "articleMarkStarred"], + '1/1/unstar' => ['PUT' => "articleMarkStarred"], 'star/multiple' => ['PUT' => "articleMarkStarredMulti"], 'unstar/multiple' => ['PUT' => "articleMarkStarredMulti"], ], @@ -135,10 +136,10 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { ]; // the first path element is the overall scope of the request $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) + // any URL components which are database IDs (integers greater than zero) should be replaced with "1", for easier comparison (we don't care about the specific ID) for ($a = 0; $a < sizeof($url); $a++) { - if ($this->validateInt($url[$a])) { - $url[$a] = "0"; + if (ValueInfo::id($url[$a])) { + $url[$a] = "1"; } } // normalize the HTTP method to uppercase @@ -336,7 +337,14 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { try { Arsse::$db->feedUpdate($data['feedId']); } catch (ExceptionInput $e) { - return new Response(404); + switch ($e->getCode()) { + case 10239: // feed does not exist + return new Response(404); + case 10237: // feed ID invalid + return new Response(422); + default: // other errors related to input + return new Response(400); // @codeCoverageIgnore + } } return new Response(204); } @@ -347,8 +355,8 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { 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; + // normalize the folder ID, if specified + $folder = isset($data['folderId']) ? $data['folderId'] : null; // try to add the feed $tr = Arsse::$db->begin(); try { @@ -446,12 +454,13 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { Arsse::$db->subscriptionPropertiesSet(Arsse::$user->id, (int) $url[1], $in); } catch (ExceptionInput $e) { switch ($e->getCode()) { - // subscription does not exist - case 10239: return new Response(404); - // folder does not exist - case 10235: return new Response(422); - // other errors related to input - default: return new Response(400); // @codeCoverageIgnore + case 10239: // subscription does not exist + return new Response(404); + case 10235: // folder does not exist + case 10237: // folder ID is invalid + return new Response(422); + default: // other errors related to input + return new Response(400); // @codeCoverageIgnore } } return new Response(204); diff --git a/lib/REST/TinyTinyRSS/API.php b/lib/REST/TinyTinyRSS/API.php index e34081d..a8e3e8d 100644 --- a/lib/REST/TinyTinyRSS/API.php +++ b/lib/REST/TinyTinyRSS/API.php @@ -58,7 +58,7 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { $method = strtolower($method); $map = get_class_methods($this); $map = array_combine(array_map("strtolower", $map), $map); - if(!array_key_exists($method, $map)) { + if (!array_key_exists($method, $map)) { // if the method really doesn't exist, throw an exception throw new Exception("UNKNWON_METHOD", ['method' => $data['op']]); } @@ -113,7 +113,7 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { if (isset($data['user']) && isset($data['password']) && Arsse::$user->auth($data['user'], $data['password'])) { $id = Arsse::$db->sessionCreate($data['user']); return [ - 'session_id' => $id, + 'session_id' => $id, 'api_level' => self::LEVEL ]; } else { @@ -144,7 +144,7 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { } catch (ExceptionInput $e) { switch ($e->getCode()) { // folder already exists - case 10236: + case 10236: // retrieve the ID of the existing folder; duplicating a category silently returns the existing one $folders = Arsse::$db->folderList(Arsse::$user->id, $in['parent'], false); foreach ($folders as $folder) { @@ -159,4 +159,4 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { } } } -} \ No newline at end of file +} diff --git a/lib/REST/TinyTinyRSS/Exception.php b/lib/REST/TinyTinyRSS/Exception.php index bc4bb8b..8833efa 100644 --- a/lib/REST/TinyTinyRSS/Exception.php +++ b/lib/REST/TinyTinyRSS/Exception.php @@ -14,4 +14,4 @@ class Exception extends \Exception { $err = ['error' => $this->getMessage()]; return array_merge($err, $this->data, $err); } -} \ No newline at end of file +} diff --git a/tests/Misc/TestContext.php b/tests/Misc/TestContext.php index f03ca41..49ab9bd 100644 --- a/tests/Misc/TestContext.php +++ b/tests/Misc/TestContext.php @@ -47,9 +47,9 @@ class TestContext extends Test\AbstractTest { $this->assertInstanceOf(Context::class, $c->$method($v[$method])); $this->assertTrue($c->$method()); if (in_array($method, $times)) { - $this->assertTime($c->$method, $v[$method]); + $this->assertTime($c->$method, $v[$method], "Context method $method did not return the expected results"); } else { - $this->assertSame($c->$method, $v[$method]); + $this->assertSame($c->$method, $v[$method], "Context method $method did not return the expected results"); } } } diff --git a/tests/Misc/TestValueInfo.php b/tests/Misc/TestValueInfo.php new file mode 100644 index 0000000..1a75099 --- /dev/null +++ b/tests/Misc/TestValueInfo.php @@ -0,0 +1,204 @@ +assertSame($exp, I::int($value), "Test returned ".decbin(I::int($value))." for value: ".var_export($value, true)); + } + } + public function testGetStringInfo() { + $tests = [ + [null, I::NULL], + ["", I::VALID | I::EMPTY], + [1, I::VALID], + [PHP_INT_MAX, I::VALID], + [1.0, I::VALID], + ["1.0", I::VALID], + ["001.0", I::VALID], + ["1.0e2", I::VALID], + ["1", I::VALID], + ["001", I::VALID], + ["1e2", I::VALID], + ["+1.0", I::VALID], + ["+001.0", I::VALID], + ["+1.0e2", I::VALID], + ["+1", I::VALID], + ["+001", I::VALID], + ["+1e2", I::VALID], + [0, I::VALID], + ["0", I::VALID], + ["000", I::VALID], + [0.0, I::VALID], + ["0.0", I::VALID], + ["000.000", I::VALID], + ["+0", I::VALID], + ["+000", I::VALID], + ["+0.0", I::VALID], + ["+000.000", I::VALID], + [-1, I::VALID], + [-1.0, I::VALID], + ["-1.0", I::VALID], + ["-001.0", I::VALID], + ["-1.0e2", I::VALID], + ["-1", I::VALID], + ["-001", I::VALID], + ["-1e2", I::VALID], + [-0, I::VALID], + ["-0", I::VALID], + ["-000", I::VALID], + [-0.0, I::VALID], + ["-0.0", I::VALID], + ["-000.000", I::VALID], + [false, 0], + [true, 0], + [INF, 0], + [-INF, 0], + [NAN, 0], + [[], 0], + ["some string", I::VALID], + [" ", I::VALID | I::WHITE], + [new \StdClass, 0], + [new StrClass(""), I::VALID | I::EMPTY], + [new StrClass("1"), I::VALID], + [new StrClass("0"), I::VALID], + [new StrClass("-1"), I::VALID], + [new StrClass("Msg"), I::VALID], + [new StrClass(" "), I::VALID | I::WHITE], + ]; + foreach ($tests as $test) { + list($value, $exp) = $test; + $this->assertSame($exp, I::str($value), "Test returned ".decbin(I::str($value))." for value: ".var_export($value, true)); + } + } + + public function testValidateDatabaseIdentifier() { + $tests = [ + [null, false, true], + ["", false, true], + [1, true, true], + [PHP_INT_MAX, true, true], + [1.0, true, true], + ["1.0", true, true], + ["001.0", true, true], + ["1.0e2", true, true], + ["1", true, true], + ["001", true, true], + ["1e2", true, true], + ["+1.0", true, true], + ["+001.0", true, true], + ["+1.0e2", true, true], + ["+1", true, true], + ["+001", true, true], + ["+1e2", true, true], + [0, false, true], + ["0", false, true], + ["000", false, true], + [0.0, false, true], + ["0.0", false, true], + ["000.000", false, true], + ["+0", false, true], + ["+000", false, true], + ["+0.0", false, true], + ["+000.000", false, true], + [-1, false, false], + [-1.0, false, false], + ["-1.0", false, false], + ["-001.0", false, false], + ["-1.0e2", false, false], + ["-1", false, false], + ["-001", false, false], + ["-1e2", false, false], + [-0, false, true], + ["-0", false, true], + ["-000", false, true], + [-0.0, false, true], + ["-0.0", false, true], + ["-000.000", false, true], + [false, false, false], + [true, false, false], + [INF, false, false], + [-INF, false, false], + [NAN, false, false], + [[], false, false], + ["some string", false, false], + [" ", false, false], + [new \StdClass, false, false], + [new StrClass(""), false, true], + [new StrClass("1"), true, true], + [new StrClass("0"), false, true], + [new StrClass("-1"), false, false], + [new StrClass("Msg"), false, false], + [new StrClass(" "), false, false], + ]; + foreach ($tests as $test) { + list($value, $exp, $expNull) = $test; + $this->assertSame($exp, I::id($value), "Non-null test failed for value: ".var_export($value, true)); + $this->assertSame($expNull, I::id($value, true), "Null test failed for value: ".var_export($value, true)); + } + } +} diff --git a/tests/REST/NextCloudNews/TestNCNV1_2.php b/tests/REST/NextCloudNews/TestNCNV1_2.php index a137df9..713267b 100644 --- a/tests/REST/NextCloudNews/TestNCNV1_2.php +++ b/tests/REST/NextCloudNews/TestNCNV1_2.php @@ -46,6 +46,21 @@ class TestNCNV1_2 extends Test\AbstractTest { 'title' => 'Second example feed', 'unread' => 23, ], + [ + 'id' => 47, + 'url' => 'http://example.net/news.atom', + 'favicon' => 'http://example.net/favicon.png', + 'source' => 'http://example.net/', + 'folder' => null, + 'top_folder' => null, + 'pinned' => 0, + 'err_count' => 0, + 'err_msg' => '', + 'order_type' => 1, + 'added' => '2017-05-20 13:35:54', + 'title' => 'Third example feed', + 'unread' => 0, + ], ], 'rest' => [ [ @@ -76,6 +91,20 @@ class TestNCNV1_2 extends Test\AbstractTest { 'title' => 'Second example feed', 'unreadCount' => 23, ], + [ + 'id' => 47, + 'url' => 'http://example.net/news.atom', + 'faviconLink' => 'http://example.net/favicon.png', + 'link' => 'http://example.net/', + 'folderId' => 0, + 'pinned' => false, + 'updateErrorCount' => 0, + 'lastUpdateError' => '', + 'ordering' => 1, + 'added' => 1495287354, + 'title' => 'Third example feed', + 'unreadCount' => 0, + ], ], ]; protected $articles = [ @@ -331,7 +360,7 @@ class TestNCNV1_2 extends Test\AbstractTest { $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/folders/1", '', 'application/json'))); } - public function testReceiveAuthenticationChallenge() { + public function testSendAuthenticationChallenge() { Phake::when(Arsse::$user)->authHTTP->thenReturn(false); $exp = new Response(401, "", "", ['WWW-Authenticate: Basic realm="'.REST\NextCloudNews\V1_2::REALM.'"']); $this->assertEquals($exp, $this->h->dispatch(new Request("GET", "/"))); @@ -381,6 +410,7 @@ class TestNCNV1_2 extends Test\AbstractTest { $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "/folders"))); $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "/folders", '{"name":""}', 'application/json'))); $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "/folders", '{"name":" "}', 'application/json'))); + $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "/folders", '{"name":{}}', 'application/json'))); // try adding the same two folders again $exp = new Response(409); $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "/folders?name=Software"))); @@ -457,26 +487,29 @@ class TestNCNV1_2 extends Test\AbstractTest { $in = [ ['url' => "http://example.com/news.atom", 'folderId' => 3], ['url' => "http://example.org/news.atom", 'folderId' => 8], - ['url' => "http://example.net/news.atom", 'folderId' => 0], + ['url' => "http://example.net/news.atom", 'folderId' => 8], + ['url' => "http://example.net/news.atom", 'folderId' => -1], [], ]; $out = [ ['feeds' => [$this->feeds['rest'][0]]], ['feeds' => [$this->feeds['rest'][1]], 'newestItemId' => 4758915], - [], - [], + ['feeds' => [$this->feeds['rest'][2]], 'newestItemId' => 2112], ]; // set up the necessary mocks Phake::when(Arsse::$db)->subscriptionAdd(Arsse::$user->id, "http://example.com/news.atom")->thenReturn(2112)->thenThrow(new ExceptionInput("constraintViolation")); // error on the second call Phake::when(Arsse::$db)->subscriptionAdd(Arsse::$user->id, "http://example.org/news.atom")->thenReturn(42)->thenThrow(new ExceptionInput("constraintViolation")); // error on the second call Phake::when(Arsse::$db)->subscriptionPropertiesGet(Arsse::$user->id, 2112)->thenReturn($this->feeds['db'][0]); Phake::when(Arsse::$db)->subscriptionPropertiesGet(Arsse::$user->id, 42)->thenReturn($this->feeds['db'][1]); + Phake::when(Arsse::$db)->subscriptionPropertiesGet(Arsse::$user->id, 47)->thenReturn($this->feeds['db'][2]); Phake::when(Arsse::$db)->editionLatest(Arsse::$user->id, (new Context)->subscription(2112))->thenReturn(0); Phake::when(Arsse::$db)->editionLatest(Arsse::$user->id, (new Context)->subscription(42))->thenReturn(4758915); - Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, 2112, ['folder' => 3])->thenThrow(new ExceptionInput("idMissing")); // folder ID 3 does not exist - Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, 42, ['folder' => 8])->thenReturn(true); - // set up a mock for a bad feed - Phake::when(Arsse::$db)->subscriptionAdd(Arsse::$user->id, "http://example.net/news.atom")->thenThrow(new \JKingWeb\Arsse\Feed\Exception("http://example.net/news.atom", new \PicoFeed\Client\InvalidUrlException())); + Phake::when(Arsse::$db)->editionLatest(Arsse::$user->id, (new Context)->subscription(47))->thenReturn(2112); + Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, 2112, ['folder' => 3])->thenThrow(new ExceptionInput("idMissing")); // folder ID 3 does not exist + Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, 42, ['folder' => 8])->thenReturn(true); + Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, 47, ['folder' => -1])->thenThrow(new ExceptionInput("typeViolation")); // folder ID -1 is invalid + // set up a mock for a bad feed which succeeds the second time + Phake::when(Arsse::$db)->subscriptionAdd(Arsse::$user->id, "http://example.net/news.atom")->thenThrow(new \JKingWeb\Arsse\Feed\Exception("http://example.net/news.atom", new \PicoFeed\Client\InvalidUrlException()))->thenReturn(47); // add the subscriptions $exp = new Response(200, $out[0]); $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "/feeds", json_encode($in[0]), 'application/json'))); @@ -485,14 +518,16 @@ class TestNCNV1_2 extends Test\AbstractTest { // try to add them a second time $exp = new Response(409); $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "/feeds", json_encode($in[0]), 'application/json'))); - $exp = new Response(409); $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "/feeds", json_encode($in[1]), 'application/json'))); // try to add a bad feed $exp = new Response(422); $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "/feeds", json_encode($in[2]), 'application/json'))); + // try again (this will succeed), with an invalid folder ID + $exp = new Response(200, $out[2]); + $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "/feeds", json_encode($in[3]), 'application/json'))); // try to add no feed $exp = new Response(422); - $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "/feeds", json_encode($in[3]), 'application/json'))); + $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "/feeds", json_encode($in[4]), 'application/json'))); } public function testRemoveASubscription() { @@ -511,11 +546,13 @@ class TestNCNV1_2 extends Test\AbstractTest { ['folderId' => 42], ['folderId' => 2112], ['folderId' => 42], + ['folderId' => -1], [], ]; - Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, 1, ['folder' => 42])->thenReturn(true); + Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, 1, ['folder' => 42])->thenReturn(true); Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, 1, ['folder' => null])->thenReturn(true); Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, 1, ['folder' => 2112])->thenThrow(new ExceptionInput("idMissing")); // folder does not exist + Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, 1, ['folder' => -1])->thenThrow(new ExceptionInput("typeViolation")); // folder is invalid Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, 42, $this->anything())->thenThrow(new ExceptionInput("subjectMissing")); // subscription does not exist $exp = new Response(204); $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/feeds/1/move", json_encode($in[0]), 'application/json'))); @@ -527,6 +564,8 @@ class TestNCNV1_2 extends Test\AbstractTest { $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/feeds/42/move", json_encode($in[3]), 'application/json'))); $exp = new Response(422); $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/feeds/1/move", json_encode($in[4]), 'application/json'))); + $exp = new Response(422); + $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/feeds/1/move", json_encode($in[5]), 'application/json'))); } public function testRenameASubscription() { @@ -584,18 +623,20 @@ class TestNCNV1_2 extends Test\AbstractTest { ['feedId' => 42], // valid ['feedId' => 2112], // feed does not exist ['feedId' => "ook"], // invalid ID + ['feedId' => -1], // invalid ID ['feed' => 42], // invalid input ]; Phake::when(Arsse::$db)->feedUpdate(42)->thenReturn(true); Phake::when(Arsse::$db)->feedUpdate(2112)->thenThrow(new ExceptionInput("subjectMissing")); + Phake::when(Arsse::$db)->feedUpdate(-1)->thenThrow(new ExceptionInput("typeViolation")); $exp = new Response(204); $this->assertEquals($exp, $this->h->dispatch(new Request("GET", "/feeds/update", json_encode($in[0]), 'application/json'))); $exp = new Response(404); $this->assertEquals($exp, $this->h->dispatch(new Request("GET", "/feeds/update", json_encode($in[1]), 'application/json'))); $exp = new Response(422); $this->assertEquals($exp, $this->h->dispatch(new Request("GET", "/feeds/update", json_encode($in[2]), 'application/json'))); - $exp = new Response(422); $this->assertEquals($exp, $this->h->dispatch(new Request("GET", "/feeds/update", json_encode($in[3]), 'application/json'))); + $this->assertEquals($exp, $this->h->dispatch(new Request("GET", "/feeds/update", json_encode($in[4]), 'application/json'))); // updating a feed when not an admin fails Phake::when(Arsse::$user)->rightsGet->thenReturn(0); $exp = new Response(403); @@ -606,13 +647,15 @@ class TestNCNV1_2 extends Test\AbstractTest { $res = new Result($this->articles['db']); $t = new \DateTime; $in = [ - ['type' => 0, 'id' => 42], - ['type' => 1, 'id' => 2112], - ['type' => 2, 'id' => 0], - ['type' => 3, 'id' => 0], + ['type' => 0, 'id' => 42], // type=0 => subscription/feed + ['type' => 1, 'id' => 2112], // type=1 => folder + ['type' => 0, 'id' => -1], // type=0 => subscription/feed; invalid ID + ['type' => 1, 'id' => -1], // type=1 => folder; invalid ID + ['type' => 2, 'id' => 0], // type=2 => starred + ['type' => 3, 'id' => 0], // type=3 => all (default); base context ['oldestFirst' => true, 'batchSize' => 10, 'offset' => 5], ['oldestFirst' => false, 'batchSize' => 5, 'offset' => 5], - ['getRead' => true], + ['getRead' => true], // base context ['getRead' => false], ['lastModified' => $t->getTimestamp()], ['oldestFirst' => false, 'batchSize' => 5, 'offset' => 0], // offset=0 should not set the latestEdition context @@ -620,6 +663,8 @@ class TestNCNV1_2 extends Test\AbstractTest { Phake::when(Arsse::$db)->articleList(Arsse::$user->id, $this->anything())->thenReturn($res); Phake::when(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->subscription(42))->thenThrow(new ExceptionInput("idMissing")); Phake::when(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->folder(2112))->thenThrow(new ExceptionInput("idMissing")); + Phake::when(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->subscription(-1))->thenThrow(new ExceptionInput("typeViolation")); + Phake::when(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->folder(-1))->thenThrow(new ExceptionInput("typeViolation")); $exp = new Response(200, ['items' => $this->articles['rest']]); // check the contents of the response $this->assertEquals($exp, $this->h->dispatch(new Request("GET", "/items"))); // first instance of base context @@ -628,22 +673,26 @@ class TestNCNV1_2 extends Test\AbstractTest { $exp = new Response(422); $this->assertEquals($exp, $this->h->dispatch(new Request("GET", "/items", json_encode($in[0]), 'application/json'))); $this->assertEquals($exp, $this->h->dispatch(new Request("GET", "/items", json_encode($in[1]), 'application/json'))); + $this->assertEquals($exp, $this->h->dispatch(new Request("GET", "/items", json_encode($in[2]), 'application/json'))); + $this->assertEquals($exp, $this->h->dispatch(new Request("GET", "/items", json_encode($in[3]), 'application/json'))); // simply run through the remainder of the input for later method verification - $this->h->dispatch(new Request("GET", "/items", json_encode($in[2]), 'application/json')); - $this->h->dispatch(new Request("GET", "/items", json_encode($in[3]), 'application/json')); // third instance of base context $this->h->dispatch(new Request("GET", "/items", json_encode($in[4]), 'application/json')); - $this->h->dispatch(new Request("GET", "/items", json_encode($in[5]), 'application/json')); - $this->h->dispatch(new Request("GET", "/items", json_encode($in[6]), 'application/json')); // fourth instance of base context + $this->h->dispatch(new Request("GET", "/items", json_encode($in[5]), 'application/json')); // third instance of base context + $this->h->dispatch(new Request("GET", "/items", json_encode($in[6]), 'application/json')); $this->h->dispatch(new Request("GET", "/items", json_encode($in[7]), 'application/json')); - $this->h->dispatch(new Request("GET", "/items", json_encode($in[8]), 'application/json')); + $this->h->dispatch(new Request("GET", "/items", json_encode($in[8]), 'application/json')); // fourth instance of base context $this->h->dispatch(new Request("GET", "/items", json_encode($in[9]), 'application/json')); + $this->h->dispatch(new Request("GET", "/items", json_encode($in[10]), 'application/json')); + $this->h->dispatch(new Request("GET", "/items", json_encode($in[11]), 'application/json')); // perform method verifications Phake::verify(Arsse::$db, Phake::times(4))->articleList(Arsse::$user->id, (new Context)->reverse(true)); Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->subscription(42)); Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->folder(2112)); + Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->subscription(-1)); + Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->folder(-1)); Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->starred(true)); - Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(false)->limit(10)->oldestEdition(6)); - Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->limit(5)->latestEdition(4)); + Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(false)->limit(10)->oldestEdition(6)); // offset is one more than specified + Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->limit(5)->latestEdition(4)); // offset is one less than specified Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->unread(true)); Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->modifiedSince($t)); Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->limit(5)); diff --git a/tests/REST/TinyTinyRSS/TestTinyTinyAPI.php b/tests/REST/TinyTinyRSS/TestTinyTinyAPI.php index ebb42b8..a712b13 100644 --- a/tests/REST/TinyTinyRSS/TestTinyTinyAPI.php +++ b/tests/REST/TinyTinyRSS/TestTinyTinyAPI.php @@ -270,7 +270,7 @@ class TestTinyTinyAPI extends Test\AbstractTest { Phake::when(Arsse::$db)->folderAdd(Arsse::$user->id, $db[0])->thenReturn(2)->thenThrow(new ExceptionInput("constraintViolation")); // error on the second call Phake::when(Arsse::$db)->folderAdd(Arsse::$user->id, $db[1])->thenReturn(3)->thenThrow(new ExceptionInput("constraintViolation")); // error on the second call Phake::when(Arsse::$db)->folderList(Arsse::$user->id, null, false)->thenReturn(new Result([$out[0], $out[2]])); - Phake::when(Arsse::$db)->folderList(Arsse::$user->id, 1, false)->thenReturn(new Result([$out[1]])); + Phake::when(Arsse::$db)->folderList(Arsse::$user->id, 1, false)->thenReturn(new Result([$out[1]])); // set up mocks that produce errors Phake::when(Arsse::$db)->folderAdd(Arsse::$user->id, [])->thenThrow(new ExceptionInput("missing")); Phake::when(Arsse::$db)->folderAdd(Arsse::$user->id, ['name' => "", 'parent' => null])->thenThrow(new ExceptionInput("missing")); @@ -286,11 +286,11 @@ class TestTinyTinyAPI extends Test\AbstractTest { $exp = $this->respGood(3); $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "", json_encode($in[1])))); Phake::verify(Arsse::$db)->folderList(Arsse::$user->id, null, false); - Phake::verify(Arsse::$db)->folderList(Arsse::$user->id, 1, false); + Phake::verify(Arsse::$db)->folderList(Arsse::$user->id, 1, false); // add some invalid folders $exp = $this->respErr("INCORRECT_USAGE"); $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "", json_encode($in[2])))); $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "", json_encode($in[3])))); $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "", json_encode($in[4])))); } -} \ No newline at end of file +} diff --git a/tests/lib/AbstractTest.php b/tests/lib/AbstractTest.php index 391ae7a..a310cda 100644 --- a/tests/lib/AbstractTest.php +++ b/tests/lib/AbstractTest.php @@ -25,10 +25,10 @@ abstract class AbstractTest extends \PHPUnit\Framework\TestCase { } } - public function assertTime($exp, $test) { + public function assertTime($exp, $test, string $msg = null) { $exp = Date::transform($exp, "iso8601"); $test = Date::transform($test, "iso8601"); - $this->assertSame($exp, $test); + $this->assertSame($exp, $test, $msg); } public function clearData(bool $loadLang = true): bool { diff --git a/tests/lib/Database/SeriesCleanup.php b/tests/lib/Database/SeriesCleanup.php index 9d60c8b..a8656ff 100644 --- a/tests/lib/Database/SeriesCleanup.php +++ b/tests/lib/Database/SeriesCleanup.php @@ -192,6 +192,5 @@ trait SeriesCleanup { unset($state['arsse_sessions']['rows'][$id - 1]); } $this->compareExpectations($state); - } } diff --git a/tests/lib/Database/SeriesFeed.php b/tests/lib/Database/SeriesFeed.php index d2cd7ed..09312e5 100644 --- a/tests/lib/Database/SeriesFeed.php +++ b/tests/lib/Database/SeriesFeed.php @@ -221,6 +221,11 @@ trait SeriesFeed { Arsse::$db->feedUpdate(2112); } + public function testUpdateAnInvalidFeed() { + $this->assertException("typeViolation", "Db", "ExceptionInput"); + Arsse::$db->feedUpdate(-1); + } + public function testUpdateAFeedThrowingExceptions() { $this->assertException("invalidUrl", "Feed"); Arsse::$db->feedUpdate(3, true); diff --git a/tests/lib/Database/SeriesFolder.php b/tests/lib/Database/SeriesFolder.php index 16312f2..5c0d15b 100644 --- a/tests/lib/Database/SeriesFolder.php +++ b/tests/lib/Database/SeriesFolder.php @@ -77,7 +77,7 @@ trait SeriesFolder { } public function testAddANestedFolderToAnInvalidParent() { - $this->assertException("idMissing", "Db", "ExceptionInput"); + $this->assertException("typeViolation", "Db", "ExceptionInput"); Arsse::$db->folderAdd("john.doe@example.com", ['name' => "Sociology", 'parent' => "stringFolderId"]); } @@ -184,6 +184,11 @@ trait SeriesFolder { Arsse::$db->folderRemove("john.doe@example.com", 2112); } + public function testRemoveAnInvalidFolder() { + $this->assertException("typeViolation", "Db", "ExceptionInput"); + Arsse::$db->folderRemove("john.doe@example.com", -1); + } + public function testRemoveAFolderOfTheWrongOwner() { $this->assertException("subjectMissing", "Db", "ExceptionInput"); Arsse::$db->folderRemove("john.doe@example.com", 4); // folder ID 4 belongs to Jane @@ -210,6 +215,11 @@ trait SeriesFolder { Arsse::$db->folderPropertiesGet("john.doe@example.com", 2112); } + public function testGetThePropertiesOfAnInvalidFolder() { + $this->assertException("typeViolation", "Db", "ExceptionInput"); + Arsse::$db->folderPropertiesGet("john.doe@example.com", -1); + } + public function testGetThePropertiesOfAFolderOfTheWrongOwner() { $this->assertException("subjectMissing", "Db", "ExceptionInput"); Arsse::$db->folderPropertiesGet("john.doe@example.com", 4); // folder ID 4 belongs to Jane @@ -233,6 +243,10 @@ trait SeriesFolder { $this->compareExpectations($state); } + public function testRenameTheRootFolder() { + $this->assertFalse(Arsse::$db->folderPropertiesSet("john.doe@example.com", null, ['name' => "Opinion"])); + } + public function testRenameAFolderToTheEmptyString() { $this->assertException("missing", "Db", "ExceptionInput"); $this->assertTrue(Arsse::$db->folderPropertiesSet("john.doe@example.com", 6, ['name' => ""])); @@ -296,6 +310,11 @@ trait SeriesFolder { Arsse::$db->folderPropertiesSet("john.doe@example.com", 2112, ['parent' => null]); } + public function testSetThePropertiesOfAnInvalidFolder() { + $this->assertException("typeViolation", "Db", "ExceptionInput"); + Arsse::$db->folderPropertiesSet("john.doe@example.com", -1, ['parent' => null]); + } + public function testSetThePropertiesOfAFolderForTheWrongOwner() { $this->assertException("subjectMissing", "Db", "ExceptionInput"); Arsse::$db->folderPropertiesSet("john.doe@example.com", 4, ['parent' => null]); // folder ID 4 belongs to Jane diff --git a/tests/lib/Database/SeriesSession.php b/tests/lib/Database/SeriesSession.php index 95d4b1b..20fada6 100644 --- a/tests/lib/Database/SeriesSession.php +++ b/tests/lib/Database/SeriesSession.php @@ -7,7 +7,6 @@ use JKingWeb\Arsse\Misc\Date; use Phake; trait SeriesSession { - public function setUpSeries() { // set up the test data $past = gmdate("Y-m-d H:i:s", strtotime("now - 1 minute")); diff --git a/tests/lib/Database/SeriesSubscription.php b/tests/lib/Database/SeriesSubscription.php index 12f5760..b68ecb4 100644 --- a/tests/lib/Database/SeriesSubscription.php +++ b/tests/lib/Database/SeriesSubscription.php @@ -193,6 +193,11 @@ trait SeriesSubscription { Arsse::$db->subscriptionRemove($this->user, 2112); } + public function testRemoveAnInvalidSubscription() { + $this->assertException("typeViolation", "Db", "ExceptionInput"); + Arsse::$db->subscriptionRemove($this->user, -1); + } + public function testRemoveASubscriptionForTheWrongOwner() { $this->user = "jane.doe@example.com"; $this->assertException("subjectMissing", "Db", "ExceptionInput"); @@ -264,6 +269,11 @@ trait SeriesSubscription { Arsse::$db->subscriptionPropertiesGet($this->user, 2112); } + public function testGetThePropertiesOfAnInvalidSubscription() { + $this->assertException("typeViolation", "Db", "ExceptionInput"); + Arsse::$db->subscriptionPropertiesGet($this->user, -1); + } + public function testGetThePropertiesOfASubscriptionWithoutAuthority() { Phake::when(Arsse::$user)->authorize->thenReturn(false); $this->assertException("notAuthorized", "User", "ExceptionAuthz"); @@ -311,7 +321,7 @@ trait SeriesSubscription { } public function testRenameASubscriptionToFalse() { - $this->assertException("missing", "Db", "ExceptionInput"); + $this->assertException("typeViolation", "Db", "ExceptionInput"); Arsse::$db->subscriptionPropertiesSet($this->user, 1, ['title' => false]); } @@ -329,6 +339,11 @@ trait SeriesSubscription { Arsse::$db->subscriptionPropertiesSet($this->user, 2112, ['folder' => null]); } + public function testSetThePropertiesOfAnInvalidSubscription() { + $this->assertException("typeViolation", "Db", "ExceptionInput"); + Arsse::$db->subscriptionPropertiesSet($this->user, -1, ['folder' => null]); + } + public function testSetThePropertiesOfASubscriptionWithoutAuthority() { Phake::when(Arsse::$user)->authorize->thenReturn(false); $this->assertException("notAuthorized", "User", "ExceptionAuthz"); diff --git a/tests/lib/Misc/StrClass.php b/tests/lib/Misc/StrClass.php new file mode 100644 index 0000000..905194f --- /dev/null +++ b/tests/lib/Misc/StrClass.php @@ -0,0 +1,15 @@ +str = (string) $str; + } + + public function __toString() { + return $this->str; + } +} diff --git a/tests/phpunit.xml b/tests/phpunit.xml index 98eba09..d1e8ea0 100644 --- a/tests/phpunit.xml +++ b/tests/phpunit.xml @@ -31,6 +31,10 @@ Conf/TestConf.php + + Misc/TestValueInfo.php + Misc/TestContext.php + User/TestUserMockInternal.php User/TestUserMockExternal.php @@ -41,9 +45,6 @@ Feed/TestFeedFetching.php Feed/TestFeed.php - - Misc/TestContext.php - Db/TestTransaction.php Db/SQLite3/TestDbResultSQLite3.php