rework favicon storage to use DiskCache

10 files changed, 167 insertions, 74 deletions

diff --git a/classes/cache/adapter.php b/classes/cache/adapter.php
index 514a14685..fecfc7667 100644
--- a/classes/cache/adapter.php
+++ b/classes/cache/adapter.php
@@ -21,6 +21,7 @@ interface Cache_Adapter {
 	public function put(string $filename, $data);
 	public function get(string $filename): ?string;
 	public function get_full_path(string $filename): string;
+	public function remove(string $filename) : bool;
 	/**
 	 * @return false|null|string false if detection failed, null if the file doesn't exist, string mime content type otherwise
 	 */
diff --git a/classes/cache/local.php b/classes/cache/local.php
index fb8aed529..d7d0c5cb1 100644
--- a/classes/cache/local.php
+++ b/classes/cache/local.php
@@ -2,12 +2,18 @@
 class Cache_Local implements Cache_Adapter {
 	private string $dir;
 
+	public function remove(string $filename): bool {
+		return unlink($this->get_full_path($filename));
+	}
+
 	public function get_mtime(string $filename) {
 		return filemtime($this->get_full_path($filename));
 	}
 
 	public function set_dir(string $dir) : void {
 		$this->dir = Config::get(Config::CACHE_DIR) . "/" . basename(clean($dir));
+
+		$this->make_dir();
 	}
 
 	public function get_dir(): string {
@@ -131,9 +137,6 @@ class Cache_Local implements Cache_Adapter {
 			if ($tmppluginhost->run_hooks_until(PluginHost::HOOK_SEND_LOCAL_FILE, true, $filename))
 				return true;
 
-			$stamp = gmdate("D, d M Y H:i:s", (int)filemtime($filename)) . " GMT";
-			header("Last-Modified: $stamp", true);
-
 			return readfile($filename);
 		} else {
 			return false;
diff --git a/classes/diskcache.php b/classes/diskcache.php
index 70c3b7e2c..a00003516 100644
--- a/classes/diskcache.php
+++ b/classes/diskcache.php
@@ -210,6 +210,10 @@ class DiskCache implements Cache_Adapter {
 		$this->adapter->set_dir($dir);
 	}
 
+	public function remove(string $filename): bool {
+		return $this->adapter->remove($filename);
+	}
+
 	public function set_dir(string $dir) : void {
 		$this->adapter->set_dir($dir);
 	}
@@ -290,6 +294,20 @@ class DiskCache implements Cache_Adapter {
 	}
 
 	public function send(string $filename) {
+
+		if (!$this->exists($filename)) {
+			header($_SERVER["SERVER_PROTOCOL"]." 404 Not Found");
+			echo "File not found.";
+			return false;
+		}
+
+		$gmt_modified = gmdate("D, d M Y H:i:s", (int)$this->get_mtime($filename)) . " GMT";
+
+		if (($_SERVER['HTTP_IF_MODIFIED_SINCE'] ?? '') == $gmt_modified) {
+			header('HTTP/1.1 304 Not Modified');
+			return false;
+		}
+
 		$mimetype = $this->adapter->get_mime_type($filename);
 
 		if ($mimetype == "application/octet-stream")
@@ -315,6 +333,15 @@ class DiskCache implements Cache_Adapter {
 		header("Content-Disposition: inline; filename=\"{$filename}{$fake_extension}\"");
 		header("Content-type: $mimetype");
 
+		$stamp_expires = gmdate("D, d M Y H:i:s",
+			(int)$this->get_mtime($filename) + 86400 * Config::get(Config::CACHE_MAX_DAYS)) . " GMT";
+
+		header("Expires: $stamp_expires", true);
+		header("Last-Modified: $gmt_modified", true);
+		header("Cache-Control: public");
+
+		header_remove("Pragma");
+
 		return $this->adapter->send($filename);
 	}
 
diff --git a/classes/feeds.php b/classes/feeds.php
index a063b9ed5..d34a23e4b 100755
--- a/classes/feeds.php
+++ b/classes/feeds.php
@@ -1163,11 +1163,28 @@ class Feeds extends Handler_Protected {
 	}
 
 	static function _get_icon_file(int $feed_id): string {
-		return Config::get(Config::ICONS_DIR) . "/$feed_id.ico";
+		$favicon_cache = new DiskCache('feed-icons');
+
+		return $favicon_cache->get_full_path((string)$feed_id);
 	}
 
-	static function _has_icon(int $id): bool {
-		return is_file(Config::get(Config::ICONS_DIR) . "/$id.ico") && filesize(Config::get(Config::ICONS_DIR) . "/$id.ico") > 0;
+	static function _get_icon_url(int $feed_id, string $fallback_url = "") : string {
+		if (self::_has_icon($feed_id)) {
+			$icon_url = Config::get_self_url() . "/public.php?" . http_build_query([
+				'op' => 'feed_icon',
+				'id' => $feed_id,
+			]);
+
+			return $icon_url;
+		}
+
+		return $fallback_url;
+	}
+
+	static function _has_icon(int $feed_id): bool {
+		$favicon_cache = new DiskCache('feed-icons');
+
+		return $favicon_cache->exists((string)$feed_id);
 	}
 
 	/**
@@ -1191,16 +1208,9 @@ class Feeds extends Handler_Protected {
 				if ($id < LABEL_BASE_INDEX) {
 					return "label";
 				} else {
-					$icon = self::_get_icon_file($id);
-
-					if ($icon && file_exists($icon)) {
-						return Config::get(Config::ICONS_URL) . "/" . basename($icon) . "?" . filemtime($icon);
-					}
+					return self::_get_icon_url($id);
 				}
-				break;
 		}
-
-		return false;
 	}
 
 	/**
diff --git a/classes/handler/public.php b/classes/handler/public.php
index 08b73b87d..484f92507 100755
--- a/classes/handler/public.php
+++ b/classes/handler/public.php
@@ -769,6 +769,18 @@ class Handler_Public extends Handler {
 		}
 	}
 
+	function feed_icon() : void {
+		$id = (int)$_REQUEST['id'];
+		$cache = new DiskCache('feed-icons');
+
+		if ($cache->exists((string)$id)) {
+			$cache->send((string)$id);
+		} else {
+			header($_SERVER["SERVER_PROTOCOL"]." 404 Not Found");
+			echo "File not found.";
+		}
+	}
+
 	private function _make_article_tag_uri(int $id, string $timestamp): string {
 
 		$timestamp = date("Y-m-d", strtotime($timestamp));
diff --git a/classes/pref/feeds.php b/classes/pref/feeds.php
index f2e8e12da..067b8225e 100755
--- a/classes/pref/feeds.php
+++ b/classes/pref/feeds.php
@@ -454,14 +454,15 @@ class Pref_Feeds extends Handler_Protected {
 
 	function removeIcon(): void {
 		$feed_id = (int) $_REQUEST["feed_id"];
-		$icon_file = Config::get(Config::ICONS_DIR) . "/$feed_id.ico";
+
+		$cache = new DiskCache('feed-icons');
 
 		$feed = ORM::for_table('ttrss_feeds')
 			->where('owner_uid', $_SESSION['uid'])
 			->find_one($feed_id);
 
-		if ($feed && file_exists($icon_file)) {
-			if (unlink($icon_file)) {
+		if ($feed && $cache->exists((string)$feed_id)) {
+			if ($cache->remove((string)$feed_id)) {
 				$feed->set([
 					'favicon_avg_color' => null,
 					'favicon_last_checked' => '1970-01-01',
@@ -486,24 +487,25 @@ class Pref_Feeds extends Handler_Protected {
 		if ($feed && $tmp_file && move_uploaded_file($_FILES['icon_file']['tmp_name'], $tmp_file)) {
 			if (filesize($tmp_file) < Config::get(Config::MAX_FAVICON_FILE_SIZE)) {
 
-				$new_filename = Config::get(Config::ICONS_DIR) . "/$feed_id.ico";
-
-				if (file_exists($new_filename)) unlink($new_filename);
-					if (rename($tmp_file, $new_filename)) {
-						chmod($new_filename, 0644);
+				$cache = new DiskCache('feed-icons');
 
-						$feed->set([
-							'favicon_avg_color' => null,
-							'favicon_is_custom' => true,
-						]);
+				if ($cache->put((string)$feed_id, file_get_contents($tmp_file))) {
 
-						if ($feed->save()) {
-							$rc = self::E_ICON_UPLOAD_SUCCESS;
-						}
+					$feed->set([
+						'favicon_avg_color' => null,
+						'favicon_is_custom' => true,
+					]);
 
-					} else {
-						$rc = self::E_ICON_RENAME_FAILED;
+					if ($feed->save()) {
+						$rc = self::E_ICON_UPLOAD_SUCCESS;
 					}
+
+				} else {
+					$rc = self::E_ICON_RENAME_FAILED;
+				}
+
+				@unlink($tmp_file);
+
 			} else {
 				$rc = self::E_ICON_FILE_TOO_LARGE;
 			}
@@ -1186,9 +1188,10 @@ class Pref_Feeds extends Handler_Protected {
 
 			$pdo->commit();
 
-			if (file_exists(Config::get(Config::ICONS_DIR) . "/$id.ico")) {
-				unlink(Config::get(Config::ICONS_DIR) . "/$id.ico");
-			}
+			$favicon_cache = new DiskCache('feed-icons');
+
+			if ($favicon_cache->exists((string)$id))
+				$favicon_cache->remove((string)$id);
 
 		} else {
 			Labels::remove(Labels::feed_to_label_id($id), $owner_uid);
diff --git a/classes/pref/users.php b/classes/pref/users.php
index c48619614..78291592d 100644
--- a/classes/pref/users.php
+++ b/classes/pref/users.php
@@ -82,11 +82,10 @@ class Pref_Users extends Handler_Administrative {
 					<?php while ($row = $sth->fetch()) { ?>
 						<li>
 							<?php
-								$icon_file = Config::get(Config::ICONS_URL) . "/" . $row["id"] . ".ico";
-								$icon = file_exists($icon_file) ? $icon_file : "images/blank_icon.gif";
+								$icon_url = Feeds::_get_icon_url($row['id'], 'images/blank_icon.gif');
 							?>
 
-							<img class="icon" src="<?= $icon_file ?>">
+							<img class="icon" src="<?= htmlspecialchars($icon_url) ?>">
 
 							<a target="_blank" href="<?= htmlspecialchars($row["site_url"]) ?>">
 								<?= htmlspecialchars($row["title"]) ?>
diff --git a/classes/rpc.php b/classes/rpc.php
index ef2cdfc49..a2250157a 100755
--- a/classes/rpc.php
+++ b/classes/rpc.php
@@ -451,7 +451,7 @@ class RPC extends Handler_Protected {
 
 		$params["safe_mode"] = !empty($_SESSION["safe_mode"]);
 		$params["check_for_updates"] = Config::get(Config::CHECK_FOR_UPDATES);
-		$params["icons_url"] = Config::get(Config::ICONS_URL);
+		$params["icons_url"] = Config::get(Config::SELF_URL_PATH) . '/public.php';
 		$params["cookie_lifetime"] = Config::get(Config::SESSION_COOKIE_LIFETIME);
 		$params["default_view_mode"] = get_pref(Prefs::_DEFAULT_VIEW_MODE);
 		$params["default_view_limit"] = (int) get_pref(Prefs::_DEFAULT_VIEW_LIMIT);
diff --git a/classes/rssutils.php b/classes/rssutils.php
index 22859791d..a8c56295b 100755
--- a/classes/rssutils.php
+++ b/classes/rssutils.php
@@ -37,20 +37,29 @@ class RSSUtils {
 		$pdo = Db::pdo();
 		$sth = $pdo->prepare("SELECT id FROM ttrss_feeds WHERE id = ?");
 
-		// check icon files once every Config::get(Config::CACHE_MAX_DAYS) days
-		$icon_files = array_filter(glob(Config::get(Config::ICONS_DIR) . "/*.ico"),
-			fn(string $f) => filemtime($f) < time() - 86400 * Config::get(Config::CACHE_MAX_DAYS));
+		$cache = new DiskCache('feed-icons');
 
-		foreach ($icon_files as $icon) {
-			$feed_id = basename($icon, ".ico");
+		if ($cache->is_writable()) {
+			$dh = opendir($cache->get_full_path(""));
 
-			$sth->execute([$feed_id]);
+			if ($dh) {
+				while (($icon = readdir($dh)) !== false) {
+					if ($cache->get_mtime($icon) < time() - 86400 * Config::get(Config::CACHE_MAX_DAYS)) {
 
-			if ($sth->fetch()) {
-				@touch($icon);
-			} else {
-				Debug::log("Removing orphaned feed icon: $icon");
-				unlink($icon);
+						$sth->execute([(int)$icon]);
+
+						if ($sth->fetch()) {
+							$cache->put($icon, $cache->get($icon));
+						} else {
+							$icon_path = $cache->get_full_path($icon);
+
+							Debug::log("Removing orphaned feed icon: $icon_path");
+							unlink($icon);
+						}
+					}
+				}
+
+				closedir($dh);
 			}
 		}
 	}
@@ -480,10 +489,10 @@ class RSSUtils {
 			}
 
 			// cache vanilla feed data for re-use
-			if ($feed_data && !$feed_obj->auth_pass && !$feed_obj->auth_login && is_writable(Config::get(Config::CACHE_DIR) . "/feeds")) {
+			if ($feed_data && !$feed_obj->auth_pass && !$feed_obj->auth_login && $cache->is_writable()) {
 				$new_rss_hash = sha1($feed_data);
 
-				if ($new_rss_hash != $rss_hash && $cache->is_writable()) {
+				if ($new_rss_hash != $rss_hash) {
 					Debug::log("saving to local cache: $cache_filename", Debug::LOG_VERBOSE);
 					$cache->put($cache_filename, $feed_data);
 				}
@@ -593,21 +602,28 @@ class RSSUtils {
 
 			if ($feed_obj->favicon_needs_check || $force_refetch) {
 
-				/* terrible hack: if we crash on floicon shit here, we won't check
-				 * the icon avgcolor again (unless the icon got updated) */
+				// restrict update attempts to once per 12h
+				$feed_obj->favicon_last_checked = Db::NOW();
+				$feed_obj->save();
 
-				$favicon_file = Config::get(Config::ICONS_DIR) . "/$feed.ico";
-				$favicon_modified = file_exists($favicon_file) ? filemtime($favicon_file) : -1;
+				$favicon_cache = new DiskCache('feed-icons');
 
+				$favicon_modified = $favicon_cache->exists($feed) ? $favicon_cache->get_mtime($feed) : -1;
+
+				// don't try to redownload custom favicons
 				if (!$feed_obj->favicon_is_custom) {
 					Debug::log("favicon: trying to update favicon...", Debug::LOG_VERBOSE);
 					self::update_favicon($feed_obj->site_url, $feed);
 
-					if ((file_exists($favicon_file) ? filemtime($favicon_file) : -1) > $favicon_modified)
+					if (!$favicon_cache->exists($feed) || $favicon_cache->get_mtime($feed) > $favicon_modified) {
 						$feed_obj->favicon_avg_color = null;
+						$feed_obj->save();
+					}
 				}
 
-				if (is_readable($favicon_file) && function_exists("imagecreatefromstring") && empty($feed_obj->favicon_avg_color)) {
+				/* terrible hack: if we crash on floicon shit here, we won't check
+				 * the icon avgcolor again (unless icon got updated) */
+				if (file_exists($favicon_cache->get_full_path($feed)) && function_exists("imagecreatefromstring") && empty($feed_obj->favicon_avg_color)) {
 					require_once "colors.php";
 
 					Debug::log("favicon: trying to calculate average color...", Debug::LOG_VERBOSE);
@@ -615,13 +631,13 @@ class RSSUtils {
 					$feed_obj->favicon_avg_color = 'fail';
 					$feed_obj->save();
 
-					$feed_obj->favicon_avg_color = \Colors\calculate_avg_color($favicon_file);
+					$feed_obj->favicon_avg_color = \Colors\calculate_avg_color($favicon_cache->get_full_path($feed));
 					$feed_obj->save();
 
 					Debug::log("favicon: avg color: {$feed_obj->favicon_avg_color}", Debug::LOG_VERBOSE);
 
 				} else if ($feed_obj->favicon_avg_color == 'fail') {
-					Debug::log("floicon failed $favicon_file, not trying to recalculate avg color", Debug::LOG_VERBOSE);
+					Debug::log("floicon failed on $feed, not trying to recalculate avg color", Debug::LOG_VERBOSE);
 				}
 			}
 
@@ -1672,10 +1688,35 @@ class RSSUtils {
 		$tmph->run_hooks(PluginHost::HOOK_HOUSE_KEEPING);
 	}
 
+	/** migrates favicons from legacy storage in feed-icons/ to cache/feed-icons/using new naming (sans .ico suffix) */
+	static function migrate_feed_icons() : void {
+		$old_dir = Config::get(Config::ICONS_DIR);
+		$new_dir = Config::get(Config::CACHE_DIR) . '/feed-icons';
+
+		$dh = opendir($old_dir);
+
+		if ($dh) {
+			while (($old_filename = readdir($dh)) !== false) {
+				if (strpos($old_filename, ".ico") !== false) {
+					$new_filename = str_replace(".ico", "", $old_filename);
+					$old_full_path = "$old_dir/$old_filename";
+					$new_full_path = "$new_dir/$new_filename";
+
+					if (is_file($old_full_path) && !file_exists($new_full_path)) {
+						rename($old_full_path, $new_full_path);
+					}
+				}
+			}
+
+			closedir($dh);
+		}
+	}
+
 	static function housekeeping_common(): void {
 		$cache = new DiskCache("");
 		$cache->expire_all();
 
+		self::migrate_feed_icons();
 		self::expire_lock_files();
 		self::expire_error_log();
 		self::expire_feed_archive();
@@ -1693,8 +1734,6 @@ class RSSUtils {
 	 * @return false|string
 	 */
 	static function update_favicon(string $site_url, int $feed) {
-		$icon_file = Config::get(Config::ICONS_DIR) . "/$feed.ico";
-
 		$favicon_urls = self::get_favicon_urls($site_url);
 
 		if (count($favicon_urls) == 0) {
@@ -1749,21 +1788,18 @@ class RSSUtils {
 				break;
 			}
 
-			Debug::log("favicon: $favicon_url looks valid, saving to $icon_file", Debug::LOG_VERBOSE);
-
-			$fp = @fopen($icon_file, "w");
-
-			if ($fp) {
+			$favicon_cache = new DiskCache('feed-icons');
 
-				fwrite($fp, $contents);
-				fclose($fp);
-				chmod($icon_file, 0644);
-				clearstatcache();
+			if ($favicon_cache->is_writable()) {
+				Debug::log("favicon: $favicon_url looks valid, saving to cache", Debug::LOG_VERBOSE);
 
-				return $icon_file;
+				// we deal with this manually
+				if (!$favicon_cache->exists(".no-auto-expiry"))
+					$favicon_cache->put(".no-auto-expiry", "");
 
+				return $favicon_cache->put((string)$feed, $contents);
 			} else {
-				Debug::log("favicon: failed to open $icon_file for writing", Debug::LOG_VERBOSE);
+				Debug::log("favicon: $favicon_url skipping, local cache is not writable", Debug::LOG_VERBOSE);
 			}
 		}
 
diff --git a/js/Feeds.js b/js/Feeds.js
index 714eb77d2..b3913b1f0 100644
--- a/js/Feeds.js
+++ b/js/Feeds.js
@@ -100,7 +100,7 @@ const	Feeds = {
 				if (id > 0) {
 					if (has_img) {
 						this.setIcon(id, false,
-							App.getInitParam("icons_url") + "/" + id + ".ico?" + has_img);
+							App.getInitParam("icons_url") + '?' + dojo.objectToQuery({op: 'feed_icon', id: id}));
 					} else {
 						this.setIcon(id, false, 'images/blank_icon.gif');
 					}
@@ -678,8 +678,10 @@ const	Feeds = {
 		});
 	},
 	renderIcon: function(feed_id, exists) {
+		const icon_url = App.getInitParam("icons_url") + '?' + dojo.objectToQuery({op: 'feed_icon', id: feed_id});
+
 		return feed_id && exists ?
-			`<img class="icon" src="${App.escapeHtml(App.getInitParam("icons_url"))}/${feed_id}.ico">` :
+			`<img class="icon" src="${App.escapeHtml(icon_url)}">` :
 				`<i class='icon-no-feed material-icons'>rss_feed</i>`;
 	}
 };