diff --git a/lib/REST.php b/lib/REST.php index 1ad740d..1e0487f 100644 --- a/lib/REST.php +++ b/lib/REST.php @@ -7,10 +7,10 @@ declare(strict_types=1); namespace JKingWeb\Arsse; use JKingWeb\Arsse\Arsse; +use JKingWeb\Arsse\Misc\URL; use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Message\ResponseInterface; -use Zend\Diactoros\ServerRequest; use Zend\Diactoros\ServerRequestFactory; use Zend\Diactoros\Response\EmptyResponse; @@ -103,7 +103,7 @@ class REST { return (strlen($a['match']) <=> strlen($b['match'])) * -1; }); // normalize the target URL - $url = REST\Target::normalize($url); + $url = URL::normalize($url); // find a match foreach ($map as $id => $api) { // first try a simple substring match diff --git a/lib/REST/NextCloudNews/V1_2.php b/lib/REST/NextCloudNews/V1_2.php index 0df5032..f64d9cb 100644 --- a/lib/REST/NextCloudNews/V1_2.php +++ b/lib/REST/NextCloudNews/V1_2.php @@ -7,15 +7,12 @@ declare(strict_types=1); namespace JKingWeb\Arsse\REST\NextCloudNews; use JKingWeb\Arsse\Arsse; -use JKingWeb\Arsse\Database; -use JKingWeb\Arsse\User; use JKingWeb\Arsse\Service; use JKingWeb\Arsse\Context\Context; use JKingWeb\Arsse\Misc\ValueInfo; use JKingWeb\Arsse\AbstractException; use JKingWeb\Arsse\Db\ExceptionInput; use JKingWeb\Arsse\Feed\Exception as FeedException; -use JKingWeb\Arsse\REST\Target; use JKingWeb\Arsse\REST\Exception404; use JKingWeb\Arsse\REST\Exception405; use Psr\Http\Message\ServerRequestInterface; @@ -85,11 +82,11 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { } else { return new EmptyResponse(401); } - // explode and normalize the URL path - $target = new Target($req->getRequestTarget()); + // get the request path only; this is assumed to already be normalized + $target = parse_url($req->getRequestTarget())['path'] ?? ""; // handle HTTP OPTIONS requests if ($req->getMethod() === "OPTIONS") { - return $this->handleHTTPOptions((string) $target); + return $this->handleHTTPOptions($target); } // normalize the input $data = (string) $req->getBody(); @@ -115,7 +112,7 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { $data = $this->normalizeInput(array_merge($req->getQueryParams(), $data), $this->validInput, "unix"); // check to make sure the requested function is implemented try { - $func = $this->chooseCall((string) $target, $req->getMethod()); + $func = $this->chooseCall($target, $req->getMethod()); } catch (Exception404 $e) { return new EmptyResponse(404); } catch (Exception405 $e) { @@ -126,7 +123,8 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { } // dispatch try { - return $this->$func($target->path, $data); + $path = explode("/", ltrim($target, "/")); + return $this->$func($path, $data); // @codeCoverageIgnoreStart } catch (Exception $e) { // if there was a REST exception return 400 @@ -139,18 +137,14 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { } protected function normalizePathIds(string $url): string { - // first parse the URL and perform syntactic normalization - $target = new Target($url); + $path = explode("/", $url); // any path 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($target->path); $a++) { - if (ValueInfo::id($target->path[$a])) { - $target->path[$a] = "1"; + for ($a = 0; $a < sizeof($path); $a++) { + if (ValueInfo::id($path[$a])) { + $path[$a] = "1"; } } - // discard any fragment ID (there shouldn't be any) and query string (the query is available in the request itself) - $target->fragment = ""; - $target->query = ""; - return (string) $target; + return implode("/", $path); } protected function chooseCall(string $url, string $method): string { diff --git a/lib/REST/Target.php b/lib/REST/Target.php deleted file mode 100644 index b3fb94d..0000000 --- a/lib/REST/Target.php +++ /dev/null @@ -1,131 +0,0 @@ -parseFragment($target); - $target = $this->parseQuery($target); - $this->path = $this->parsePath($target); - } - - public function __toString(): string { - $out = ""; - $path = []; - foreach ($this->path as $segment) { - if (is_null($segment)) { - if (!$path) { - $path[] = ".."; - } else { - continue; - } - } elseif ($segment === ".") { - $path[] = "%2E"; - } elseif ($segment === "..") { - $path[] = "%2E%2E"; - } else { - $path[] = rawurlencode(ValueInfo::normalize($segment, ValueInfo::T_STRING)); - } - } - $path = implode("/", $path); - if (!$this->relative) { - $out .= "/"; - } - $out .= $path; - if ($this->index && strlen($path)) { - $out .= "/"; - } - if (strlen($this->query)) { - $out .= "?".$this->query; - } - if (strlen($this->fragment)) { - $out .= "#".rawurlencode($this->fragment); - } - return $out; - } - - public static function normalize(string $target): string { - return (string) new self($target); - } - - protected function parseFragment(string $target): string { - // store and strip off any fragment identifier and return the target without a fragment - $pos = strpos($target, "#"); - if ($pos !== false) { - $this->fragment = rawurldecode(substr($target, $pos + 1)); - $target = substr($target, 0, $pos); - } - return $target; - } - - protected function parseQuery(string $target): string { - // store and strip off any query string and return the target without a query - // note that the function assumes any fragment identifier has already been stripped off - // unlike the other parts the query string is currently neither parsed nor normalized - $pos = strpos($target, "?"); - if ($pos !== false) { - $this->query = substr($target, $pos + 1); - $target = substr($target, 0, $pos); - } - return $target; - } - - protected function parsePath(string $target): array { - // note that the function assumes any fragment identifier or query has already been stripped off - // syntax-based normalization is applied to the path segments (see RFC 3986 sec. 6.2.2) - // duplicate slashes are NOT collapsed - if (substr($target, 0, 1) === "/") { - // if the path starts with a slash, strip it off - $target = substr($target, 1); - } else { - // otherwise this is a relative target - $this->relative = true; - } - if (!strlen($target)) { - // if the target is an empty string, this is an index target - $this->index = true; - } elseif (substr($target, -1, 1) === "/") { - // if the path ends in a slash, this is an index target and the slash should be stripped off - $this->index = true; - $target = substr($target, 0, strlen($target) -1); - } - // after stripping, explode the path parts - if (strlen($target)) { - $target = explode("/", $target); - $out = []; - // resolve relative path segments and decode each retained segment - foreach ($target as $index => $segment) { - if ($segment === ".") { - // self-referential segments can be ignored - continue; - } elseif ($segment === "..") { - if ($index == 0) { - // if the first path segment refers to its parent (which we don't know about) we cannot output a correct path, so we do the best we can - $out[] = null; - } else { - // for any other segments after the first we pop off the last stored segment - array_pop($out); - } - } else { - // any other segment is decoded and retained - $out[] = rawurldecode($segment); - } - } - return $out; - } else { - return []; - } - } -} diff --git a/tests/cases/REST/TestTarget.php b/tests/cases/REST/TestTarget.php deleted file mode 100644 index 6b5a383..0000000 --- a/tests/cases/REST/TestTarget.php +++ /dev/null @@ -1,66 +0,0 @@ -assertEquals($path, $test->path, "Path does not match"); - $this->assertSame($path, $test->path, "Path does not match exactly"); - $this->assertSame($relative, $test->relative, "Relative flag does not match"); - $this->assertSame($index, $test->index, "Index flag does not match"); - $this->assertSame($query, $test->query, "Query does not match"); - $this->assertSame($fragment, $test->fragment, "Fragment does not match"); - } - - /** @dataProvider provideTargetUrls */ - public function testNormalizeTargetUrl(string $target, array $path, bool $relative, bool $index, string $query, string $fragment, string $normalized) { - $test = new Target(""); - $test->path = $path; - $test->relative = $relative; - $test->index = $index; - $test->query = $query; - $test->fragment = $fragment; - $this->assertSame($normalized, (string) $test); - $this->assertSame($normalized, Target::normalize($target)); - } - - public function provideTargetUrls() { - return [ - ["/", [], false, true, "", "", "/"], - ["", [], true, true, "", "", ""], - ["/index.php", ["index.php"], false, false, "", "", "/index.php"], - ["index.php", ["index.php"], true, false, "", "", "index.php"], - ["/ook/", ["ook"], false, true, "", "", "/ook/"], - ["ook/", ["ook"], true, true, "", "", "ook/"], - ["/eek/../ook/", ["ook"], false, true, "", "", "/ook/"], - ["eek/../ook/", ["ook"], true, true, "", "", "ook/"], - ["/./ook/", ["ook"], false, true, "", "", "/ook/"], - ["./ook/", ["ook"], true, true, "", "", "ook/"], - ["/../ook/", [null,"ook"], false, true, "", "", "/../ook/"], - ["../ook/", [null,"ook"], true, true, "", "", "../ook/"], - ["0", ["0"], true, false, "", "", "0"], - ["%6f%6F%6b", ["ook"], true, false, "", "", "ook"], - ["%2e%2E%2f%2E%2Fook%2f", [".././ook/"], true, false, "", "", "..%2F.%2Fook%2F"], - ["%2e%2E/%2E/ook%2f", ["..",".","ook/"], true, false, "", "", "%2E%2E/%2E/ook%2F"], - ["...", ["..."], true, false, "", "", "..."], - ["%2e%2e%2e", ["..."], true, false, "", "", "..."], - ["/?", [], false, true, "", "", "/"], - ["/#", [], false, true, "", "", "/"], - ["/?#", [], false, true, "", "", "/"], - ["#%2e", [], true, true, "", ".", "#."], - ["?%2e", [], true, true, "%2e", "", "?%2e"], - ["?%2e#%2f", [], true, true, "%2e", "/", "?%2e#%2F"], - ["#%2e?%2f", [], true, true, "", ".?/", "#.%3F%2F"], - ]; - } -} diff --git a/tests/phpunit.dist.xml b/tests/phpunit.dist.xml index 0086602..cae3734 100644 --- a/tests/phpunit.dist.xml +++ b/tests/phpunit.dist.xml @@ -106,7 +106,6 @@ cases/Db/MySQLPDO/TestDatabase.php - cases/REST/TestTarget.php cases/REST/TestREST.php