Make API endpoint validation more strict, improve errors

SETUP/tests/smoketests/pageload_smoketest.py

@@ -71,6 +71,44 @@
     {'path': 'api/index.php?url=v1/stats/site/rounds/P3'},
 ]
 
+API_INVALID_TESTS = [
+    {
+        'method': 'DELETE',
+        'path': 'api/index.php?url=v1/projects',
+        'expect_status': 405
+    },
+    {
+        'path': 'api/index.php?url=v1/projectz/projectID5e23a810ef693/wordcheck',
+        'expect_status': 404
+    },
+    {
+        'path': 'api/index.php?url=v1/projects/projectID5e23a810ef693/wordcheck/ai',
+        'expect_status': 404
+    },
+    {
+        'path': 'api/index.php?url=v1/stats',
+        'expect_status': 404
+    },
+    {
+        'method': 'PUT',
+        'path': 'api/index.php?url=v1/projects/projectID5e23a810ef693/checkout&state=wibble',
+        'expect_status': 400
+    },
+    {
+        'method': 'PUT',
+        'path': 'api/index.php?url=v1/projects/projectID5e23a810ef693/pages/042.png&state=F1.proj_avail&pagestate=wibble',
+        'expect_status': 400
+    },
+    {
+        'path': 'api/index.php?url=v1/dictionaries&param1=invalid&param2',
+        'expect_status': 400
+    },
+    {
+        'path': 'api/index.php?url=v1/projects&per_page=1&page=1&state=P3.proj_avail&field[]=title&field[]=title',
+        'expect_status': 400
+    }
+]
+
 FAQ_TESTS = [
     {'path': 'faq/doc-copy.php'},
     {'path': 'faq/font_sample.php'},
@@ -450,6 +488,7 @@
     NOLOGIN_TESTS +
     BASE_TESTS +
     API_TESTS +
+    API_INVALID_TESTS +
     FAQ_TESTS +
     MISC_TESTS +
     QUIZ_TESTS +

api/ApiRouter.inc

@@ -64,25 +64,36 @@ class ApiRouter
         $node = $this->root;
         $data = [];
         $parts = explode("/", $url);
+        $path = "";
         foreach ($parts as $part) {
             $next_node = $node->children[$part] ?? null;
             if ($next_node) {
                 $node = $next_node;
             } else {
-                [$param_name, $validator] = $this->get_validator($node);
+                [$param_name, $validator] = $this->get_validator($path, $part, $node);
                 $node = $node->children[$param_name];
                 $data[$param_name] = $validator($part, $data);
             }
+            $path = "$path/$part";
         }
         if (empty($node->handlers)) {
-            throw new InvalidAPI();
+            throw new InvalidAPI($path, null, array_keys($node->children));
         }
         $method = $_SERVER["REQUEST_METHOD"];
         $handler = $node->handlers[$method] ?? null;
         if (!$handler) {
-            throw new MethodNotAllowed();
+            throw new MethodNotAllowed($url, $method, array_keys($node->handlers));
+        }
+        $ref = new ReflectionFunction($handler);
+        if ($ref->getNumberOfParameters() == 2) {
+            if (!empty($query_params)) {
+                $err = "API endpoint $path takes no query parameters, but was called with " . implode(", ", array_keys($query_params));
+                throw new InvalidParam($err);
+            }
+            $this->_response = $handler($method, $data);
+        } else {
+            $this->_response = $handler($method, $data, $query_params);
         }
-        $this->_response = $handler($method, $data, $query_params);
         return $this->_response;
     }
 
@@ -92,14 +103,19 @@ class ApiRouter
     }
 
     /** @return array{0: string, 1: callable} */
-    private function get_validator(TrieNode $node): array
+    private function get_validator(string $path, string $part, TrieNode $node): array
     {
-        foreach (array_keys($node->children) as $route) {
+        if (isset($node->children)) {
+            $children = array_keys($node->children);
+        } else {
+            $children = [];
+        }
+        foreach ($children as $route) {
             if (str_starts_with($route, ":")) {
                 return [$route, $this->_validators[$route]];
             }
         }
-        throw new InvalidAPI();
+        throw new InvalidAPI($path, $part, $children);
     }
 
     /** @return mixed */

api/exceptions.inc

@@ -7,14 +7,17 @@
  */
 class ApiException extends Exception
 {
-    public function __construct(string $message = "API exception", int $code = 1)
-    {
+    public function __construct(
+        string $message = "API exception",
+        int $code = 1,
+        public readonly int $status_code = 400
+    ) {
         parent::__construct($message, $code);
     }
 
-    public function getStatusCode()
+    public static function fromException(Exception $e): self
     {
-        return 400;
+        return new self($e->getMessage(), $e->getCode());
     }
 }
 
@@ -25,72 +28,55 @@ class BadRequest extends ApiException
 {
     public function __construct(string $message = "Bad request", int $code = 2)
     {
-        parent::__construct($message, $code);
+        parent::__construct($message, $code, 400);
     }
 }
 
 class UnauthorizedError extends ApiException
 {
     public function __construct(string $message = "Unauthorized", int $code = 3)
     {
-        parent::__construct($message, $code);
-    }
-
-    public function getStatusCode()
-    {
-        return 401;
+        parent::__construct($message, $code, 401);
     }
 }
 
 class NotFoundError extends ApiException
 {
     public function __construct(string $message = "Object not found", int $code = 4)
     {
-        parent::__construct($message, $code);
-    }
-
-    public function getStatusCode()
-    {
-        return 404;
+        parent::__construct($message, $code, 404);
     }
 }
 
 class RateLimitExceeded extends ApiException
 {
     public function __construct(string $message = "Rate limit exceeded", int $code = 5)
     {
-        parent::__construct($message, $code);
-    }
-
-    public function getStatusCode()
-    {
-        return 429;
+        parent::__construct($message, $code, 429);
     }
 }
 
 class InvalidValue extends ApiException
 {
     public function __construct(string $message = "Request contained an invalid value for a parameter", int $code = 6)
     {
-        parent::__construct($message, $code);
+        parent::__construct($message, $code, 400);
     }
+}
 
-    public function getStatusCode()
+class InvalidParam extends ApiException
+{
+    public function __construct(string $message = "Request contained an invalid parameter", int $code = 12)
     {
-        return 400;
+        parent::__construct($message, $code, 400);
     }
 }
 
 class ForbiddenError extends ApiException
 {
     public function __construct(string $message = "Forbidden", int $code = 7)
     {
-        parent::__construct($message, $code);
-    }
-
-    public function getStatusCode()
-    {
-        return 403;
+        parent::__construct($message, $code, 403);
     }
 }
 
@@ -108,27 +94,31 @@ class UnexpectedError extends ApiException
 
 class InvalidAPI extends ApiException
 {
-    public function __construct(string $message = "Invalid API path", int $code = 8)
-    {
-        parent::__construct($message, $code);
-    }
-
-    public function getStatusCode()
-    {
-        return 404;
+    public function __construct(string $url, ?string $part, array $children, int $code = 8)
+    {
+        if (is_null($part)) {
+            $err = "is missing a part";
+        } else {
+            $err = "has no part $part";
+        }
+
+        if (!empty($children)) {
+            $dym = " Valid child parts: " .implode(", ", $children) . ".";
+        } else {
+            $dym = "";
+        }
+        $message = "API endpoint $url $err.$dym";
+        parent::__construct($message, $code, 404);
     }
 }
 
 class MethodNotAllowed extends ApiException
 {
-    public function __construct(string $message = "API endpoint doesn't support this method", int $code = 9)
+    public function __construct(string $url, string $invalid, array $valids, $code = 9)
     {
-        parent::__construct($message, $code);
-    }
-
-    public function getStatusCode()
-    {
-        return 405;
+        $valid = implode(", ", $valids);
+        $message = "API endpoint $url: Method $invalid not supported. Valid methods: $valid.";
+        parent::__construct($message, $code, 405);
     }
 }
 
@@ -144,11 +134,6 @@ class ServerError extends ApiException
 {
     public function __construct(string $message = "An unhandled error happened on the server", int $code = 11)
     {
-        parent::__construct($message, $code);
-    }
-
-    public function getStatusCode()
-    {
-        return 500;
+        parent::__construct($message, $code, 500);
     }
 }

api/index.php

@@ -244,7 +244,7 @@ function handle_cors_headers()
 function production_exception_handler($exception)
 {
     if ($exception instanceof ApiException) {
-        $response_code = $exception->getStatusCode();
+        $response_code = $exception->status_code;
     } else {
         $response_code = 500;
     }

api/v1_docs.inc

@@ -36,8 +36,7 @@ function api_v1_document(string $method, array $data, array $query_params): stri
     return $faq_url;
 }
 
-/** @param array<string, string|string[]> $query_params */
-function api_v1_dictionaries(string $method, array $data, array $query_params): array
+function api_v1_dictionaries(string $method, array $data): array
 {
     $dict_list = get_languages_with_dictionaries();
     asort($dict_list);