diff --git a/lib/REST/AbstractHandler.php b/lib/REST/AbstractHandler.php index 8f119981..e942bc33 100644 --- a/lib/REST/AbstractHandler.php +++ b/lib/REST/AbstractHandler.php @@ -23,7 +23,7 @@ abstract class AbstractHandler implements Handler { return $data; } - protected function validateId($id):bool { + protected function validateId($id): bool { try { $ch1 = strval(intval($id)); $ch2 = strval($id); diff --git a/lib/REST/Exception405.php b/lib/REST/Exception405.php new file mode 100644 index 00000000..8c1f148b --- /dev/null +++ b/lib/REST/Exception405.php @@ -0,0 +1,6 @@ +query); - // match the path - if(preg_match("<^/(items|folders|feeds|cleanup|version|status|user)(?:/([^/]+))?(?:/([^/]+))?(?:/([^/]+))?/?$>", $req->path, $url)) { - // clean up the path - $scope = $url[1]; - unset($url[0]); - unset($url[1]); - $url = array_filter($url); - $url = array_values($url); - // decode any % sequences in the URL - $url = array_map(function($v){return rawurldecode($v);}, $url); - // check to make sure the requested function is implemented - $func = $scope.$req->method; - if(!method_exists($this, $func)) return new Response(501); - // dispatch - try { - Data::$db->dateFormatDefault("unix"); - return $this->$func($url, $data); - } catch(Exception $e) { - // if there was a REST exception return 400 - return new Response(400); - } catch(AbstractException $e) { - // if there was any other Arsse exception return 500 - return new Response(500); - } - } else { - return new Response(404); + // check to make sure the requested function is implemented + try { + $func = $this->chooseCall($req->paths, $req->method); + } catch(Exception501 $e) { + return new Response(501); + } catch(Exception405 $e) { + return new Response(405, "", "", ["Allow: ".$e->getMessage()]); } + if(!method_exists($this, $func)) return new Response(501); + // dispatch + try { + Data::$db->dateFormatDefault("unix"); + return $this->$func($req->paths, $data); + } catch(Exception $e) { + // if there was a REST exception return 400 + return new Response(400); + } catch(AbstractException $e) { + // if there was any other Arsse exception return 500 + return new Response(500); + } + } + + protected function chooseCall(array $url, string $method): string { + $choices = [ + 'items' => [], + 'folders' => [ + '' => ['GET' => "folderList", 'POST' => "folderAdd"], + '#' => ['PUT' => "folderRename", 'DELETE' => "folderRemove"], + '#/read' => ['PUT' => "folderMarkRead"], + ], + 'feeds' => [ + '' => ['GET' => "subscriptionList", 'POST' => "subscriptionAdd"], + '#' => ['DELETE' => "subscriptionRemove"], + '#/move' => ['PUT' => "subscriptionMove"], + '#/rename' => ['PUT' => "subscriptionRename"], + '#/read' => ['PUT' => "subscriptionMarkRead"], + 'all' => ['GET' => "feedListStale"], + 'update' => ['GET' => "feedUpdate"], + ], + 'cleanup' => [], + 'version' => [ + '' => ['GET' => "versionReport"], + ], + 'status' => [], + 'user' => [], + ]; + // 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 "#", for easier comparison + for($a = 0; $a < sizeof($url); $a++) { + if($this->validateId($url[$a])) $url[$a] = "#"; + } + // 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(); + // 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) { + // add the scope to the path to match against and split it + $path = (strlen($path)) ? "$scope/$path" : $scope; + $path = explode("/", $path); + if($path===$url) { + // if the path matches, make sure the method is allowed + if(array_key_exists($method,$funcs)) { + // if it is allowed, return the object method to run + return $funcs[$method]; + } else { + // otherwise return 405 + throw new Exception405(implode(", ", array_keys($funcs))); + } + } + } + // if the path was not found, return 501 + throw new Exception501(); } // list folders - protected function foldersGET(array $url, array $data): Response { - // if URL is more than '/folders' this is an error - if(sizeof($url)==1) return new Response(405, "", "", ['Allow: PUT, DELETE']); - if(sizeof($url) > 1) return new Response(404); + protected function folderList(array $url, array $data): Response { $folders = Data::$db->folderList(Data::$user->id, null, false)->getAll(); return new Response(200, ['folders' => $folders]); } // create a folder - protected function foldersPOST(array $url, array $data): Response { - // if URL is more than '/folders' this is an error - if(sizeof($url)==1) return new Response(405, "", "", ['Allow: PUT, DELETE']); - if(sizeof($url) > 1) return new Response(404); + protected function folderAdd(array $url, array $data): Response { try { $folder = Data::$db->folderAdd(Data::$user->id, $data); } catch(ExceptionInput $e) { @@ -93,15 +137,10 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { } // delete a folder - protected function foldersDELETE(array $url, array $data): Response { - // if URL is more or less than '/folders/$id' this is an error - if(sizeof($url) < 1) return new Response(405, "", "", ['Allow: GET, POST']); - if(sizeof($url) > 1) return new Response(404); - // folder ID must be integer - if(!$this->validateId($url[0])) return new Response(404); + protected function folderRemove(array $url, array $data): Response { // perform the deletion try { - Data::$db->folderRemove(Data::$user->id, (int) $url[0]); + Data::$db->folderRemove(Data::$user->id, (int) $url[1]); } catch(ExceptionInput $e) { // folder does not exist return new Response(404); @@ -110,17 +149,12 @@ 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 foldersPUT(array $url, array $data): Response { - // if URL is more or less than '/folders/$id' this is an error - if(sizeof($url) < 1) return new Response(405, "", "", ['Allow: GET, POST']); - if(sizeof($url) > 1) return new Response(404); - // folder ID must be integer - if(!$this->validateId($url[0])) return new Response(404); + protected function folderRename(array $url, array $data): Response { // there must be some change to be made if(!sizeof($data)) return new Response(422); // perform the edit try { - Data::$db->folderPropertiesSet(Data::$user->id, (int) $url[0], $data); + Data::$db->folderPropertiesSet(Data::$user->id, (int) $url[1], $data); } catch(ExceptionInput $e) { switch($e->getCode()) { // folder does not exist @@ -138,33 +172,10 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { } // return the server version - protected function versionGET(array $url, array $data): Response { - // if URL is more than '/version' this is an error - if(sizeof($url)) return new Response(404); + protected function versionReport(array $url, array $data): Response { return new Response(200, ['version' => \JKingWeb\Arsse\VERSION]); } - // invalid function - protected function versionPOST(array $url, array $data): Response { - // if URL is more than '/version' this is an error - if(sizeof($url)) return new Response(404); - return new Response(405, "", "", ['Allow: GET']); - } - - // invalid function - protected function versionPUT(array $url, array $data): Response { - // if URL is more than '/version' this is an error - if(sizeof($url)) return new Response(404); - return new Response(405, "", "", ['Allow: GET']); - } - - // invalid function - protected function versionDELETE(array $url, array $data): Response { - // if URL is more than '/version' this is an error - if(sizeof($url)) return new Response(404); - return new Response(405, "", "", ['Allow: GET']); - } - protected function feedTranslate(array $feed, bool $overwrite = false): array { // cast values $feed = $this->mapFieldTypes($feed, [ @@ -185,62 +196,51 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { } // return list of feeds for the logged-in user + protected function subscriptionList(array $url, array $data): Response { + $subs = Data::$db->subscriptionList(Data::$user->id); + $out = []; + foreach($subs as $sub) { + $sub = $this->feedTranslate($sub); + $out[] = $sub; + } + $out = ['feeds' => $out]; + $out['starredCount'] = Data::$db->articleStarredCount(Data::$user->id); + $newest = Data::$db->editionLatest(Data::$user->id, ['subscription' => $id]); + if($newest) $out['newestItemId'] = $newest; + return new Response(200, $out); + } + // return list of feeds which should be refreshed // refresh a feed - protected function feedsGET(array $url, array $data): Response { - // URL may be /feeds/[all|update] only - $args = sizeof($url); - if($args==2 && in_array($url[1], ["rename","move","read"])) return new Response(405, "", "", ['Allow: PUT, DELETE']); - if($args > 1) return new Response(404); - if($args==1 && !in_array($url[0], ["all","update"])) return new Response(405, "", "", ['Allow: PUT, DELETE']); - // valid action are listing owned subscriptions or (for admins) listing stale feeds or updating a feed - if($args==1) { - // listing stale feeds for updating and updating itself require admin rights per spec - if(Data::$user->rightsGet(Data::$user->id)==User::RIGHTS_NONE) return new Response(403); - if($url[0]=="all") { - // list stale feeds which should be checked for updates - $feeds = Data::$db->feedListStale(); - $out = []; - foreach($feeds as $feed) { - // since in our implementation feeds don't belong the users, the 'userId' field will always be an empty string - $out[] = ['id' => $feed, 'userId' => ""]; - } - return new Response(200, ['feeds' => $out]); - } elseif($url[0]=="update") { - // perform an update of a single feed - if(!array_key_exists("feedId", $data) || $data['feedId'] < 1) return new Response(422); - try { - Data::$db->feedUpdate($data['feedId']); - } catch(ExceptionInput $e) { - return new Response(404); - } - return new Response(200); - } - } else { - // list subscriptions for the logged-in user - $subs = Data::$db->subscriptionList(Data::$user->id); - $out = []; - foreach($subs as $sub) { - $sub = $this->feedTranslate($sub); - $out[] = $sub; - } - $out = ['feeds' => $out]; - $out['starredCount'] = Data::$db->articleStarredCount(Data::$user->id); - $newest = Data::$db->editionLatest(Data::$user->id, ['subscription' => $id]); - if($newest) $out['newestItemId'] = $newest; - return new Response(200, $out); + protected function feedListStale(array $url, array $data): Response { + // function requires admin rights per spec + if(Data::$user->rightsGet(Data::$user->id)==User::RIGHTS_NONE) return new Response(403); + // list stale feeds which should be checked for updates + $feeds = Data::$db->feedListStale(); + $out = []; + foreach($feeds as $feed) { + // since in our implementation feeds don't belong the users, the 'userId' field will always be an empty string + $out[] = ['id' => $feed, 'userId' => ""]; } + return new Response(200, ['feeds' => $out]); + } + + // refresh a feed + protected function feedUpdate(array $url, array $data): Response { + // perform an update of a single feed + if(!array_key_exists("feedId", $data)) return new Response(422); + if(!$this->validateId($data['feedId'])) return new Response(404); + try { + Data::$db->feedUpdate((int) $data['feedId']); + } catch(ExceptionInput $e) { + return new Response(404); + } + return new Response(200); } // add a new feed - protected function feedsPOST(array $url, array $data): Response { - // if URL is more than '/feeds' this is an error - $args = sizeof($url); - if($args==1 && in_array($url[0], ["all","update"])) return new Response(405, "", "", ['Allow: GET']); - if($args==1) return new Response(405, "", "", ['Allow: PUT, DELETE']); - if($args==2 && in_array($url[1], ["rename","move","read"])) return new Response(405, "", "", ['Allow: PUT']); - if($args) return new Response(404); - // normalize the URL + protected function subscriptionAdd(array $url, array $data): Response { + // normalize the feed URL if(!array_key_exists("url", $data)) { $url = ""; } else { @@ -251,7 +251,7 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { $folder = null; } else { $folder = $data['folderId']; - $folder = $folder ? null : $folder; + $folder = $folder ? $folder : null; } // try to add the feed $tr = Data::$db->begin(); @@ -281,15 +281,9 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { } // delete a feed - protected function feedsDELETE(array $url, array $data): Response { - // if URL is more or less than '/feeds/$id' this is an error - if(sizeof($url) < 1) return new Response(405, "", "", ['Allow: GET, POST']); - if(sizeof($url) > 1) return new Response(404); - // folder ID must be integer - if(!$this->validateId($url[0])) return new Response(404); - // perform the deletion + protected function subscriptionRemove(array $url, array $data): Response { try { - Data::$db->subscriptionRemove(Data::$user->id, (int) $url[0]); + Data::$db->subscriptionRemove(Data::$user->id, (int) $url[1]); } catch(ExceptionInput $e) { // feed does not exist return new Response(404); @@ -298,44 +292,40 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { } // rename a feed - // move a feed to a folder - // mark items from a feed as read - protected function feedsPUT(array $url, array $data): Response { - // URL may be /feeds//[rename|move|read] - $args = sizeof($url); - if(!$args) return new Response(405, "", "", ['Allow: GET, POST']); - if($args > 2) return new Response(404); - if(in_array($url[0], ["all", "update"])) { - if($args==1) return new Response(405, "", "", ['Allow: GET']); - return new Response(404); - } - if($args==2 && !in_array($url[1], ["rename","move","read"])) return new Response(404); - // if the feed ID is not an integer, this is also an error - if(!$this->validateId($url[0])) return new Response(404); - // normalize input for move and rename + protected function subscriptionRename(array $url, array $data): Response { + // normalize input $in = []; if(array_key_exists("feedTitle", $data)) { $in['title'] = $data['feedTitle']; + } else { + return new Response(422); } + // perform the renaming + try { + Data::$db->subscriptionPropertiesSet(Data::$user->id, (int) $url[1], $in); + } catch(ExceptionInput $e) { + return new Response(404); + } + return new Response(204); + } + + // move a feed to a folder + protected function subscriptionMove(array $url, array $data): Response { + // normalize input for move and rename + $in = []; if(array_key_exists("folderId", $data)) { $folder = $data['folderId']; if(!$this->validateId($folder)) return new Response(422); if(!$folder) $folder = null; $in['folder'] = $folder; + } else { + return new Response(422); } - // perform the move and/or rename - if($in) { - try { - Data::$db->subscriptionPropertiesSet(Data::$user->id, (int) $url[0], $in); - } catch(ExceptionInput $e) { - return new Response(404); - } - } - // mark items as read, if requested - if(array_key_exists("newestItemId", $data)) { - $newest = $data['newestItemId']; - if(!$this->validateId($newest)) return new Response(422); - // FIXME: do the marking as read + // perform the move + try { + Data::$db->subscriptionPropertiesSet(Data::$user->id, (int) $url[1], $in); + } catch(ExceptionInput $e) { + return new Response(404); } return new Response(204); } diff --git a/lib/REST/NextCloudNews/Versions.php b/lib/REST/NextCloudNews/Versions.php index 5fcbec32..a59ea940 100644 --- a/lib/REST/NextCloudNews/Versions.php +++ b/lib/REST/NextCloudNews/Versions.php @@ -22,7 +22,7 @@ class Versions extends \JKingWeb\Arsse\REST\AbstractHandler { return new Response(200, $out); } else { // if the URL path was anything else, the client is probably trying a version we don't support - return new Response(404); + return new Response(501); } } } \ No newline at end of file diff --git a/lib/REST/Request.php b/lib/REST/Request.php index 0dada443..b70fa774 100644 --- a/lib/REST/Request.php +++ b/lib/REST/Request.php @@ -6,6 +6,7 @@ class Request { public $method = "GET"; public $url = ""; public $path =""; + public $paths = []; public $query = ""; public $type =""; public $body = ""; @@ -31,13 +32,14 @@ class Request { public function refreshURL() { $url = $this->parseURL($this->url); $this->path = $url['path']; + $this->paths = $url['paths']; $this->query = $url['query']; } protected function parseURL(string $url): array { // split the query string from the path $parts = explode("?", $url); - $out = ['path' => $parts[0], 'query' => []]; + $out = ['path' => $parts[0], 'paths' => [''], 'query' => []]; // if there is a query string, parse it if(isset($parts[1])) { // split along & to get key-value pairs @@ -56,6 +58,17 @@ class Request { $out['query'][$key] = $value; } } + // also include the path as a set of decoded elements + // if the path is an empty string or just / nothing needs be done + if(!in_array($out['path'],["/",""])) { + $paths = explode("/", $out['path']); + // remove the first and last empty elements, if present (others should remain) + 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; + } return $out; } } \ No newline at end of file diff --git a/tests/REST/NextCloudNews/TestNCNV1_2.php b/tests/REST/NextCloudNews/TestNCNV1_2.php index b0f3b9b8..c88c2730 100644 --- a/tests/REST/NextCloudNews/TestNCNV1_2.php +++ b/tests/REST/NextCloudNews/TestNCNV1_2.php @@ -29,7 +29,7 @@ class TestNCNV1_2 extends \PHPUnit\Framework\TestCase { function testRespondToInvalidPaths() { $errs = [ - 404 => [ + 501 => [ ['GET', "/"], ['PUT', "/"], ['POST', "/"], @@ -59,10 +59,10 @@ class TestNCNV1_2 extends \PHPUnit\Framework\TestCase { ], ], ]; - foreach($errs[404] as $req) { - $exp = new Response(404); + foreach($errs[501] as $req) { + $exp = new Response(501); list($method, $path) = $req; - $this->assertEquals($exp, $this->h->dispatch(new Request($method, $path)), "$method call to $path did not return 404."); + $this->assertEquals($exp, $this->h->dispatch(new Request($method, $path)), "$method call to $path did not return 501."); } foreach($errs[405] as $allow => $cases) { $exp = new Response(405, "", "", ['Allow: '.$allow]); @@ -130,9 +130,6 @@ class TestNCNV1_2 extends \PHPUnit\Framework\TestCase { $exp = new Response(404); $this->assertEquals($exp, $this->h->dispatch(new Request("DELETE", "/folders/1"))); Phake::verify(Data::$db, Phake::times(2))->folderRemove(Data::$user->id, 1); - // use a non-integer folder ID - $exp = new Response(404); - $this->assertEquals($exp, $this->h->dispatch(new Request("DELETE", "/folders/invalid"))); } function testRenameAFolder() { diff --git a/tests/REST/NextCloudNews/TestNCNVersionDiscovery.php b/tests/REST/NextCloudNews/TestNCNVersionDiscovery.php index 3a049b76..bad565a2 100644 --- a/tests/REST/NextCloudNews/TestNCNVersionDiscovery.php +++ b/tests/REST/NextCloudNews/TestNCNVersionDiscovery.php @@ -35,7 +35,7 @@ class TestNCNVersionDiscovery extends \PHPUnit\Framework\TestCase { } function testUseIncorrectPath() { - $exp = new Response(404); + $exp = new Response(501); $h = new Rest\NextCloudNews\Versions(); $req = new Request("GET", "/ook"); $res = $h->dispatch($req);