Fix domain-wide Retry-After (#9390)

* Fix domain-wide Retry-After

- Fix the domain-wide HTTP `Retry-After` for public servers, which has not applied since #8195 (FreshRSS 1.28.0).
  - `getRetryAfterFile()` passed the bare host name to `Minz_Request::serverIsPublic()`. That function expects a URL, so it always returned `false`.
  - So a 429 or 503 paused only the URL that got it, despite the log saying "For that domain, will first retry after …", and FreshRSS went on requesting the other feeds of that server. With several Reddit feeds, for instance, each one got its own 429 in turn.
  - `serverIsPublic()` now gets the URL. Its answer is kept per host for the rest of the request, because it resolves the host name (`dns_get_record()`), and a refresh may load many feeds of the same host.
- Fix the per-URL `Retry-After` of servers on a local network (#8195), which never applied to feeds whose URL, as stored, differs from the URL as requested.
  - Stored URLs are HTML-encoded, e.g. `&` in an RSS-Bridge URL, and may end with `#force_feed`.
  - `FreshRSS_Feed::load()` checked the stored URL, while the pause is stored for the URL as requested. It now checks the requested URL, as `loadJson()` and `loadHtmlXpath()` already do.

* Minor final

---------

Co-authored-by: Alexandre Alapetite <alexandre@alapetite.fr>

app/Models/Feed.php

@@ -605,15 +605,19 @@ class FreshRSS_Feed extends Minz_Model {
 					Minz_Exception::ERROR
 				);
 			} else {
-				if (($retryAfter = FreshRSS_http_Util::getRetryAfter($this->url, $this->proxyParam())) > 0) {
+				$url = htmlspecialchars_decode($this->url, ENT_QUOTES);
+				$forceFeed = str_ends_with($url, '#force_feed');
+				if ($forceFeed) {
+					$url = substr($url, 0, -11);
+				}
+				// Retry-After is stored for the URL as requested, not as stored for the feed
+				if (($retryAfter = FreshRSS_http_Util::getRetryAfter($url, $this->proxyParam())) > 0) {
 					throw new FreshRSS_Feed_Exception('For that domain, will first retry after ' . date('c', $retryAfter) .
 						'. ' . $this->url(includeCredentials: false), code: 503);
 				}
 				$simplePie = new FreshRSS_SimplePieCustom($this->attributes(), $this->curlOptions());
-				$url = htmlspecialchars_decode($this->url, ENT_QUOTES);
-				if (str_ends_with($url, '#force_feed')) {
+				if ($forceFeed) {
 					$simplePie->force_feed(true);
-					$url = substr($url, 0, -11);
 				}
 				$simplePie->set_feed_url($url);
 				if (!$loadDetails) {	//Only activates auto-discovery when adding a new feed

app/Utils/httpUtil.php

@@ -22,13 +22,16 @@ final class FreshRSS_http_Util {
 	];
 	/** @var array<string, string[]> $resolve_ok */
 	private static array $resolve_ok = [];
+	/** @var array<string, bool> $retry_after_domain_wide */
+	private static array $retry_after_domain_wide = [];
 
 	private static function getRetryAfterFile(string $url, string $proxy): string {
 		$domain = parse_url($url, PHP_URL_HOST);
 		if (!is_string($domain) || $domain === '') {
 			return '';
 		}
-		$domainWide = Minz_Request::serverIsPublic($domain);
+		// Once per host, as serverIsPublic() may resolve it
+		$domainWide = self::$retry_after_domain_wide[$domain] ??= Minz_Request::serverIsPublic($url);
 		$port = parse_url($url, PHP_URL_PORT);
 		if (is_int($port)) {
 			$domain .= ':' . $port;

tests/app/Models/FeedTest.php

@@ -0,0 +1,32 @@
+<?php
+declare(strict_types=1);
+
+use PHPUnit\Framework\Attributes\DataProvider;
+
+final class FeedTest extends \PHPUnit\Framework\TestCase {
+
+	#[DataProvider('provideStoredAndRequestedUrls')]
+	public function test_load_obeysRetryAfter(string $storedUrl, string $requestedUrl): void {
+		$getRetryAfterFile = new ReflectionMethod(FreshRSS_http_Util::class, 'getRetryAfterFile');
+		$file = $getRetryAfterFile->invoke(null, $requestedUrl, '');
+		self::assertIsString($file);
+		self::assertTrue(touch($file, time() + 600));
+		try {
+			(new FreshRSS_Feed($storedUrl, validate: false))->load();
+			self::fail('The feed was loaded during its Retry-After');
+		} catch (FreshRSS_Feed_Exception $e) {
+			self::assertStringContainsString('will first retry after', $e->getMessage());
+		} finally {
+			unlink($file);
+		}
+	}
+
+	/** @return array<string,array{string,string}> */
+	public static function provideStoredAndRequestedUrls(): array {
+		// On a local network, where Retry-After is per URL
+		return [
+			'HTML-encoded' => ['http://192.168.1.2/?action=display&amp;bridge=Example', 'http://192.168.1.2/?action=display&bridge=Example'],
+			'force_feed' => ['http://192.168.1.2/feed#force_feed', 'http://192.168.1.2/feed'],
+		];
+	}
+}

tests/app/Utils/httpUtilTest.php

@@ -13,6 +13,25 @@ class httpUtilTest extends \PHPUnit\Framework\TestCase {
 		self::assertEquals($expected, FreshRSS_http_Util::compareUrlIgnoringHttps($url1, $url2) === 0);
 	}
 
+	#[DataProvider('provideUrlsForRetryAfter')]
+	public function test_getRetryAfterFile(string $url1, string $url2, bool $sameFile): void {
+		$getRetryAfterFile = new ReflectionMethod(FreshRSS_http_Util::class, 'getRetryAfterFile');
+		self::assertSame($sameFile, $getRetryAfterFile->invoke(null, $url1, '') === $getRetryAfterFile->invoke(null, $url2, ''));
+	}
+
+	/** @return array<string,array{string,string,bool}> */
+	public static function provideUrlsForRetryAfter(): array {
+		return [
+			// A public server waits as a whole, per port
+			'public server' => ['https://198.51.100.7/feed1', 'https://198.51.100.7/feed2?a=1&b=2', true],
+			'public server, other port' => ['https://198.51.100.7/feed', 'https://198.51.100.7:8443/feed', false],
+			// A server on a local network waits URL by URL
+			'local IP address' => ['http://192.168.1.2/feed1', 'http://192.168.1.2/feed2', false],
+			'local domain' => ['http://rss-bridge.lan/?bridge=A', 'http://rss-bridge.lan/?bridge=B', false],
+			'local, same URL' => ['http://192.168.1.2/feed', 'http://192.168.1.2/feed', true],
+		];
+	}
+
 	#[DataProvider('provideCidrRanges')]
 	public function test_checkCIDR(string $ip, string $range, bool $expected): void {
 		$checkCIDR = new ReflectionMethod(FreshRSS_http_Util::class, 'checkCIDR');