From 0e37e2cf9a7f688fbcd8e5c607aa6b13628ed161 Mon Sep 17 00:00:00 2001 From: Brian Foley Date: Tue, 2 Jun 2026 13:41:19 +0000 Subject: [PATCH 1/7] ApiException: Use a readonly property to avoid boilerplate Readonly properties were introduced in PHP 8.1 --- api/exceptions.inc | 70 ++++++++++------------------------------------ api/index.php | 2 +- 2 files changed, 15 insertions(+), 57 deletions(-) diff --git a/api/exceptions.inc b/api/exceptions.inc index 2b05a74a38..83755cccae 100644 --- a/api/exceptions.inc +++ b/api/exceptions.inc @@ -7,15 +7,13 @@ */ 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() - { - return 400; - } } //--------------------------------------------------------------------------- @@ -25,7 +23,7 @@ class BadRequest extends ApiException { public function __construct(string $message = "Bad request", int $code = 2) { - parent::__construct($message, $code); + parent::__construct($message, $code, 400); } } @@ -33,12 +31,7 @@ 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); } } @@ -46,12 +39,7 @@ 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); } } @@ -59,12 +47,7 @@ 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); } } @@ -72,12 +55,7 @@ class InvalidValue extends ApiException { public function __construct(string $message = "Request contained an invalid value for a parameter", int $code = 6) { - parent::__construct($message, $code); - } - - public function getStatusCode() - { - return 400; + parent::__construct($message, $code, 400); } } @@ -85,12 +63,7 @@ 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); } } @@ -110,12 +83,7 @@ class InvalidAPI extends ApiException { public function __construct(string $message = "Invalid API path", int $code = 8) { - parent::__construct($message, $code); - } - - public function getStatusCode() - { - return 404; + parent::__construct($message, $code, 404); } } @@ -123,12 +91,7 @@ class MethodNotAllowed extends ApiException { public function __construct(string $message = "API endpoint doesn't support this method", int $code = 9) { - parent::__construct($message, $code); - } - - public function getStatusCode() - { - return 405; + parent::__construct($message, $code, 405); } } @@ -144,11 +107,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); } } diff --git a/api/index.php b/api/index.php index 39e1e1dec8..adb215c57d 100644 --- a/api/index.php +++ b/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; } From 9a9fd532fd1fd3ebc07edabbd38cb80964e0cdb2 Mon Sep 17 00:00:00 2001 From: Brian Foley Date: Tue, 2 Jun 2026 13:54:51 +0000 Subject: [PATCH 2/7] Improve MethodNotAllowed error for clients. --- SETUP/tests/smoketests/pageload_smoketest.py | 9 +++++++++ api/ApiRouter.inc | 2 +- api/exceptions.inc | 4 +++- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/SETUP/tests/smoketests/pageload_smoketest.py b/SETUP/tests/smoketests/pageload_smoketest.py index ad35161490..de31df51c0 100755 --- a/SETUP/tests/smoketests/pageload_smoketest.py +++ b/SETUP/tests/smoketests/pageload_smoketest.py @@ -71,6 +71,14 @@ {'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 + }, +] + FAQ_TESTS = [ {'path': 'faq/doc-copy.php'}, {'path': 'faq/font_sample.php'}, @@ -450,6 +458,7 @@ NOLOGIN_TESTS + BASE_TESTS + API_TESTS + + API_INVALID_TESTS + FAQ_TESTS + MISC_TESTS + QUIZ_TESTS + diff --git a/api/ApiRouter.inc b/api/ApiRouter.inc index ce10fdad90..602745b9c7 100644 --- a/api/ApiRouter.inc +++ b/api/ApiRouter.inc @@ -80,7 +80,7 @@ class ApiRouter $method = $_SERVER["REQUEST_METHOD"]; $handler = $node->handlers[$method] ?? null; if (!$handler) { - throw new MethodNotAllowed(); + throw new MethodNotAllowed($url, $method, array_keys($node->handlers)); } $this->_response = $handler($method, $data, $query_params); return $this->_response; diff --git a/api/exceptions.inc b/api/exceptions.inc index 83755cccae..e8ce6ab0d7 100644 --- a/api/exceptions.inc +++ b/api/exceptions.inc @@ -89,8 +89,10 @@ class InvalidAPI extends ApiException 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) { + $valid = implode(", ", $valids); + $message = "API endpoint $url: Method $invalid not supported. Valid methods: $valid."; parent::__construct($message, $code, 405); } } From ea1a4542565c71e8011c782d7a74fb61e908f68d Mon Sep 17 00:00:00 2001 From: Brian Foley Date: Tue, 2 Jun 2026 14:30:39 +0000 Subject: [PATCH 3/7] Improve InvalidAPI errors for clients. Handle the following cases: 1. A misspelled part is provided 2. A part for a node with no children 3. A missing part for nodes with children but no handler of its own. --- SETUP/tests/smoketests/pageload_smoketest.py | 12 ++++++++++++ api/ApiRouter.inc | 17 ++++++++++++----- api/exceptions.inc | 14 +++++++++++++- 3 files changed, 37 insertions(+), 6 deletions(-) diff --git a/SETUP/tests/smoketests/pageload_smoketest.py b/SETUP/tests/smoketests/pageload_smoketest.py index de31df51c0..ef54d39b03 100755 --- a/SETUP/tests/smoketests/pageload_smoketest.py +++ b/SETUP/tests/smoketests/pageload_smoketest.py @@ -77,6 +77,18 @@ '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 + }, ] FAQ_TESTS = [ diff --git a/api/ApiRouter.inc b/api/ApiRouter.inc index 602745b9c7..67f9be1ddb 100644 --- a/api/ApiRouter.inc +++ b/api/ApiRouter.inc @@ -64,18 +64,20 @@ 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; @@ -92,14 +94,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 */ diff --git a/api/exceptions.inc b/api/exceptions.inc index e8ce6ab0d7..0159756a7e 100644 --- a/api/exceptions.inc +++ b/api/exceptions.inc @@ -81,8 +81,20 @@ class UnexpectedError extends ApiException class InvalidAPI extends ApiException { - public function __construct(string $message = "Invalid API path", int $code = 8) + 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); } } From 6a3afbebdb6a35c9dc8b85195b8d0ba2dcfe84a5 Mon Sep 17 00:00:00 2001 From: Brian Foley Date: Tue, 2 Jun 2026 14:47:55 +0000 Subject: [PATCH 4/7] api/v1/projects: Improve error messages for invalid proj/page state --- SETUP/tests/smoketests/pageload_smoketest.py | 10 ++++++++++ api/v1_projects.inc | 12 ++++++++---- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/SETUP/tests/smoketests/pageload_smoketest.py b/SETUP/tests/smoketests/pageload_smoketest.py index ef54d39b03..0922c24987 100755 --- a/SETUP/tests/smoketests/pageload_smoketest.py +++ b/SETUP/tests/smoketests/pageload_smoketest.py @@ -89,6 +89,16 @@ '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 + }, ] FAQ_TESTS = [ diff --git a/api/v1_projects.inc b/api/v1_projects.inc index 4689576917..127f96d4e2 100644 --- a/api/v1_projects.inc +++ b/api/v1_projects.inc @@ -1019,8 +1019,10 @@ function validate_project_state(Project $project, ?string $state): void if (null === $state) { throw new InvalidValue("No project state found in request."); } - if (!in_array($state, ProjectStates::get_states())) { - throw new InvalidValue("Invalid project state"); + $valid = ProjectStates::get_states(); + if (!in_array($state, $valid)) { + $valid = "Valid states: " . implode(", ", $valid) . "."; + throw new InvalidValue("Invalid project state $state. $valid"); } if ($state != $project->state) { $err = sprintf( @@ -1038,8 +1040,10 @@ function validate_page_state(ProjectPage $project_page, ?string $page_state): vo if (null === $page_state) { throw new InvalidValue("No page state found in request."); } - if (!in_array($page_state, Rounds::get_page_states())) { - throw new InvalidValue(sprintf("%s is not a valid page state", $page_state)); + $valid = Rounds::get_page_states(); + if (!in_array($page_state, $valid)) { + $valid = "Valid states: " . implode(", ", $valid) . "."; + throw new InvalidValue("Invalid page state $page_state. $valid"); } if ($page_state != $project_page->page_state) { $err = sprintf( From 807bcbfea7fedeb3ab437fcb6a324446538cb46f Mon Sep 17 00:00:00 2001 From: Brian Foley Date: Tue, 2 Jun 2026 15:20:52 +0000 Subject: [PATCH 5/7] api/v1: Error if endpoint w. no params is called with params This has two benefits: 1. It tightens up the type signature of the handler functions (as well as removing boilerplate). 2. It catches latent errors in the client. --- SETUP/tests/smoketests/pageload_smoketest.py | 4 ++ api/ApiRouter.inc | 11 +++- api/exceptions.inc | 8 +++ api/v1_docs.inc | 3 +- api/v1_projects.inc | 54 +++++++------------- api/v1_queues.inc | 6 +-- api/v1_stats.inc | 22 +++----- api/v1_storage.inc | 6 +-- 8 files changed, 52 insertions(+), 62 deletions(-) diff --git a/SETUP/tests/smoketests/pageload_smoketest.py b/SETUP/tests/smoketests/pageload_smoketest.py index 0922c24987..cc98807dc2 100755 --- a/SETUP/tests/smoketests/pageload_smoketest.py +++ b/SETUP/tests/smoketests/pageload_smoketest.py @@ -99,6 +99,10 @@ '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¶m1=invalid¶m2', + 'expect_status': 400 + }, ] FAQ_TESTS = [ diff --git a/api/ApiRouter.inc b/api/ApiRouter.inc index 67f9be1ddb..d9b0b0a13c 100644 --- a/api/ApiRouter.inc +++ b/api/ApiRouter.inc @@ -84,7 +84,16 @@ class ApiRouter if (!$handler) { throw new MethodNotAllowed($url, $method, array_keys($node->handlers)); } - $this->_response = $handler($method, $data, $query_params); + $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); + } return $this->_response; } diff --git a/api/exceptions.inc b/api/exceptions.inc index 0159756a7e..c3ae3c9721 100644 --- a/api/exceptions.inc +++ b/api/exceptions.inc @@ -59,6 +59,14 @@ class InvalidValue extends ApiException } } +class InvalidParam extends ApiException +{ + public function __construct(string $message = "Request contained an invalid parameter", int $code = 12) + { + parent::__construct($message, $code, 400); + } +} + class ForbiddenError extends ApiException { public function __construct(string $message = "Forbidden", int $code = 7) diff --git a/api/v1_docs.inc b/api/v1_docs.inc index 08998d05f5..c42210cad6 100755 --- a/api/v1_docs.inc +++ b/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 $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); diff --git a/api/v1_projects.inc b/api/v1_projects.inc index 127f96d4e2..934d351026 100644 --- a/api/v1_projects.inc +++ b/api/v1_projects.inc @@ -380,8 +380,7 @@ function render_project_json(Project $project, ?array $return_fields = null) //--------------------------------------------------------------------------- // projects/:projectID/artifacts/:stageid -/** @param array $query_params */ -function api_v1_project_artifacts(string $method, array $data, array $query_params) +function api_v1_project_artifacts(string $method, array $data) { // get the project and the stage $project = $data[":projectid"]; @@ -422,8 +421,7 @@ function api_v1_project_artifacts(string $method, array $data, array $query_para //--------------------------------------------------------------------------- // projects/:projectID/wordlists/:type -/** @param array $query_params */ -function api_v1_project_wordlists(string $method, array $data, array $query_params) +function api_v1_project_wordlists(string $method, array $data) { // get the project this is for and the type of word list $project = $data[":projectid"]; @@ -456,8 +454,7 @@ function api_v1_project_wordlists(string $method, array $data, array $query_para //--------------------------------------------------------------------------- // projects/:projectID/holdstates -/** @param array $query_params */ -function api_v1_project_holdstates(string $method, array $data, array $query_params) +function api_v1_project_holdstates(string $method, array $data) { $project = $data[":projectid"]; @@ -497,8 +494,7 @@ function api_v1_project_holdstates(string $method, array $data, array $query_par //--------------------------------------------------------------------------- // projects/:projectid/pages -/** @param array $query_params */ -function api_v1_project_pages(string $method, array $data, array $query_params) +function api_v1_project_pages(string $method, array $data) { $project = $data[":projectid"]; @@ -576,8 +572,7 @@ function api_v1_project_pagedetails(string $method, array $data, array $query_pa //--------------------------------------------------------------------------- // projects/:projectid/pages/:pagename/pagerounds/:pageroundid -/** @param array $query_params */ -function api_v1_project_page_round(string $method, array $data, array $query_params) +function api_v1_project_page_round(string $method, array $data) { if ($data[":pageroundid"] == "OCR") { $text_column = "master_text"; @@ -636,8 +631,7 @@ function render_project_page_json($row) //--------------------------------------------------------------------------- // projects/:projectid/transitions -/** @param array $query_params */ -function api_v1_project_transitions(string $method, array $data, array $query_params) +function api_v1_project_transitions(string $method, array $data) { $sql = sprintf( " @@ -664,8 +658,7 @@ function api_v1_project_transitions(string $method, array $data, array $query_pa //--------------------------------------------------------------------------- // projects/difficulties -/** @param array $query_params */ -function api_v1_projects_difficulties(string $method, array $data, array $query_params) +function api_v1_projects_difficulties(string $method, array $data) { $difficulties = get_project_difficulties(); return array_keys($difficulties); @@ -674,8 +667,7 @@ function api_v1_projects_difficulties(string $method, array $data, array $query_ //--------------------------------------------------------------------------- // projects/genres -/** @param array $query_params */ -function api_v1_projects_genres(string $method, array $data, array $query_params) +function api_v1_projects_genres(string $method, array $data) { $genres = ProjectSearchForm::genre_options(); unset($genres['']); @@ -685,8 +677,7 @@ function api_v1_projects_genres(string $method, array $data, array $query_params //--------------------------------------------------------------------------- // projects/languages -/** @param array $query_params */ -function api_v1_projects_languages(string $method, array $data, array $query_params) +function api_v1_projects_languages(string $method, array $data) { $languages = ProjectSearchForm::language_options(); unset($languages['']); @@ -696,8 +687,7 @@ function api_v1_projects_languages(string $method, array $data, array $query_par //--------------------------------------------------------------------------- // projects/states -/** @param array $query_params */ -function api_v1_projects_states(string $method, array $data, array $query_params) +function api_v1_projects_states(string $method, array $data) { $states = ProjectSearchForm::state_options(); unset($states['']); @@ -707,8 +697,7 @@ function api_v1_projects_states(string $method, array $data, array $query_params //--------------------------------------------------------------------------- // projects/pagerounds -/** @param array $query_params */ -function api_v1_projects_pagerounds(string $method, array $data, array $query_params) +function api_v1_projects_pagerounds(string $method, array $data) { return array_merge(["OCR"], Rounds::get_ids()); } @@ -817,8 +806,7 @@ function api_v1_projects_imagesources(string $method, array $data, array $query_ //--------------------------------------------------------------------------- // projects/holdstates -/** @param array $query_params */ -function api_v1_projects_holdstates(string $method, array $data, array $query_params): array +function api_v1_projects_holdstates(string $method, array $data): array { return Project::get_holdable_states(); } @@ -862,16 +850,14 @@ function api_v1_project_checkout(string $method, array $data, array $query_param } } -/** @param array $query_params */ -function api_v1_project_validatetext(string $method, array $data, array $query_params): array +function api_v1_project_validatetext(string $method, array $data): array { $project = $data[":projectid"]; $invalid_characters = $project->find_invalid_characters(receive_project_text_from_request_body()); return ["invalid_chars" => $invalid_characters]; } -/** @param array $query_params */ -function api_v1_project_wordcheck(string $method, array $data, array $query_params): array +function api_v1_project_wordcheck(string $method, array $data): array { $project = $data[":projectid"]; $accepted_words = receive_data_from_request_body("accepted_words") ?? []; @@ -888,8 +874,7 @@ function api_v1_project_wordcheck(string $method, array $data, array $query_para ]; } -/** @param array $query_params */ -function api_v1_project_pickersets(string $method, array $data, array $query_params): array +function api_v1_project_pickersets(string $method, array $data): array { $project = $data[":projectid"]; send_cache_header(60 * 60, "public"); @@ -949,8 +934,7 @@ function api_v1_project_page(string $method, array $data, array $query_params) } } -/** @param array $query_params */ -function api_v1_project_page_format_preview(string $method, array $data, array $query_params): void +function api_v1_project_page_format_preview(string $method, array $data): void { try { $project = $data[":projectid"]; @@ -967,8 +951,7 @@ function api_v1_project_page_format_preview(string $method, array $data, array $ } } -/** @param array $query_params */ -function api_v1_project_page_report_bad(string $method, array $data, array $query_params): void +function api_v1_project_page_report_bad(string $method, array $data): void { global $pguser, $PAGE_BADNESS_REASONS; @@ -996,8 +979,7 @@ function api_v1_project_page_report_bad(string $method, array $data, array $quer } } -/** @param array $query_params */ -function api_v1_project_page_wordcheck(string $method, array $data, array $query_params): void +function api_v1_project_page_wordcheck(string $method, array $data): void { try { $project = $data[":projectid"]; diff --git a/api/v1_queues.inc b/api/v1_queues.inc index 6b3e108c42..9f43b36064 100644 --- a/api/v1_queues.inc +++ b/api/v1_queues.inc @@ -56,8 +56,7 @@ function api_v1_queues(string $method, array $data, array $query_params): array //--------------------------------------------------------------------------- // queues/:queueid -- return release queue info -/** @param array $query_params */ -function api_v1_queue(string $method, array $data, array $query_params): array +function api_v1_queue(string $method, array $data): array { $queue = $data[":queueid"]; @@ -66,8 +65,7 @@ function api_v1_queue(string $method, array $data, array $query_params): array //--------------------------------------------------------------------------- // queue/:queueid/stats -- return release queue stats -/** @param array $query_params */ -function api_v1_queue_stats(string $method, array $data, array $query_params): array +function api_v1_queue_stats(string $method, array $data): array { $queue = $data[":queueid"]; diff --git a/api/v1_stats.inc b/api/v1_stats.inc index c0452388e5..639a9c4548 100644 --- a/api/v1_stats.inc +++ b/api/v1_stats.inc @@ -6,8 +6,7 @@ include_once("exceptions.inc"); //--------------------------------------------------------------------------- // stats/site -/** @param array $query_params */ -function api_v1_stats_site(string $method, array $data, array $query_params) +function api_v1_stats_site(string $method, array $data) { $res = DPDatabase::query("SELECT COUNT(*) FROM users"); [$registered_users] = mysqli_fetch_row($res); @@ -54,8 +53,7 @@ function api_v1_stats_site(string $method, array $data, array $query_params) // "proj_post_second_checked_out": 2, // "proj_delete": 46, // }``` -/** @param array $query_params */ -function api_v1_stats_site_projects_states(string $method, array $data, array $query_params) +function api_v1_stats_site_projects_states(string $method, array $data) { // Make sure all project state statistics are provided. $output = []; @@ -76,8 +74,7 @@ function api_v1_stats_site_projects_states(string $method, array $data, array $q return $output; } -/** @param array $query_params */ -function api_v1_stats_site_projects_stages(string $method, array $data, array $query_params) +function api_v1_stats_site_projects_stages(string $method, array $data) { // Make sure all project stage statistics are provided. $output = []; @@ -139,8 +136,7 @@ function render_round_stats($round_id) ]; } -/** @param array $query_params */ -function api_v1_stats_site_rounds(string $method, array $data, array $query_params) +function api_v1_stats_site_rounds(string $method, array $data) { $return = []; foreach (Rounds::get_ids() as $round_id) { @@ -153,8 +149,7 @@ function api_v1_stats_site_rounds(string $method, array $data, array $query_para //--------------------------------------------------------------------------- // stats/site/rounds/:roundid -/** @param array $query_params */ -function api_v1_stats_site_round(string $method, array $data, array $query_params) +function api_v1_stats_site_round(string $method, array $data) { $round = $data[":roundid"]; @@ -189,8 +184,7 @@ function render_round_user_stats(User $user, Round $round) ]; } -/** @param array $query_params */ -function api_v1_stats_user_rounds(string $method, array $data, array $query_params) +function api_v1_stats_user_rounds(string $method, array $data) { $user = $data[":username"]; @@ -204,9 +198,7 @@ function api_v1_stats_user_rounds(string $method, array $data, array $query_para //--------------------------------------------------------------------------- // stats/user/:username/rounds/:roundid -// -/** @param array $query_params */ -function api_v1_stats_user_round(string $method, array $data, array $query_params) +function api_v1_stats_user_round(string $method, array $data) { $user = $data[":username"]; $round = $data[":roundid"]; diff --git a/api/v1_storage.inc b/api/v1_storage.inc index 9444480648..1ae99cef1f 100644 --- a/api/v1_storage.inc +++ b/api/v1_storage.inc @@ -8,10 +8,9 @@ /** * Store/retrieve storage item (depending on the method). * - * @param array $query_params * @return mixed */ -function api_v1_storage(string $method, array $data, array $query_params) +function api_v1_storage(string $method, array $data) { global $pguser; @@ -34,8 +33,7 @@ function api_v1_storage(string $method, array $data, array $query_params) } } -/** @param array $query_params */ -function api_v1_storage_delete(string $method, array $data, array $query_params): void +function api_v1_storage_delete(string $method, array $data): void { global $pguser; From de3324fbf59592a7a88b28bbeac7c690e9bcb8e5 Mon Sep 17 00:00:00 2001 From: Brian Foley Date: Tue, 2 Jun 2026 15:50:17 +0000 Subject: [PATCH 6/7] api/v1/projects: Error on duplicated field[] arguments Because of how the ouput JSON is built, api/v1/projects/ has the effect of sliently deduplicating the field. Report it as an error, as it may be a latent bug in the client --- SETUP/tests/smoketests/pageload_smoketest.py | 4 ++++ api/v1_projects.inc | 9 +++++++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/SETUP/tests/smoketests/pageload_smoketest.py b/SETUP/tests/smoketests/pageload_smoketest.py index cc98807dc2..6fb6ae12ae 100755 --- a/SETUP/tests/smoketests/pageload_smoketest.py +++ b/SETUP/tests/smoketests/pageload_smoketest.py @@ -103,6 +103,10 @@ 'path': 'api/index.php?url=v1/dictionaries¶m1=invalid¶m2', '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 = [ diff --git a/api/v1_projects.inc b/api/v1_projects.inc index 934d351026..c351784baf 100644 --- a/api/v1_projects.inc +++ b/api/v1_projects.inc @@ -19,7 +19,7 @@ include_once("api_common.inc"); * @param ?Project $project * If we're querying a single project, what is it? (Certain fields * aren't queryable or viewable by most users.) - * @return ?array + * @return ?array null means 'render all fields' * @throws InvalidValue if any of the filter arguments aren't recognized */ function get_return_fields(array $query_params, ?Project $project) @@ -31,7 +31,12 @@ function get_return_fields(array $query_params, ?Project $project) $valid_render_fields = array_keys(get_project_fields_with_attr(null, $project)); $invalid_fields = array_diff($return_fields, $valid_render_fields); if (!empty($invalid_fields)) { - throw new InvalidValue("Invalid filter args: " . implode(", ", $invalid_fields)); + throw new InvalidValue("Invalid field[] args: " . implode(", ", $invalid_fields)); + } + /** @var list $return_fields */ + $duplicate_fields = array_keys(array_filter(array_count_values($return_fields), fn ($c) => $c > 1)); + if (!empty($duplicate_fields)) { + throw new InvalidValue("Duplicate field[] args: " . implode(", ", $duplicate_fields)); } return $return_fields; } From 8df072fbf85c1a3265034529d7390cb7c7482b38 Mon Sep 17 00:00:00 2001 From: Brian Foley Date: Tue, 2 Jun 2026 16:13:44 +0000 Subject: [PATCH 7/7] Add ApiException::fromException builder to avoid conversion boilerplate --- api/exceptions.inc | 5 +++++ api/v1_projects.inc | 28 ++++++++++++++-------------- api/v1_validators.inc | 12 ++++++------ 3 files changed, 25 insertions(+), 20 deletions(-) diff --git a/api/exceptions.inc b/api/exceptions.inc index c3ae3c9721..4e28ace9ba 100644 --- a/api/exceptions.inc +++ b/api/exceptions.inc @@ -14,6 +14,11 @@ class ApiException extends Exception ) { parent::__construct($message, $code); } + + public static function fromException(Exception $e): self + { + return new self($e->getMessage(), $e->getCode()); + } } //--------------------------------------------------------------------------- diff --git a/api/v1_projects.inc b/api/v1_projects.inc index c351784baf..8268290465 100644 --- a/api/v1_projects.inc +++ b/api/v1_projects.inc @@ -264,7 +264,7 @@ function create_or_update_project(Project $project) $project->validate(true); } catch (ProjectException $exception) { - throw new InvalidValue($exception->getMessage(), $exception->getCode()); + throw InvalidValue::fromException($exception); } // validate charsuites @@ -513,7 +513,7 @@ function api_v1_project_pages(string $method, array $data) ]; } } catch (NoProjectPageTable $exception) { - throw new NotFoundError($exception->getMessage(), $exception->getCode()); + throw NotFoundError::fromException($exception); } return $return_data; } @@ -545,7 +545,7 @@ function api_v1_project_pagedetails(string $method, array $data, array $query_pa try { $rounds_to_display = get_rounds_to_display($project); } catch (NoProjectPageTable $exception) { - throw new NotFoundError($exception->getMessage(), $exception->getCode()); + throw NotFoundError::fromException($exception); } if (!is_null($only_rounds)) { $rounds_to_display = array_intersect($rounds_to_display, $only_rounds); @@ -849,9 +849,9 @@ function api_v1_project_checkout(string $method, array $data, array $query_param $proof_project = new ProofProject($project); return $proof_project->checkout(); } catch (ProjectException $exception) { - throw new NotFoundError($exception->getMessage(), $exception->getCode()); + throw NotFoundError::fromException($exception); } catch (UserAccessException $exception) { - throw new ForbiddenError($exception->getMessage(), $exception->getCode()); + throw ForbiddenError::fromException($exception); } } @@ -933,9 +933,9 @@ function api_v1_project_page(string $method, array $data, array $query_params) throw new BadRequest("$page_action is not a valid page action."); } } catch (ProjectException $exception) { - throw new NotFoundError($exception->getMessage(), $exception->getCode()); + throw NotFoundError::fromException($exception); } catch (UserAccessException $exception) { - throw new ForbiddenError($exception->getMessage(), $exception->getCode()); + throw ForbiddenError::fromException($exception); } } @@ -950,9 +950,9 @@ function api_v1_project_page_format_preview(string $method, array $data): void $proof_project_page->fp_report(); } catch (ProjectException $exception) { - throw new NotFoundError($exception->getMessage(), $exception->getCode()); + throw NotFoundError::fromException($exception); } catch (UserAccessException $exception) { - throw new ForbiddenError($exception->getMessage(), $exception->getCode()); + throw ForbiddenError::fromException($exception); } } @@ -976,11 +976,11 @@ function api_v1_project_page_report_bad(string $method, array $data): void } $proof_project_page->markAsBad($pguser, $reason_key); } catch (ProjectTransitionException $exception) { - throw new UnexpectedError($exception->getMessage(), $exception->getCode()); + throw UnexpectedError::fromException($exception); } catch (ProjectException $exception) { - throw new BadRequest($exception->getMessage(), $exception->getCode()); + throw BadRequest::fromException($exception); } catch (UserAccessException $exception) { - throw new ForbiddenError($exception->getMessage(), $exception->getCode()); + throw ForbiddenError::fromException($exception); } } @@ -995,9 +995,9 @@ function api_v1_project_page_wordcheck(string $method, array $data): void $proof_project_page->wc_report(receive_data_from_request_body("accepted_words") ?? []); } catch (ProjectException $exception) { - throw new NotFoundError($exception->getMessage(), $exception->getCode()); + throw NotFoundError::fromException($exception); } catch (UserAccessException $exception) { - throw new ForbiddenError($exception->getMessage(), $exception->getCode()); + throw ForbiddenError::fromException($exception); } } diff --git a/api/v1_validators.inc b/api/v1_validators.inc index 92281654a9..8f9fb9927f 100644 --- a/api/v1_validators.inc +++ b/api/v1_validators.inc @@ -14,7 +14,7 @@ function validate_stage(string $stageid, array $_data): Stage } return $stage; } catch (InvalidValue $exception) { - throw new NotFoundError($exception->getMessage(), $exception->getCode()); + throw NotFoundError::fromException($exception); } } @@ -27,7 +27,7 @@ function validate_round(string $roundid, array $_data): Round } return $round; } catch (InvalidValue $exception) { - throw new NotFoundError($exception->getMessage(), $exception->getCode()); + throw NotFoundError::fromException($exception); } } @@ -37,7 +37,7 @@ function validate_project(string $projectid, array $_data): Project try { return new Project($projectid); } catch (NonexistentProjectException $exception) { - throw new NotFoundError($exception->getMessage(), $exception->getCode()); + throw NotFoundError::fromException($exception); } } @@ -54,7 +54,7 @@ function validate_page_name(string $pagename, array $data): ProjectPage try { return $data[":projectid"]->get_project_page($pagename); } catch (NonexistentPageException | NoProjectPageTable $exception) { - throw new NotFoundError($exception->getMessage(), $exception->getCode()); + throw NotFoundError::fromException($exception); } } @@ -68,7 +68,7 @@ function validate_page_round(string $pageround, array $_data): string } return $pageround; } catch (InvalidPageRoundException $exception) { - throw new NotFoundError($exception->getMessage(), $exception->getCode()); + throw NotFoundError::fromException($exception); } } @@ -104,6 +104,6 @@ function validate_username(string $username): User try { return new User($username); } catch (NonexistentUserException | NonuniqueUserException $exception) { - throw new NotFoundError($exception->getMessage(), $exception->getCode()); + throw NotFoundError::fromException($exception); } }