diff --git a/README.md b/README.md index c9f9b85..705a4d4 100644 --- a/README.md +++ b/README.md @@ -71,6 +71,7 @@ Deliberately mirrors core's WebSub layout under `PSHB_PATH`: ```text data/rssCloud/resources//!cloud.json subscription state data/rssCloud/resources//.txt one marker per interested user +data/rssCloud/redirects/.json where a URL permanently moved to ``` Subscriptions are instance-wide (the cloud server knows one callback), but resources are per-user, @@ -81,6 +82,10 @@ Feeds additionally carry an `rssCloud` attribute holding the resource URL they a is what makes the polling decision possible *before* a feed is fetched — `FreshRSS_Feed::selfUrl()` is only populated during the SimplePie parse and is not persisted. +Redirect resolutions are cached one file per URL, so that concurrent notifications cannot lose each +other's writes. An answer is trusted for 30 days; a resolution that *failed* — as opposed to one that +found no move — is retried after 6 hours. + Logs go to `data/users/_/log_rsscloud.txt`. ## Hooks used @@ -88,14 +93,46 @@ Logs go to `data/users/_/log_rsscloud.txt`. | Hook | Purpose | | --- | --- | | `ApiMisc` | serve the callback | -| `SimplepieAfterInit` | discover a feed's cloud, subscribe, persist the resource attribute | +| `SimplepieAfterInit` | discover a feed's cloud, subscribe, persist the resource attribute; pin dynamic OPML feed URLs | | `FeedsListBeforeActualize` | renew feed subscriptions (capped per cycle) | | `FeedBeforeActualize` | skip polling a feed with a healthy subscription | | `FreshrssUserMaintenance` | discover and renew dynamic OPML subscriptions | +| `FeedBeforeInsert` | reconcile a redirected feed against one already subscribed | Renewal has to happen in `FeedsListBeforeActualize` rather than at discovery time, because a feed whose polling is skipped never reaches `SimplepieAfterInit` and would otherwise never renew. +### Redirected feeds in dynamic OPML categories + +FreshRSS and OPML disagree about what identifies a feed: + +* `FreshRSS_Feed::load()` treats a feed as the document it resolves to, and rewrites the stored URL + when the feed answers HTTP 301. +* `FreshRSS_Category::refreshDynamicOpml()` treats a feed as the exact `xmlUrl` string in the list, + which does not change. + +One 301 is enough to make them disagree forever. Every later refresh then reads the entry as new and +inserts it again, and mutes the drifted copy for having disappeared from the list. Nothing catches +the collision: `_feed.url` has no unique index, and `FeedDAO::updateFeed()` does not check for one. +Copies accumulate at one per refresh — unbounded here, because rssCloud refreshes on notification +rather than on a timer. + +`FeedBeforeInsert` settles it at the import step, before core does its matching. A feed whose URL is +not already subscribed is resolved to wherever it permanently moved; if *that* is a feed we hold, the +import is addressed to it instead. Core then recognises it as existing, so it is neither inserted nor +muted, and `FeedDAO::addFeedObject()` unmutes it if an earlier refresh muted it. + +Only 301 and 308 are followed. A temporary redirect deliberately says the resource has *not* moved, +so following it would merge two feeds the publisher considers distinct. A feed that moved onto +nothing we hold is left exactly as the list gives it, and core canonicalises it on first fetch as +usual. + +The hook fires on every import path, so this covers refreshes driven by cron or the CLI as well as by +a notification, and keeps a manual subscription from duplicating a feed already held under its +post-redirect URL. It is registered regardless of the two configuration switches: those govern +whether rssCloud *subscribes* to a resource, while these duplicates are created by the dynamic OPML +refresh itself, which runs either way. + ## Known gaps These are deliberate scaffold-level limitations, not oversights: diff --git a/RssCloud/Redirects.php b/RssCloud/Redirects.php new file mode 100644 index 0000000..5fc33b6 --- /dev/null +++ b/RssCloud/Redirects.php @@ -0,0 +1,221 @@ +.json {"url":…,"target":…,"failed":…,"time":…} + * ``` + * + * One file per URL rather than one shared map, so that concurrent notifications cannot lose each + * other's writes. `target` is null for a URL that resolves to itself, which is cached too: the + * common case is a URL that has not moved, and it should not be probed again on every refresh. + * `failed` separates "it did not move" from "we could not tell", so only the latter is retried soon. + */ +final class RssCloud_Redirects { + + /** How long a completed resolution is trusted. Permanent redirects rarely stop being permanent. */ + public const TTL_SECONDS = 30 * 86400; + + /** How long to wait before probing a URL whose resolution failed, e.g. because the host was down. */ + public const TTL_FAILED_SECONDS = 6 * 3600; + + /** Redirect hops to follow before giving up. Matches the default cURL limit core applies. */ + public const MAX_HOPS = 4; + + public function __construct( + private readonly string $basePath, + ) { + } + + /** + * The URL that `$url` permanently moved to, or `$url` itself if it did not move or could not be + * checked. Never throws: a failure to resolve has to leave the caller with the status quo. + * + * @param array $attributes the feed attributes, for `curl_params` and `timeout` + */ + public function resolve(string $url, array $attributes = []): string { + $cached = $this->load($url); + if ($cached !== null) { + return $cached; + } + + $target = $this->follow($url, $attributes); + $this->store($url, $target); + return $target ?? $url; + } + + /** The cached target for `$url`, or null if there is no fresh entry. */ + private function load(string $url): ?string { + $json = @file_get_contents($this->filename($url)); + if (!is_string($json) || $json === '') { + return null; + } + $entry = json_decode($json, true); + if (!is_array($entry)) { + return null; + } + $time = is_numeric($entry['time'] ?? null) ? (int)$entry['time'] : 0; + $target = is_string($entry['target'] ?? null) ? $entry['target'] : null; + // "It did not move" is an answer and is trusted for as long as a move is; "we could not tell" + // is not, and is retried sooner. Both store a missing target, so they are told apart by flag. + $ttl = ($entry['failed'] ?? false) === true ? self::TTL_FAILED_SECONDS : self::TTL_SECONDS; + if ($time < time() - $ttl) { + return null; + } + return $target ?? $url; + } + + private function store(string $url, ?string $target): void { + $directory = $this->basePath . '/redirects'; + if (!@is_dir($directory) && !@mkdir($directory, 0770, true)) { + Minz_Log::error('rssCloud: cannot create ' . $directory, RSSCLOUD_LOG); + return; + } + $entry = [ + 'url' => $url, + 'target' => $target === null || $target === $url ? null : $target, + 'failed' => $target === null, + 'time' => time(), + ]; + @file_put_contents($this->filename($url), json_encode($entry)); + } + + private function filename(string $url): string { + return $this->basePath . '/redirects/' . sha1($url) . '.json'; + } + + /** + * Walk the chain of permanent redirects, or null if any hop could not be checked. + * + * Only 301 and 308 are followed. A temporary redirect deliberately says the resource has *not* + * moved, so following it would merge two feeds that the publisher considers distinct — and it is + * also what core declines to follow when it rewrites a feed URL, via SimplePie's permanent URL. + * + * @param array $attributes + */ + private function follow(string $url, array $attributes): ?string { + $current = $url; + $seen = [$current => true]; + + // One probe more than the hop limit: the URL reached by the last allowed hop still has to be + // checked, or a chain of exactly MAX_HOPS would be reported as unresolvable despite being + // within the limit — which is also how core counts, in `FreshRSS_http_Util::httpGet()`. + for ($hop = 0; $hop <= self::MAX_HOPS; $hop++) { + $location = self::permanentLocation($current, $attributes); + if ($location === false) { + return null; + } + if ($location === null) { + return $current; + } + if ($hop === self::MAX_HOPS) { + break; + } + $absolute = \SimplePie\Misc::absolutize_url($location, $current); + $next = is_string($absolute) ? (FreshRSS_http_Util::checkUrl($absolute, fixScheme: false) ?: '') : ''; + if ($next === '' || isset($seen[$next])) { + // Unusable or looping: stop where we are rather than report a move we cannot trust. + return $current; + } + $seen[$next] = true; + $current = $next; + } + + Minz_Log::warning('rssCloud: too many permanent redirects from ' . + \SimplePie\Misc::url_remove_credentials($url), RSSCLOUD_LOG); + return null; + } + + /** + * Issue one HEAD and report the `Location` of a permanent redirect. + * + * @param array $attributes + * @return string|false|null the location; null if this is not a permanent redirect; false if the + * request could not be made or failed, which is not the same answer and must not be cached as one + */ + private static function permanentLocation(string $url, array $attributes): string|false|null { + if ($url === '') { + return false; + } + + // Re-checked at every hop, so a redirect cannot walk into the private network. + $resolve = FreshRSS_http_Util::getCurlResolveInfo($url); + if (!is_array($resolve)) { + // null: the host's IP is not in the allowlist. false: the host did not resolve. + return false; + } + + $ch = curl_init(); + if ($ch === false) { + return false; + } + + $limits = FreshRSS_Context::systemConf()->limits; + $timeout = is_numeric($attributes['timeout'] ?? null) && (int)$attributes['timeout'] > 0 ? + (int)$attributes['timeout'] : (int)($limits['timeout'] ?? 10); + + curl_setopt_array($ch, [ + CURLOPT_URL => $url, + // Only the status line and Location are wanted, so never ask for a body. + CURLOPT_NOBODY => true, + // Hops are walked by hand above, so that each one is re-checked against the allowlist. + CURLOPT_FOLLOWLOCATION => false, + CURLOPT_RETURNTRANSFER => true, + CURLOPT_USERAGENT => FRESHRSS_USERAGENT, + CURLOPT_CONNECTTIMEOUT => $timeout, + CURLOPT_TIMEOUT => $timeout, + ]); + if ($resolve !== []) { + curl_setopt($ch, CURLOPT_RESOLVE, $resolve); // Prevent DNS rebinding + } + if (defined('CURLOPT_PROTOCOLS_STR') && is_int(CURLOPT_PROTOCOLS_STR)) { + curl_setopt($ch, CURLOPT_PROTOCOLS_STR, 'http,https'); + } elseif (defined('CURLOPT_PROTOCOLS') && defined('CURLPROTO_HTTP') && defined('CURLPROTO_HTTPS')) { + curl_setopt($ch, CURLOPT_PROTOCOLS, CURLPROTO_HTTP | CURLPROTO_HTTPS); + } + + // Instance-wide options carry the proxy configuration, so they must not be skipped. + curl_setopt_array($ch, FreshRSS_Context::systemConf()->curl_options); + if (is_array($attributes['curl_params'] ?? null)) { + curl_setopt_array($ch, FreshRSS_http_Util::sanitizeCurlParams($attributes['curl_params'])); + } + // Reassert what the options above are not allowed to undo. + curl_setopt($ch, CURLOPT_FOLLOWLOCATION, false); + + curl_exec($ch); + $status = (int)curl_getinfo($ch, CURLINFO_HTTP_CODE); + $location = curl_getinfo($ch, CURLINFO_REDIRECT_URL); + $error = curl_error($ch); + + if ($error !== '') { + Minz_Log::debug('rssCloud: cannot check ' . \SimplePie\Misc::url_remove_credentials($url) . + ' for a permanent redirect: ' . $error, RSSCLOUD_LOG); + return false; + } + + if (in_array($status, [301, 308], true)) { + // A permanent redirect with nowhere to go is malformed, and says nothing either way. + return is_string($location) && $location !== '' ? $location : false; + } + + // Transient: the same request may well answer differently later, so this must not be recorded + // as a lasting "did not move" — that would let duplicates resume for the whole cache lifetime. + if ($status === 0 || $status === 408 || $status === 429 || $status >= 500) { + return false; + } + + // Anything else is a stable answer of "this has not permanently moved", including a server + // that rejects HEAD outright: retrying that in six hours would not produce a different answer. + return null; + } +} diff --git a/extension.php b/extension.php index c7af565..612307f 100644 --- a/extension.php +++ b/extension.php @@ -44,6 +44,7 @@ final class RssCloudExtension extends Minz_Extension { private const FEED_ATTRIBUTE = 'rssCloud'; private ?RssCloud_Registry $registry = null; + private ?RssCloud_Redirects $redirects = null; private ?RssCloud_Subscriber $subscriber = null; /** Whether {@see self::subscriber()} has run, so a failure is diagnosed and logged only once. */ @@ -81,6 +82,11 @@ public function init(): void { if ($this->isEnabledForOpml()) { $this->registerHook(Minz_HookType::FreshrssUserMaintenance, [$this, 'onUserMaintenance']); } + + // Deliberately not gated on either switch. Those govern whether rssCloud *subscribes* to a + // resource, whereas the duplicates this guards against are created by the dynamic OPML + // refresh itself — which cron and the CLI perform whatever this extension is set to do. + $this->registerHook(Minz_HookType::FeedBeforeInsert, [$this, 'onFeedBeforeInsert']); } #[\Override] @@ -88,12 +94,15 @@ public function install() { // Note: install() runs *before* Minz_ExtensionManager enables the extension, so the // autoloader registered above is not active yet. Nothing here may touch an RssCloud_* class. $resources = RSSCLOUD_PATH . '/resources'; - if (!@is_dir($resources) && !@mkdir($resources, 0770, true)) { - return 'Cannot create ' . $resources; + $redirects = RSSCLOUD_PATH . '/redirects'; + foreach ([$resources, $redirects] as $directory) { + if (!@is_dir($directory) && !@mkdir($directory, 0770, true)) { + return 'Cannot create ' . $directory; + } } // `data/.htaccess` denies all, but add the same directory-listing guards core ships under // `data/PubSubHubbub/` for servers that do not read .htaccess. - foreach ([RSSCLOUD_PATH, $resources] as $directory) { + foreach ([RSSCLOUD_PATH, $resources, $redirects] as $directory) { if (!file_exists($directory . '/index.html')) { @file_put_contents($directory . '/index.html', ''); } @@ -176,6 +185,10 @@ private function registry(): RssCloud_Registry { return $this->registry ??= new RssCloud_Registry(RSSCLOUD_PATH); } + private function redirects(): RssCloud_Redirects { + return $this->redirects ??= new RssCloud_Redirects(RSSCLOUD_PATH); + } + private function subscriber(): ?RssCloud_Subscriber { if ($this->subscriberResolved) { return $this->subscriber; @@ -325,6 +338,64 @@ public function onUserMaintenance(): void { } } + /** + * Address a feed being imported by the URL it has permanently moved to, when that is a feed we + * already hold. Otherwise leave it exactly as the list gives it. + * + * This is what stops a dynamic OPML category accumulating duplicates. The two sides disagree + * about what identifies a feed: + * + * * `FreshRSS_Feed::load()` rewrites a feed's stored URL to wherever it moved on HTTP 301. + * * `FreshRSS_Category::refreshDynamicOpml()` matches what it holds against the OPML by exact URL. + * + * So one 301 is enough to make every later refresh read the entry as new and insert it again, + * then mute the drifted copy for having vanished from the list. Nothing catches the collision: + * `_feed.url` carries no unique index, and `FeedDAO::updateFeed()` does not check for one. The + * copies therefore accumulate at one per refresh — unbounded under rssCloud, where refreshes + * follow notifications rather than a timer. + * + * Resolving the redirect here settles the disagreement in core's favour, at the import step and + * before core does its matching: the feed is recognised as one we already hold, so it is neither + * inserted nor muted, and `FeedDAO::addFeedObject()` unmutes it if an earlier refresh muted it. + * + * The hook fires on every import path, so this also covers refreshes driven by cron or the CLI + * rather than by a notification, and keeps a manual subscription from duplicating a feed already + * held under its post-redirect URL. + * + * The feed is always returned: the hook can cancel an import by returning null, but a URL this + * extension failed to reconcile is not a reason to drop a subscription the list asked for. + */ + public function onFeedBeforeInsert(FreshRSS_Feed $feed): FreshRSS_Feed { + $url = $feed->url(); + $feedDAO = FreshRSS_Factory::createFeedDao(); + if ($url === '' || $feedDAO->searchByUrl($url) !== null) { + // Already held under this exact URL, so there is nothing to reconcile — and no reason to + // spend a request finding that out. + return $feed; + } + + $target = $this->redirects()->resolve($url, $feed->attributes()); + if ($target === $url) { + return $feed; + } + if ($feedDAO->searchByUrl($target) === null) { + // It moved, but not onto anything we hold. Genuinely new, so let core add it under the + // URL the list gives and rewrite that itself on the first fetch. + return $feed; + } + + try { + $feed->_url($target); + } catch (FreshRSS_BadUrl_Exception $e) { + Minz_Log::warning('rssCloud: ' . $e->getMessage(), RSSCLOUD_LOG); + return $feed; + } + Minz_Log::notice('rssCloud: ' . \SimplePie\Misc::url_remove_credentials($url) . + ' permanently moved to a feed already subscribed as ' . + \SimplePie\Misc::url_remove_credentials($target) . ', reusing it', RSSCLOUD_LOG); + return $feed; + } + // /**