From 2422aae57711fafcedde6b381a0624ca67fa79cb Mon Sep 17 00:00:00 2001 From: wn_ Date: Thu, 18 Nov 2021 18:07:43 +0000 Subject: [PATCH 1/5] Consistently handle param string to bool conversions in handlers. --- classes/api.php | 40 +++++++++++++++----------------------- classes/feeds.php | 2 +- classes/handler.php | 6 ++++++ classes/handler/public.php | 4 ++-- classes/pref/feeds.php | 8 ++++---- classes/rpc.php | 4 ++-- 6 files changed, 31 insertions(+), 33 deletions(-) diff --git a/classes/api.php b/classes/api.php index 82d3ce208..f911cd591 100755 --- a/classes/api.php +++ b/classes/api.php @@ -16,13 +16,6 @@ class API extends Handler { /** @var int|null */ private $seq; - /** - * @param mixed $p - */ - private static function _param_to_bool($p): bool { - return $p && ($p !== "f" && $p !== "false"); - } - /** * @param array $reply */ @@ -110,7 +103,7 @@ class API extends Handler { function getUnread(): bool { $feed_id = clean($_REQUEST["feed_id"] ?? ""); - $is_cat = clean($_REQUEST["is_cat"] ?? ""); + $is_cat = self::_param_to_bool($_REQUEST["is_cat"] ?? null); if ($feed_id) { return $this->_wrap(self::STATUS_OK, array("unread" => getFeedUnread($feed_id, $is_cat))); @@ -126,10 +119,10 @@ class API extends Handler { function getFeeds(): bool { $cat_id = (int) clean($_REQUEST["cat_id"]); - $unread_only = self::_param_to_bool(clean($_REQUEST["unread_only"] ?? 0)); + $unread_only = self::_param_to_bool($_REQUEST["unread_only"] ?? null); $limit = (int) clean($_REQUEST["limit"] ?? 0); $offset = (int) clean($_REQUEST["offset"] ?? 0); - $include_nested = self::_param_to_bool(clean($_REQUEST["include_nested"] ?? false)); + $include_nested = self::_param_to_bool($_REQUEST["include_nested"] ?? null); $feeds = $this->_api_get_feeds($cat_id, $unread_only, $limit, $offset, $include_nested); @@ -137,9 +130,9 @@ class API extends Handler { } function getCategories(): bool { - $unread_only = self::_param_to_bool(clean($_REQUEST["unread_only"] ?? false)); - $enable_nested = self::_param_to_bool(clean($_REQUEST["enable_nested"] ?? false)); - $include_empty = self::_param_to_bool(clean($_REQUEST['include_empty'] ?? false)); + $unread_only = self::_param_to_bool($_REQUEST["unread_only"] ?? null); + $enable_nested = self::_param_to_bool($_REQUEST["enable_nested"] ?? null); + $include_empty = self::_param_to_bool($_REQUEST["include_empty"] ?? null); // TODO do not return empty categories, return Uncategorized and standard virtual cats @@ -204,21 +197,20 @@ class API extends Handler { $offset = (int)clean($_REQUEST["skip"] ?? 0); $filter = clean($_REQUEST["filter"] ?? ""); - $is_cat = self::_param_to_bool(clean($_REQUEST["is_cat"] ?? false)); - $show_excerpt = self::_param_to_bool(clean($_REQUEST["show_excerpt"] ?? false)); - $show_content = self::_param_to_bool(clean($_REQUEST["show_content"] ?? false)); + $is_cat = self::_param_to_bool($_REQUEST["is_cat"] ?? null); + $show_excerpt = self::_param_to_bool($_REQUEST["show_excerpt"] ?? null); + $show_content = self::_param_to_bool($_REQUEST["show_content"] ?? null); /* all_articles, unread, adaptive, marked, updated */ $view_mode = clean($_REQUEST["view_mode"] ?? null); - $include_attachments = self::_param_to_bool(clean($_REQUEST["include_attachments"] ?? false)); + $include_attachments = self::_param_to_bool($_REQUEST["include_attachments"] ?? null); $since_id = (int)clean($_REQUEST["since_id"] ?? 0); - $include_nested = self::_param_to_bool(clean($_REQUEST["include_nested"] ?? false)); - $sanitize_content = !isset($_REQUEST["sanitize"]) || - self::_param_to_bool($_REQUEST["sanitize"]); - $force_update = self::_param_to_bool(clean($_REQUEST["force_update"] ?? false)); - $has_sandbox = self::_param_to_bool(clean($_REQUEST["has_sandbox"] ?? false)); + $include_nested = self::_param_to_bool($_REQUEST["include_nested"] ?? null); + $sanitize_content = self::_param_to_bool($_REQUEST["sanitize"] ?? true); + $force_update = self::_param_to_bool($_REQUEST["force_update"] ?? null); + $has_sandbox = self::_param_to_bool($_REQUEST["has_sandbox"] ?? null); $excerpt_length = (int)clean($_REQUEST["excerpt_length"] ?? 0); $check_first_id = (int)clean($_REQUEST["check_first_id"] ?? 0); - $include_header = self::_param_to_bool(clean($_REQUEST["include_header"] ?? false)); + $include_header = self::_param_to_bool($_REQUEST["include_header"] ?? null); $_SESSION['hasSandbox'] = $has_sandbox; @@ -417,7 +409,7 @@ class API extends Handler { function catchupFeed(): bool { $feed_id = clean($_REQUEST["feed_id"]); - $is_cat = self::_param_to_bool($_REQUEST["is_cat"] ?? false); + $is_cat = self::_param_to_bool($_REQUEST["is_cat"] ?? null); $mode = clean($_REQUEST["mode"] ?? ""); if (!in_array($mode, ["all", "1day", "1week", "2week"])) diff --git a/classes/feeds.php b/classes/feeds.php index 970db0107..f71e4b3e8 100755 --- a/classes/feeds.php +++ b/classes/feeds.php @@ -456,7 +456,7 @@ class Feeds extends Handler_Protected { $method = $_REQUEST["m"] ?? ""; $view_mode = $_REQUEST["view_mode"] ?? ""; $limit = 30; - $cat_view = $_REQUEST["cat"] == "true"; + $cat_view = self::_param_to_bool($_REQUEST["cat"] ?? null); $next_unread_feed = $_REQUEST["nuf"] ?? 0; $offset = (int) ($_REQUEST["skip"] ?? 0); $order_by = $_REQUEST["order_by"] ?? ""; diff --git a/classes/handler.php b/classes/handler.php index 3ee42cedb..aca5bf4d2 100644 --- a/classes/handler.php +++ b/classes/handler.php @@ -27,4 +27,10 @@ class Handler implements IHandler { return true; } + /** + * @param mixed $p + */ + protected static function _param_to_bool($p): bool { + return $p && ($p !== "f" && $p !== "false"); + } } diff --git a/classes/handler/public.php b/classes/handler/public.php index b5282c222..6d8f0bd8f 100755 --- a/classes/handler/public.php +++ b/classes/handler/public.php @@ -307,7 +307,7 @@ class Handler_Public extends Handler { function rss(): void { $feed = clean($_REQUEST["id"]); $key = clean($_REQUEST["key"]); - $is_cat = clean($_REQUEST["is_cat"] ?? false); + $is_cat = self::_param_to_bool($_REQUEST["is_cat"] ?? null); $limit = (int)clean($_REQUEST["limit"] ?? 0); $offset = (int)clean($_REQUEST["offset"] ?? 0); @@ -317,7 +317,7 @@ class Handler_Public extends Handler { $start_ts = clean($_REQUEST["ts"] ?? ""); $format = clean($_REQUEST['format'] ?? "atom"); - $orig_guid = clean($_REQUEST["orig_guid"] ?? false); + $orig_guid = clean($_REQUEST["orig_guid"] ?? ""); if (Config::get(Config::SINGLE_USER_MODE)) { UserHelper::authenticate("admin", null); diff --git a/classes/pref/feeds.php b/classes/pref/feeds.php index 47479e124..00fd140fe 100755 --- a/classes/pref/feeds.php +++ b/classes/pref/feeds.php @@ -47,7 +47,7 @@ class Pref_Feeds extends Handler_Protected { $search = ""; // first one is set by API - $show_empty_cats = clean($_REQUEST['force_show_empty'] ?? false) || + $show_empty_cats = self::_param_to_bool($_REQUEST['force_show_empty'] ?? null) || (clean($_REQUEST['mode'] ?? 0) != 2 && !$search); $items = []; @@ -208,7 +208,7 @@ class Pref_Feeds extends Handler_Protected { } if ($enable_cats) { - $show_empty_cats = clean($_REQUEST['force_show_empty'] ?? false) || + $show_empty_cats = self::_param_to_bool($_REQUEST['force_show_empty'] ?? null) || (clean($_REQUEST['mode'] ?? 0) != 2 && !$search); $feed_categories = ORM::for_table('ttrss_feed_categories') @@ -1260,7 +1260,7 @@ class Pref_Feeds extends Handler_Protected { function regenFeedKey(): void { $feed_id = clean($_REQUEST['id']); - $is_cat = clean($_REQUEST['is_cat']); + $is_cat = self::_param_to_bool($_REQUEST['is_cat'] ?? null); $new_key = Feeds::_update_access_key($feed_id, $is_cat, $_SESSION["uid"]); @@ -1269,7 +1269,7 @@ class Pref_Feeds extends Handler_Protected { function getSharedURL(): void { $feed_id = clean($_REQUEST['id']); - $is_cat = clean($_REQUEST['is_cat']) == "true"; + $is_cat = self::_param_to_bool($_REQUEST['is_cat'] ?? null); $search = clean($_REQUEST['search']); $link = Config::get_self_url() . "/public.php?" . http_build_query([ diff --git a/classes/rpc.php b/classes/rpc.php index 75d008b8b..584b86b23 100755 --- a/classes/rpc.php +++ b/classes/rpc.php @@ -173,7 +173,7 @@ class RPC extends Handler_Protected { } function sanityCheck(): void { - $_SESSION["hasSandbox"] = clean($_REQUEST["hasSandbox"]) === "true"; + $_SESSION["hasSandbox"] = self::_param_to_bool($_REQUEST["hasSandbox"] ?? null); $_SESSION["clientTzOffset"] = clean($_REQUEST["clientTzOffset"]); $client_location = $_REQUEST["clientLocation"]; @@ -225,7 +225,7 @@ class RPC extends Handler_Protected { function catchupFeed(): void { $feed_id = clean($_REQUEST['feed_id']); - $is_cat = clean($_REQUEST['is_cat']) == "true"; + $is_cat = self::_param_to_bool($_REQUEST['is_cat'] ?? null); $mode = clean($_REQUEST['mode'] ?? ''); $search_query = clean($_REQUEST['search_query']); $search_lang = clean($_REQUEST['search_lang']); From 16a7208893aa395de2178e1d308b2acec1a50edd Mon Sep 17 00:00:00 2001 From: wn_ Date: Thu, 18 Nov 2021 18:16:50 +0000 Subject: [PATCH 2/5] Clean string params in Handler._param_to_bool() --- classes/handler.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/classes/handler.php b/classes/handler.php index aca5bf4d2..15f9a1bd8 100644 --- a/classes/handler.php +++ b/classes/handler.php @@ -31,6 +31,9 @@ class Handler implements IHandler { * @param mixed $p */ protected static function _param_to_bool($p): bool { + if (is_string($p)) { + $p = clean($p); + } return $p && ($p !== "f" && $p !== "false"); } } From cd712926105f18e906132ded25a86dec676ed9cc Mon Sep 17 00:00:00 2001 From: wn_ Date: Thu, 18 Nov 2021 18:18:49 +0000 Subject: [PATCH 3/5] Actually, always clean in Handler._param_to_bool() --- classes/handler.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/classes/handler.php b/classes/handler.php index 15f9a1bd8..806c9cfbe 100644 --- a/classes/handler.php +++ b/classes/handler.php @@ -31,9 +31,7 @@ class Handler implements IHandler { * @param mixed $p */ protected static function _param_to_bool($p): bool { - if (is_string($p)) { - $p = clean($p); - } + $p = clean($p); return $p && ($p !== "f" && $p !== "false"); } } From d532eb773bc460fe01a10e980fecbb1fdc53497c Mon Sep 17 00:00:00 2001 From: wn_ Date: Thu, 18 Nov 2021 18:23:44 +0000 Subject: [PATCH 4/5] Switch from null to false as the default for missing bool params. --- classes/api.php | 32 ++++++++++++++++---------------- classes/feeds.php | 2 +- classes/handler/public.php | 2 +- classes/pref/feeds.php | 8 ++++---- classes/rpc.php | 4 ++-- 5 files changed, 24 insertions(+), 24 deletions(-) diff --git a/classes/api.php b/classes/api.php index f911cd591..c6a6a69a4 100755 --- a/classes/api.php +++ b/classes/api.php @@ -103,7 +103,7 @@ class API extends Handler { function getUnread(): bool { $feed_id = clean($_REQUEST["feed_id"] ?? ""); - $is_cat = self::_param_to_bool($_REQUEST["is_cat"] ?? null); + $is_cat = self::_param_to_bool($_REQUEST["is_cat"] ?? false); if ($feed_id) { return $this->_wrap(self::STATUS_OK, array("unread" => getFeedUnread($feed_id, $is_cat))); @@ -119,10 +119,10 @@ class API extends Handler { function getFeeds(): bool { $cat_id = (int) clean($_REQUEST["cat_id"]); - $unread_only = self::_param_to_bool($_REQUEST["unread_only"] ?? null); + $unread_only = self::_param_to_bool($_REQUEST["unread_only"] ?? false); $limit = (int) clean($_REQUEST["limit"] ?? 0); $offset = (int) clean($_REQUEST["offset"] ?? 0); - $include_nested = self::_param_to_bool($_REQUEST["include_nested"] ?? null); + $include_nested = self::_param_to_bool($_REQUEST["include_nested"] ?? false); $feeds = $this->_api_get_feeds($cat_id, $unread_only, $limit, $offset, $include_nested); @@ -130,9 +130,9 @@ class API extends Handler { } function getCategories(): bool { - $unread_only = self::_param_to_bool($_REQUEST["unread_only"] ?? null); - $enable_nested = self::_param_to_bool($_REQUEST["enable_nested"] ?? null); - $include_empty = self::_param_to_bool($_REQUEST["include_empty"] ?? null); + $unread_only = self::_param_to_bool($_REQUEST["unread_only"] ?? false); + $enable_nested = self::_param_to_bool($_REQUEST["enable_nested"] ?? false); + $include_empty = self::_param_to_bool($_REQUEST["include_empty"] ?? false); // TODO do not return empty categories, return Uncategorized and standard virtual cats @@ -197,20 +197,20 @@ class API extends Handler { $offset = (int)clean($_REQUEST["skip"] ?? 0); $filter = clean($_REQUEST["filter"] ?? ""); - $is_cat = self::_param_to_bool($_REQUEST["is_cat"] ?? null); - $show_excerpt = self::_param_to_bool($_REQUEST["show_excerpt"] ?? null); - $show_content = self::_param_to_bool($_REQUEST["show_content"] ?? null); + $is_cat = self::_param_to_bool($_REQUEST["is_cat"] ?? false); + $show_excerpt = self::_param_to_bool($_REQUEST["show_excerpt"] ?? false); + $show_content = self::_param_to_bool($_REQUEST["show_content"] ?? false); /* all_articles, unread, adaptive, marked, updated */ - $view_mode = clean($_REQUEST["view_mode"] ?? null); - $include_attachments = self::_param_to_bool($_REQUEST["include_attachments"] ?? null); + $view_mode = clean($_REQUEST["view_mode"] ?? false); + $include_attachments = self::_param_to_bool($_REQUEST["include_attachments"] ?? false); $since_id = (int)clean($_REQUEST["since_id"] ?? 0); - $include_nested = self::_param_to_bool($_REQUEST["include_nested"] ?? null); + $include_nested = self::_param_to_bool($_REQUEST["include_nested"] ?? false); $sanitize_content = self::_param_to_bool($_REQUEST["sanitize"] ?? true); - $force_update = self::_param_to_bool($_REQUEST["force_update"] ?? null); - $has_sandbox = self::_param_to_bool($_REQUEST["has_sandbox"] ?? null); + $force_update = self::_param_to_bool($_REQUEST["force_update"] ?? false); + $has_sandbox = self::_param_to_bool($_REQUEST["has_sandbox"] ?? false); $excerpt_length = (int)clean($_REQUEST["excerpt_length"] ?? 0); $check_first_id = (int)clean($_REQUEST["check_first_id"] ?? 0); - $include_header = self::_param_to_bool($_REQUEST["include_header"] ?? null); + $include_header = self::_param_to_bool($_REQUEST["include_header"] ?? false); $_SESSION['hasSandbox'] = $has_sandbox; @@ -409,7 +409,7 @@ class API extends Handler { function catchupFeed(): bool { $feed_id = clean($_REQUEST["feed_id"]); - $is_cat = self::_param_to_bool($_REQUEST["is_cat"] ?? null); + $is_cat = self::_param_to_bool($_REQUEST["is_cat"] ?? false); $mode = clean($_REQUEST["mode"] ?? ""); if (!in_array($mode, ["all", "1day", "1week", "2week"])) diff --git a/classes/feeds.php b/classes/feeds.php index f71e4b3e8..3fc4162ad 100755 --- a/classes/feeds.php +++ b/classes/feeds.php @@ -456,7 +456,7 @@ class Feeds extends Handler_Protected { $method = $_REQUEST["m"] ?? ""; $view_mode = $_REQUEST["view_mode"] ?? ""; $limit = 30; - $cat_view = self::_param_to_bool($_REQUEST["cat"] ?? null); + $cat_view = self::_param_to_bool($_REQUEST["cat"] ?? false); $next_unread_feed = $_REQUEST["nuf"] ?? 0; $offset = (int) ($_REQUEST["skip"] ?? 0); $order_by = $_REQUEST["order_by"] ?? ""; diff --git a/classes/handler/public.php b/classes/handler/public.php index 6d8f0bd8f..d0776f03c 100755 --- a/classes/handler/public.php +++ b/classes/handler/public.php @@ -307,7 +307,7 @@ class Handler_Public extends Handler { function rss(): void { $feed = clean($_REQUEST["id"]); $key = clean($_REQUEST["key"]); - $is_cat = self::_param_to_bool($_REQUEST["is_cat"] ?? null); + $is_cat = self::_param_to_bool($_REQUEST["is_cat"] ?? false); $limit = (int)clean($_REQUEST["limit"] ?? 0); $offset = (int)clean($_REQUEST["offset"] ?? 0); diff --git a/classes/pref/feeds.php b/classes/pref/feeds.php index 00fd140fe..d2a30662b 100755 --- a/classes/pref/feeds.php +++ b/classes/pref/feeds.php @@ -47,7 +47,7 @@ class Pref_Feeds extends Handler_Protected { $search = ""; // first one is set by API - $show_empty_cats = self::_param_to_bool($_REQUEST['force_show_empty'] ?? null) || + $show_empty_cats = self::_param_to_bool($_REQUEST['force_show_empty'] ?? false) || (clean($_REQUEST['mode'] ?? 0) != 2 && !$search); $items = []; @@ -208,7 +208,7 @@ class Pref_Feeds extends Handler_Protected { } if ($enable_cats) { - $show_empty_cats = self::_param_to_bool($_REQUEST['force_show_empty'] ?? null) || + $show_empty_cats = self::_param_to_bool($_REQUEST['force_show_empty'] ?? false) || (clean($_REQUEST['mode'] ?? 0) != 2 && !$search); $feed_categories = ORM::for_table('ttrss_feed_categories') @@ -1260,7 +1260,7 @@ class Pref_Feeds extends Handler_Protected { function regenFeedKey(): void { $feed_id = clean($_REQUEST['id']); - $is_cat = self::_param_to_bool($_REQUEST['is_cat'] ?? null); + $is_cat = self::_param_to_bool($_REQUEST['is_cat'] ?? false); $new_key = Feeds::_update_access_key($feed_id, $is_cat, $_SESSION["uid"]); @@ -1269,7 +1269,7 @@ class Pref_Feeds extends Handler_Protected { function getSharedURL(): void { $feed_id = clean($_REQUEST['id']); - $is_cat = self::_param_to_bool($_REQUEST['is_cat'] ?? null); + $is_cat = self::_param_to_bool($_REQUEST['is_cat'] ?? false); $search = clean($_REQUEST['search']); $link = Config::get_self_url() . "/public.php?" . http_build_query([ diff --git a/classes/rpc.php b/classes/rpc.php index 584b86b23..4f6a2fe1d 100755 --- a/classes/rpc.php +++ b/classes/rpc.php @@ -173,7 +173,7 @@ class RPC extends Handler_Protected { } function sanityCheck(): void { - $_SESSION["hasSandbox"] = self::_param_to_bool($_REQUEST["hasSandbox"] ?? null); + $_SESSION["hasSandbox"] = self::_param_to_bool($_REQUEST["hasSandbox"] ?? false); $_SESSION["clientTzOffset"] = clean($_REQUEST["clientTzOffset"]); $client_location = $_REQUEST["clientLocation"]; @@ -225,7 +225,7 @@ class RPC extends Handler_Protected { function catchupFeed(): void { $feed_id = clean($_REQUEST['feed_id']); - $is_cat = self::_param_to_bool($_REQUEST['is_cat'] ?? null); + $is_cat = self::_param_to_bool($_REQUEST['is_cat'] ?? false); $mode = clean($_REQUEST['mode'] ?? ''); $search_query = clean($_REQUEST['search_query']); $search_lang = clean($_REQUEST['search_lang']); From 4a891b20f06e04b015d4da9b755d444b915f4b7d Mon Sep 17 00:00:00 2001 From: wn_ Date: Thu, 18 Nov 2021 21:31:52 +0000 Subject: [PATCH 5/5] Fix 'view_mode' default in API#getHeadlines() --- classes/api.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/classes/api.php b/classes/api.php index c6a6a69a4..fcc2bdfd9 100755 --- a/classes/api.php +++ b/classes/api.php @@ -201,7 +201,7 @@ class API extends Handler { $show_excerpt = self::_param_to_bool($_REQUEST["show_excerpt"] ?? false); $show_content = self::_param_to_bool($_REQUEST["show_content"] ?? false); /* all_articles, unread, adaptive, marked, updated */ - $view_mode = clean($_REQUEST["view_mode"] ?? false); + $view_mode = clean($_REQUEST["view_mode"] ?? null); $include_attachments = self::_param_to_bool($_REQUEST["include_attachments"] ?? false); $since_id = (int)clean($_REQUEST["since_id"] ?? 0); $include_nested = self::_param_to_bool($_REQUEST["include_nested"] ?? false);