From 7b55001eeeb326323d020dcfac2f864f8c3ad633 Mon Sep 17 00:00:00 2001 From: Andrew Dolgov Date: Wed, 26 Apr 2017 15:29:22 +0300 Subject: [PATCH] fix various issues reported by static analysis update gitlab-ci config --- .gitlab-ci.yml | 6 +- classes/api.php | 10 ++- classes/article.php | 2 +- classes/auth/base.php | 6 ++ classes/db/prefs.php | 3 - classes/feeds.php | 14 ++-- classes/logger/syslog.php | 3 + classes/opml.php | 23 ++---- classes/plugin.php | 3 + classes/pluginhost.php | 2 +- classes/pref/filters.php | 2 +- classes/pref/users.php | 1 - classes/rpc.php | 2 +- include/errorhandler.php | 3 - include/functions.php | 5 +- include/functions2.php | 9 +- include/rssfuncs.php | 167 ++++++++++++++++---------------------- include/sanity_check.php | 6 +- include/sessions.php | 11 ++- update.php | 2 +- 20 files changed, 131 insertions(+), 149 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 4f146016..d19f7b85 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -11,6 +11,6 @@ jobs: when: manual stage: test script: - - phpmd include text utils/gitlab-ci/phpmd-ruleset.xml - - phpmd classes text utils/gitlab-ci/phpmd-ruleset.xml - + - phpmd include text utils/gitlab-ci/phpmd-ruleset.xml + - phpmd classes text utils/gitlab-ci/phpmd-ruleset.xml + - FILES=$(ls -dm *.php | sed "s/ //g") && phpmd $FILES text utils/gitlab-ci/phpmd-ruleset.xml diff --git a/classes/api.php b/classes/api.php index c43a78eb..895ea526 100644 --- a/classes/api.php +++ b/classes/api.php @@ -1,5 +1,4 @@ publish_update($rss_link); + $p->publish_update($rss_link); } } @@ -418,7 +417,7 @@ class API extends Handler { $feed_id = (int) $this->dbh->escape_string($_REQUEST["feed_id"]); if (!ini_get("open_basedir")) { - update_rss_feed($feed_id, true); + update_rss_feed($feed_id); } $this->wrap(self::STATUS_OK, array("status" => "OK")); @@ -658,6 +657,9 @@ class API extends Handler { return $feeds; } + /** + * @SuppressWarnings(PHPMD.UnusedFormalParameter) + */ static function api_get_headlines($feed_id, $limit, $offset, $filter, $is_cat, $show_excerpt, $show_content, $view_mode, $order, $include_attachments, $since_id, @@ -677,7 +679,7 @@ class API extends Handler { if (!$cache_images && time() - $last_updated > 120) { include "rssfuncs.php"; - update_rss_feed($feed_id, true, true); + update_rss_feed($feed_id, true); } else { db_query("UPDATE ttrss_feeds SET last_updated = '1970-01-01', last_update_started = '1970-01-01' WHERE id = '$feed_id'"); diff --git a/classes/article.php b/classes/article.php index 4126843a..2e7a05e0 100644 --- a/classes/article.php +++ b/classes/article.php @@ -361,7 +361,7 @@ class Article extends Handler_Protected { $labels = get_article_labels($id, $_SESSION["uid"]); array_push($reply["info-for-headlines"], - array("id" => $id, "labels" => format_article_labels($labels, $id))); + array("id" => $id, "labels" => format_article_labels($labels))); } } diff --git a/classes/auth/base.php b/classes/auth/base.php index 69acd098..26b6e4ab 100644 --- a/classes/auth/base.php +++ b/classes/auth/base.php @@ -6,10 +6,16 @@ class Auth_Base { $this->dbh = Db::get(); } + /** + * @SuppressWarnings(unused) + */ function check_password($owner_uid, $password) { return false; } + /** + * @SuppressWarnings(unused) + */ function authenticate($login, $password) { return false; } diff --git a/classes/db/prefs.php b/classes/db/prefs.php index 3e92bb89..0d4ebc2d 100644 --- a/classes/db/prefs.php +++ b/classes/db/prefs.php @@ -23,8 +23,6 @@ class Db_Prefs { } function cache() { - $profile = false; - $user_id = $_SESSION["uid"]; @$profile = $_SESSION["profile"]; @@ -129,7 +127,6 @@ class Db_Prefs { @$profile = $_SESSION["profile"]; } else { $user_id = sprintf("%d", $user_id); - $prefs_cache = false; } if ($profile) { diff --git a/classes/feeds.php b/classes/feeds.php index 48c4ad19..4c4972d7 100755 --- a/classes/feeds.php +++ b/classes/feeds.php @@ -13,7 +13,7 @@ class Feeds extends Handler_Protected { private function format_headline_subtoolbar($feed_site_url, $feed_title, $feed_id, $is_cat, $search, - $view_mode, $error, $feed_last_updated) { + $error, $feed_last_updated) { $catchup_sel_link = "catchupSelection()"; @@ -153,7 +153,7 @@ class Feeds extends Handler_Protected { } private function format_headlines_list($feed, $method, $view_mode, $limit, $cat_view, - $next_unread_feed, $offset, $vgr_last_feed = false, + $offset, $vgr_last_feed = false, $override_order = false, $include_children = false, $check_first_id = false, $skip_first_id_check = false) { @@ -203,7 +203,7 @@ class Feeds extends Handler_Protected { if (!$cache_images && time() - $last_updated > 120) { include "rssfuncs.php"; - update_rss_feed($feed, true, true); + update_rss_feed($feed, true); } else { $this->dbh->query("UPDATE ttrss_feeds SET last_updated = '1970-01-01', last_update_started = '1970-01-01' WHERE id = '$feed'"); @@ -306,7 +306,7 @@ class Feeds extends Handler_Protected { $reply['toolbar'] = $this->format_headline_subtoolbar($feed_site_url, $feed_title, - $feed, $cat_view, $search, $view_mode, + $feed, $cat_view, $search, $last_error, $last_updated); $headlines_count = is_numeric($result) ? 0 : $this->dbh->num_rows($result); @@ -361,7 +361,7 @@ class Feeds extends Handler_Protected { if (!is_array($labels)) $labels = get_article_labels($id); $labels_str = ""; - $labels_str .= format_article_labels($labels, $id); + $labels_str .= format_article_labels($labels); $labels_str .= ""; if (count($topmost_article_ids) < 3) { @@ -937,7 +937,7 @@ class Feeds extends Handler_Protected { if ($_REQUEST["debug"]) $timing_info = print_checkpoint("04", $timing_info); $ret = $this->format_headlines_list($feed, $method, - $view_mode, $limit, $cat_view, $next_unread_feed, $offset, + $view_mode, $limit, $cat_view, $offset, $vgroup_last_feed, $override_order, true, $check_first_id, $skip_first_id_check); //$topmost_article_ids = $ret[0]; @@ -1231,7 +1231,7 @@ class Feeds extends Handler_Protected { if ($do_update) { include "rssfuncs.php"; - update_rss_feed($feed_id, true, true); + update_rss_feed($feed_id, true); } ?> diff --git a/classes/logger/syslog.php b/classes/logger/syslog.php index b8b5260a..04416060 100644 --- a/classes/logger/syslog.php +++ b/classes/logger/syslog.php @@ -1,6 +1,9 @@ attributes; $feed_title = $this->dbh->escape_string(mb_substr($attrs->getNamedItem('text')->nodeValue, 0, 250)); @@ -284,7 +284,7 @@ class Opml extends Handler_Protected { } } - private function opml_import_label($doc, $node, $owner_uid) { + private function opml_import_label($node, $owner_uid) { $attrs = $node->attributes; $label_name = $this->dbh->escape_string($attrs->getNamedItem('label-name')->nodeValue); @@ -301,7 +301,7 @@ class Opml extends Handler_Protected { } } - private function opml_import_preference($doc, $node, $owner_uid) { + private function opml_import_preference($node) { $attrs = $node->attributes; $pref_name = $this->dbh->escape_string($attrs->getNamedItem('pref-name')->nodeValue); @@ -315,7 +315,7 @@ class Opml extends Handler_Protected { } } - private function opml_import_filter($doc, $node, $owner_uid) { + private function opml_import_filter($node) { $attrs = $node->attributes; $filter_type = $this->dbh->escape_string($attrs->getNamedItem('filter-type')->nodeValue); @@ -386,8 +386,6 @@ class Opml extends Handler_Protected { } private function opml_import_category($doc, $root_node, $owner_uid, $parent_id) { - $body = $doc->getElementsByTagName('body'); - $default_cat_id = (int) get_feed_category('Imported feeds', false); if ($root_node) { @@ -442,16 +440,16 @@ class Opml extends Handler_Protected { switch ($cat_title) { case "tt-rss-prefs": - $this->opml_import_preference($doc, $node, $owner_uid); + $this->opml_import_preference($node); break; case "tt-rss-labels": - $this->opml_import_label($doc, $node, $owner_uid); + $this->opml_import_label($node, $owner_uid); break; case "tt-rss-filters": - $this->opml_import_filter($doc, $node, $owner_uid); + $this->opml_import_filter($node); break; default: - $this->opml_import_feed($doc, $node, $dst_cat_id, $owner_uid); + $this->opml_import_feed($node, $dst_cat_id, $owner_uid); } } } @@ -461,19 +459,14 @@ class Opml extends Handler_Protected { function opml_import($owner_uid) { if (!$owner_uid) return; - $debug = isset($_REQUEST["debug"]); $doc = false; -# if ($debug) $doc = DOMDocument::load("/tmp/test.opml"); - if ($_FILES['opml_file']['error'] != 0) { print_error(T_sprintf("Upload failed with error code %d", $_FILES['opml_file']['error'])); return; } - $tmp_file = false; - if (is_uploaded_file($_FILES['opml_file']['tmp_name'])) { $tmp_file = tempnam(CACHE_DIR . '/upload', 'opml'); diff --git a/classes/plugin.php b/classes/plugin.php index 09204098..5041de0c 100644 --- a/classes/plugin.php +++ b/classes/plugin.php @@ -22,6 +22,9 @@ class Plugin { return array(); } + /** + * @SuppressWarnings(PHPMD.UnusedFormalParameter) + */ function is_public_method($method) { return false; } diff --git a/classes/pluginhost.php b/classes/pluginhost.php index 3c2d7931..48ef43e0 100644 --- a/classes/pluginhost.php +++ b/classes/pluginhost.php @@ -288,7 +288,7 @@ class PluginHost { } } - function load_data($force = false) { + function load_data() { if ($this->owner_uid) { $result = $this->dbh->query("SELECT name, content FROM ttrss_plugin_storage WHERE owner_uid = '".$this->owner_uid."'"); diff --git a/classes/pref/filters.php b/classes/pref/filters.php index 5629f530..f3836d8e 100755 --- a/classes/pref/filters.php +++ b/classes/pref/filters.php @@ -130,7 +130,7 @@ class Pref_Filters extends Handler_Protected { while ($line = db_fetch_assoc($result)) { $rc = get_article_filters(array($filter), $line['title'], $line['content'], $line['link'], - false, $line['author'], explode(",", $line['tag_cache'])); + $line['author'], explode(",", $line['tag_cache'])); if (count($rc) > 0) { diff --git a/classes/pref/users.php b/classes/pref/users.php index f211a8f2..215f0564 100644 --- a/classes/pref/users.php +++ b/classes/pref/users.php @@ -251,7 +251,6 @@ class Pref_Users extends Handler_Protected { $login = db_fetch_result($result, 0, "login"); $email = db_fetch_result($result, 0, "email"); - $salt = db_fetch_result($result, 0, "salt"); $new_salt = substr(bin2hex(get_random_bytes(125)), 0, 250); $tmp_user_pwd = make_password(8); diff --git a/classes/rpc.php b/classes/rpc.php index ca8ce39a..008ed876 100755 --- a/classes/rpc.php +++ b/classes/rpc.php @@ -453,7 +453,7 @@ class RPC extends Handler_Protected { $search_query = $this->dbh->escape_string($_REQUEST['search_query']); $search_lang = $this->dbh->escape_string($_REQUEST['search_lang']); - catchup_feed($feed_id, $is_cat, false, false, $mode, [$search_query, $search_lang]); + catchup_feed($feed_id, $is_cat, false, $mode, [$search_query, $search_lang]); print json_encode(array("message" => "UPDATE_COUNTERS")); } diff --git a/include/errorhandler.php b/include/errorhandler.php index f757b68d..bb2fbaee 100644 --- a/include/errorhandler.php +++ b/include/errorhandler.php @@ -32,8 +32,6 @@ function format_backtrace($trace) { } function ttrss_error_handler($errno, $errstr, $file, $line, $context) { - global $logger; - if (error_reporting() == 0 || !$errno) return false; $file = substr(str_replace(dirname(dirname(__FILE__)), "", $file), 1); @@ -46,7 +44,6 @@ function ttrss_error_handler($errno, $errstr, $file, $line, $context) { } function ttrss_fatal_handler() { - global $logger; global $last_query; $error = error_get_last(); diff --git a/include/functions.php b/include/functions.php index 52675057..baeaa03a 100644 --- a/include/functions.php +++ b/include/functions.php @@ -1163,7 +1163,7 @@ } } - function catchup_feed($feed, $cat_view, $owner_uid = false, $max_id = false, $mode = 'all', $search = false) { + function catchup_feed($feed, $cat_view, $owner_uid = false, $mode = 'all', $search = false) { if (!$owner_uid) $owner_uid = $_SESSION['uid']; @@ -1746,7 +1746,6 @@ global $fetch_last_error; global $fetch_last_error_content; - global $fetch_last_error_code; require_once "include/rssfuncs.php"; @@ -2066,5 +2065,3 @@ // TODO: less dumb splitting require_once "functions2.php"; - -?> diff --git a/include/functions2.php b/include/functions2.php index 84469365..f9ee8245 100644 --- a/include/functions2.php +++ b/include/functions2.php @@ -1557,7 +1557,7 @@ } } // function encrypt_password - function load_filters($feed_id, $owner_uid, $action_id = false) { + function load_filters($feed_id, $owner_uid) { $filters = array(); $cat_id = (int)getFeedCategory($feed_id); @@ -1655,7 +1655,7 @@ return true; } - function format_tags_string($tags, $id) { + function format_tags_string($tags) { if (!is_array($tags) || count($tags) == 0) { return __("no tags"); } else { @@ -1675,7 +1675,7 @@ } } - function format_article_labels($labels, $id) { + function format_article_labels($labels) { if (!is_array($labels)) return ''; @@ -2484,6 +2484,9 @@ return false; } + /** + * @SuppressWarnings(unused) + */ function error_json($code) { require_once "errors.php"; diff --git a/include/rssfuncs.php b/include/rssfuncs.php index 1eea3df3..645d06f8 100644 --- a/include/rssfuncs.php +++ b/include/rssfuncs.php @@ -60,20 +60,7 @@ } - - /** - * Update a feed batch. - * Used by daemons to update n feeds by run. - * Only update feed needing a update, and not being processed - * by another process. - * - * @param mixed $link Database link - * @param integer $limit Maximum number of feeds in update batch. Default to DAEMON_FEED_LIMIT. - * @param boolean $from_http Set to true if you call this function from http to disable cli specific code. - * @param boolean $debug Set to false to disable debug output. Default to true. - * @return void - */ - function update_daemon_common($limit = DAEMON_FEED_LIMIT, $from_http = false, $debug = true) { + function update_daemon_common($limit = DAEMON_FEED_LIMIT, $debug = true) { // Process all other feeds using last_updated and interval parameters $schema_version = get_schema_version(); @@ -200,8 +187,6 @@ ORDER BY ttrss_feeds.id $query_limit"); if (db_num_rows($tmp_result) > 0) { - $rss = false; - while ($tline = db_fetch_assoc($tmp_result)) { if($debug) _debug(" => " . $tline["last_updated"] . ", " . $tline["id"] . " " . $tline["owner_uid"]); @@ -209,7 +194,7 @@ array_push($batch_owners, $tline["owner_uid"]); $fstarted = microtime(true); - $rss = update_rss_feed($tline["id"], true, false); + update_rss_feed($tline["id"], true, false); _debug_suppress(false); _debug(sprintf(" %.4f (sec)", microtime(true) - $fstarted)); @@ -237,7 +222,7 @@ return $nf; - } // function update_daemon_common + } // this is used when subscribing function set_basic_feed_info($feed) { @@ -300,8 +285,10 @@ } } - // ignore_daemon is not used - function update_rss_feed($feed, $ignore_daemon = false, $no_cache = false, $rss = false) { + /** + * @SuppressWarnings(PHPMD.UnusedFormalParameter) + */ + function update_rss_feed($feed, $no_cache = false) { $debug_enabled = defined('DAEMON_EXTENDED_DEBUG') || $_REQUEST['xdebug']; @@ -370,94 +357,90 @@ $pluginhost->load($user_plugins, PluginHost::KIND_USER, $owner_uid); $pluginhost->load_data(); - if ($rss && is_object($rss) && get_class($rss) == "FeedParser") { - _debug("using previously initialized parser object"); - } else { - $rss_hash = false; + $rss_hash = false; - $force_refetch = isset($_REQUEST["force_refetch"]); - $feed_data = ""; + $force_refetch = isset($_REQUEST["force_refetch"]); + $feed_data = ""; - foreach ($pluginhost->get_hooks(PluginHost::HOOK_FETCH_FEED) as $plugin) { - $feed_data = $plugin->hook_fetch_feed($feed_data, $fetch_url, $owner_uid, $feed, 0, $auth_login, $auth_pass); - } - - // try cache - if (!$feed_data && - file_exists($cache_filename) && - is_readable($cache_filename) && - !$auth_login && !$auth_pass && - filemtime($cache_filename) > time() - 30) { + foreach ($pluginhost->get_hooks(PluginHost::HOOK_FETCH_FEED) as $plugin) { + $feed_data = $plugin->hook_fetch_feed($feed_data, $fetch_url, $owner_uid, $feed, 0, $auth_login, $auth_pass); + } - _debug("using local cache [$cache_filename].", $debug_enabled); + // try cache + if (!$feed_data && + file_exists($cache_filename) && + is_readable($cache_filename) && + !$auth_login && !$auth_pass && + filemtime($cache_filename) > time() - 30) { - @$feed_data = file_get_contents($cache_filename); + _debug("using local cache [$cache_filename].", $debug_enabled); - if ($feed_data) { - $rss_hash = sha1($feed_data); - } + @$feed_data = file_get_contents($cache_filename); - } else { - _debug("local cache will not be used for this feed", $debug_enabled); + if ($feed_data) { + $rss_hash = sha1($feed_data); } - // fetch feed from source - if (!$feed_data) { - _debug("fetching [$fetch_url]...", $debug_enabled); + } else { + _debug("local cache will not be used for this feed", $debug_enabled); + } - if (ini_get("open_basedir") && function_exists("curl_init")) { - _debug("not using CURL due to open_basedir restrictions"); - } + // fetch feed from source + if (!$feed_data) { + _debug("fetching [$fetch_url]...", $debug_enabled); - $feed_data = fetch_file_contents($fetch_url, false, - $auth_login, $auth_pass, false, - $no_cache ? FEED_FETCH_NO_CACHE_TIMEOUT : FEED_FETCH_TIMEOUT, - 0); + if (ini_get("open_basedir") && function_exists("curl_init")) { + _debug("not using CURL due to open_basedir restrictions"); + } - global $fetch_curl_used; + $feed_data = fetch_file_contents($fetch_url, false, + $auth_login, $auth_pass, false, + $no_cache ? FEED_FETCH_NO_CACHE_TIMEOUT : FEED_FETCH_TIMEOUT, + 0); - if (!$fetch_curl_used) { - $tmp = @gzdecode($feed_data); + global $fetch_curl_used; - if ($tmp) $feed_data = $tmp; - } + if (!$fetch_curl_used) { + $tmp = @gzdecode($feed_data); - $feed_data = trim($feed_data); + if ($tmp) $feed_data = $tmp; + } - _debug("fetch done.", $debug_enabled); + $feed_data = trim($feed_data); - // cache vanilla feed data for re-use - if ($feed_data && !$auth_pass && !$auth_login && is_writable(CACHE_DIR . "/simplepie")) { - $new_rss_hash = sha1($feed_data); + _debug("fetch done.", $debug_enabled); - if ($new_rss_hash != $rss_hash) { - _debug("saving $cache_filename", $debug_enabled); - @file_put_contents($cache_filename, $feed_data); - } + // cache vanilla feed data for re-use + if ($feed_data && !$auth_pass && !$auth_login && is_writable(CACHE_DIR . "/simplepie")) { + $new_rss_hash = sha1($feed_data); + + if ($new_rss_hash != $rss_hash) { + _debug("saving $cache_filename", $debug_enabled); + @file_put_contents($cache_filename, $feed_data); } } + } - if (!$feed_data) { - global $fetch_last_error; - global $fetch_last_error_code; + if (!$feed_data) { + global $fetch_last_error; + global $fetch_last_error_code; - _debug("unable to fetch: $fetch_last_error [$fetch_last_error_code]", $debug_enabled); + _debug("unable to fetch: $fetch_last_error [$fetch_last_error_code]", $debug_enabled); - $error_escaped = ''; + $error_escaped = ''; - // If-Modified-Since - if ($fetch_last_error_code != 304) { - $error_escaped = db_escape_string($fetch_last_error); - } else { - _debug("source claims data not modified, nothing to do.", $debug_enabled); - } + // If-Modified-Since + if ($fetch_last_error_code != 304) { + $error_escaped = db_escape_string($fetch_last_error); + } else { + _debug("source claims data not modified, nothing to do.", $debug_enabled); + } - db_query( - "UPDATE ttrss_feeds SET last_error = '$error_escaped', - last_updated = NOW() WHERE id = '$feed'"); + db_query( + "UPDATE ttrss_feeds SET last_error = '$error_escaped', + last_updated = NOW() WHERE id = '$feed'"); - return; - } + return; } foreach ($pluginhost->get_hooks(PluginHost::HOOK_FEED_FETCHED) as $plugin) { @@ -797,7 +780,7 @@ $matched_rules = array(); $article_filters = get_article_filters($filters, $article["title"], - $article["content"], $article["link"], 0, $article["author"], + $article["content"], $article["link"], $article["author"], $article["tags"], $matched_rules); if ($debug_enabled) { @@ -1224,11 +1207,12 @@ last_updated = NOW() WHERE id = '$feed'"); unset($rss); + return; } _debug("done", $debug_enabled); - return $rss; + return true; } function cache_enclosures($enclosures, $site_url, $debug) { @@ -1369,7 +1353,7 @@ return $params; } - function get_article_filters($filters, $title, $content, $link, $timestamp, $author, $tags, &$matched_rules = false) { + function get_article_filters($filters, $title, $content, $link, $author, $tags, &$matched_rules = false) { $matches = array(); foreach ($filters as $filter) { @@ -1505,15 +1489,6 @@ mb_strtolower(strip_tags($title), 'utf-8')); } - /* function verify_feed_xml($feed_data) { - libxml_use_internal_errors(true); - $doc = new DOMDocument(); - $doc->loadXML($feed_data); - $error = libxml_get_last_error(); - libxml_clear_errors(); - return $error; - } */ - function cleanup_counters_cache($debug) { $result = db_query("DELETE FROM ttrss_counters_cache WHERE feed_id > 0 AND @@ -1529,7 +1504,7 @@ ttrss_cat_counters_cache.owner_uid = ttrss_feed_categories.owner_uid) = 0"); $crows = db_affected_rows($result); - _debug("Removed $frows (feeds) $crows (cats) orphaned counter cache entries."); + if ($debug) _debug("Removed $frows (feeds) $crows (cats) orphaned counter cache entries."); } function housekeeping_user($owner_uid) { diff --git a/include/sanity_check.php b/include/sanity_check.php index f1181f88..3b3e281e 100755 --- a/include/sanity_check.php +++ b/include/sanity_check.php @@ -1,6 +1,5 @@ query("DELETE FROM ttrss_sessions WHERE expire < " . time()); diff --git a/update.php b/update.php index 23289f1d..2245ea02 100755 --- a/update.php +++ b/update.php @@ -401,7 +401,7 @@ $_REQUEST['xdebug'] = 1; - $rc = is_object(update_rss_feed($feed)) ? 0 : 1; + $rc = update_rss_feed($feed) != false ? 0 : 1; exit($rc); } -- 2.39.2