diff --git a/RssCloud/Registry.php b/RssCloud/Registry.php index 1074f05..ff861df 100644 --- a/RssCloud/Registry.php +++ b/RssCloud/Registry.php @@ -16,7 +16,7 @@ * identifies the resource by the `url` parameter of each notification. * * @phpstan-type RssCloudState array{url:string,kind:string,endpoint:string,registerProcedure:string, - * lease_start:int,last_notify:int,error:bool,error_message:string} + * protocol:string,lease_start:int,last_notify:int,error:bool,error_message:string} */ final class RssCloud_Registry { @@ -82,6 +82,13 @@ private static function normalise(array $state): array { 'kind' => ($state['kind'] ?? null) === self::KIND_OPML ? self::KIND_OPML : self::KIND_FEED, 'endpoint' => is_string($state['endpoint'] ?? null) ? $state['endpoint'] : '', 'registerProcedure' => is_string($state['registerProcedure'] ?? null) ? $state['registerProcedure'] : '', + // The `protocol` value this server was last known to accept; empty until one has worked. + // Constrained to the two this extension can speak, because whatever is stored here is + // tried first: an unrecognised one would be advertised to the cloud server ahead of a + // value known to work. + 'protocol' => in_array($state['protocol'] ?? null, + [RssCloud_Endpoint::PROTOCOL_HTTP, RssCloud_Endpoint::PROTOCOL_HTTPS], true) + ? $state['protocol'] : '', 'lease_start' => is_numeric($state['lease_start'] ?? null) ? (int)$state['lease_start'] : 0, 'last_notify' => is_numeric($state['last_notify'] ?? null) ? (int)$state['last_notify'] : 0, // Assume broken until a notification actually arrives, like the core WebSub code does. @@ -112,10 +119,12 @@ public function save(string $resourceUrl, array $state): bool { public function remember(string $resourceUrl, RssCloud_Endpoint $endpoint, string $kind): array { $state = $this->load($resourceUrl) ?? self::normalise(['url' => $resourceUrl]); if ($state['endpoint'] !== $endpoint->url) { - // The publisher moved to a different cloud server: start over. + // The publisher moved to a different cloud server: start over. The new server need not + // accept the same `protocol` value as the old one, so that is forgotten too. $state['lease_start'] = 0; $state['error'] = true; $state['error_message'] = ''; + $state['protocol'] = ''; } $state['kind'] = $kind; $state['endpoint'] = $endpoint->url; diff --git a/RssCloud/Subscriber.php b/RssCloud/Subscriber.php index 9159cc4..ba5069a 100644 --- a/RssCloud/Subscriber.php +++ b/RssCloud/Subscriber.php @@ -72,7 +72,6 @@ public function subscribe(array $state): bool { 'domain' => $this->callback->domain, 'port' => (string)$this->callback->port, 'path' => $this->callback->path, - 'protocol' => $this->callback->protocol, 'registerProcedure' => $state['registerProcedure'], 'url1' => $state['url'], ]; @@ -81,27 +80,72 @@ public function subscribe(array $state): bool { $state['lease_start'] = time(); $this->registry->save($state['url'], $state); - $response = FreshRSS_http_Util::httpGet($endpoint->url, null, 'xml', [], [ - CURLOPT_POSTFIELDS => http_build_query($parameters), - CURLOPT_MAXREDIRS => 10, - ]); + $candidates = self::protocolCandidates($state['protocol'], $this->callback->protocol); + $message = ''; + $status = 0; + + foreach ($candidates as $i => $protocol) { + $response = FreshRSS_http_Util::httpGet($endpoint->url, null, 'xml', [], [ + CURLOPT_POSTFIELDS => http_build_query($parameters + ['protocol' => $protocol]), + CURLOPT_MAXREDIRS => 10, + ]); + $status = (int)$response['status']; + [$success, $message] = self::parseNotifyResult((string)$response['body'], $status); + + $log = 'rssCloud pleaseNotify ' . $state['url'] . ' via ' . $endpoint->url + . ' with callback ' . $this->callback->url() . ' as ' . $protocol + . ': ' . $status . ' ' . $message; + + if ($success) { + // Remembered so that later renewals go straight to the value this server accepts, + // instead of failing the preferred one every time. + $state['protocol'] = $protocol; + $state['error'] = false; + $state['error_message'] = ''; + $this->registry->save($state['url'], $state); + Minz_Log::notice($log, RSSCLOUD_LOG); + return true; + } - [$success, $message] = self::parseNotifyResult((string)$response['body'], (int)$response['status']); + // A negative status is FreshRSS-internal: the request never reached the server, so it + // cannot be objecting to the protocol and another value would fail identically. + $retrying = $status > 0 && isset($candidates[$i + 1]); + if ($retrying) { + // Not yet a fault: the fallback may still succeed, and warning here every time + // would make a working subscription look broken in the log. + Minz_Log::debug($log, RSSCLOUD_LOG); + } else { + Minz_Log::warning($log, RSSCLOUD_LOG); + break; + } + } - $state['error'] = !$success; - $state['error_message'] = $success ? '' : $message; + $state['error'] = true; + $state['error_message'] = $message === '' ? "HTTP {$status}" : $message; $this->registry->save($state['url'], $state); - $log = 'rssCloud pleaseNotify ' . $state['url'] . ' via ' . $endpoint->url - . ' with callback ' . $this->callback->url() - . ': ' . $response['status'] . ' ' . $message; - if ($success) { - Minz_Log::notice($log, RSSCLOUD_LOG); - } else { - Minz_Log::warning($log, RSSCLOUD_LOG); - } + return false; + } - return $success; + /** + * The `protocol` parameter of `pleaseNotify` names the notification method — `http-post` for + * REST, as against `xml-rpc` or `soap` — and not the scheme of the callback, which is carried by + * `port`. Servers disagree about this: some accept `https-post` as a TLS-flavoured spelling, + * while others take only the value the specification lists, so an HTTPS callback cannot simply + * assume either one. + * + * The value this server last accepted is therefore tried first, falling back to plain + * `http-post`, which every server understands. Registering over a plain HTTP callback has + * nothing to fall back to, and yields a single candidate. + * + * @return non-empty-list + */ + public static function protocolCandidates(string $remembered, string $callbackProtocol): array { + $preferred = $remembered !== '' ? $remembered : $callbackProtocol; + if ($preferred === RssCloud_Endpoint::PROTOCOL_HTTP) { + return [$preferred]; + } + return [$preferred, RssCloud_Endpoint::PROTOCOL_HTTP]; } /** diff --git a/i18n/en/ext.php b/i18n/en/ext.php index 325ed4d..6a44747 100644 --- a/i18n/en/ext.php +++ b/i18n/en/ext.php @@ -1,41 +1,68 @@ array( - 'base_url' => 'Public base URL override', - 'base_url.help' => 'Leave empty to use the instance base URL. Set this when a reverse proxy makes FreshRSS reachable under a different scheme, host or port than it sees internally.', - 'callback' => 'Notification callback', - 'callback.help' => 'This is the address advertised to rssCloud servers. It must be reachable from the public internet, otherwise the registration handshake fails and subscriptions are cancelled.', - 'callback.invalid' => 'No usable callback URL could be derived. Check the base URL below and the instance base_url setting.', - 'callback.private' => 'This address does not look publicly reachable, so rssCloud registration will fail.', - 'cooldown' => 'Notification cooldown (seconds)', - 'cooldown.help' => 'Minimum delay between two honoured notifications for the same resource. The callback is unauthenticated, so this bounds how often a stranger can make this server do work.', - 'feeds_enabled' => 'Subscribe to feeds', - 'feeds_enabled.help' => 'Use rssCloud for feeds advertising a or element.', - 'opml_enabled' => 'Subscribe to dynamic OPML', - 'opml_enabled.help' => 'Use rssCloud for dynamic OPML subscription lists advertising in their .', - 'regenerate_token' => 'Regenerate callback token', - 'regenerate_token.help' => 'Changes the secret path segment of the callback. Existing subscriptions stop being delivered until they are renewed.', - 'renew_hours' => 'Renew subscriptions after (hours)', - 'renew_hours.help' => 'rssCloud does not negotiate a lease duration. Subscriptions expire after 25 hours and are meant to be renewed every 24, so the default of 23 leaves a margin. Anything above 24 would simply lapse, and is capped.', - 'skip_polling' => 'Skip polling covered resources', - 'skip_polling.help' => 'Stop polling a resource on a timer while its cloud subscription is healthy. It is still polled if it goes stale, or when refreshed individually.', - 'status' => 'Subscriptions', - 'status.active' => 'Active', - 'status.endpoint' => 'Cloud server', - 'status.error' => 'Failed', - 'status.help' => 'One row per resource this instance has discovered a cloud server for. "Renewed" is when registration was last attempted, not when it last succeeded; the state column says whether it did.', - 'status.kind' => 'Type', - 'status.kind.feed' => 'Feed', - 'status.kind.opml' => 'Dynamic OPML', - 'status.never' => 'Never', - 'status.none' => 'No cloud servers have been discovered yet. Resources are registered as they are refreshed, so this fills in once feeds advertising a cloud have been fetched at least once.', - 'status.notified' => 'Last notified', - 'status.pending' => 'Pending', - 'status.renewed' => 'Renewal attempted', - 'status.resource' => 'Resource', - 'status.stale' => 'Stale', - 'status.state' => 'State', - 'status.subscribers' => 'Users', + 'base_url' => array( + '_' => 'Public base URL override', + 'help' => 'Leave empty to use the instance base URL. Set this when a reverse proxy makes FreshRSS reachable under a different scheme, host or port than it sees internally.', + ), + 'callback' => array( + '_' => 'Notification callback', + 'help' => 'This is the address advertised to rssCloud servers. It must be reachable from the public internet, otherwise the registration handshake fails and subscriptions are cancelled.', + 'invalid' => 'No usable callback URL could be derived. Check the base URL below and the instance base_url setting.', + 'private' => 'This address does not look publicly reachable, so rssCloud registration will fail.', + ), + 'cooldown' => array( + '_' => 'Notification cooldown (seconds)', + 'help' => 'Minimum delay between two honoured notifications for the same resource. The callback is unauthenticated, so this bounds how often a stranger can make this server do work.', + ), + 'feeds_enabled' => array( + '_' => 'Subscribe to feeds', + 'help' => 'Use rssCloud for feeds advertising a or element.', + ), + 'opml_enabled' => array( + '_' => 'Subscribe to dynamic OPML', + 'help' => 'Use rssCloud for dynamic OPML subscription lists advertising in their .', + ), + 'regenerate_token' => array( + '_' => 'Regenerate callback token', + 'help' => 'Changes the secret path segment of the callback. Existing subscriptions stop being delivered until they are renewed.', + ), + 'renew_hours' => array( + '_' => 'Renew subscriptions after (hours)', + 'help' => 'rssCloud does not negotiate a lease duration. Subscriptions expire after 25 hours and are meant to be renewed every 24, so the default of 23 leaves a margin. Anything above 24 would simply lapse, and is capped.', + ), + 'skip_polling' => array( + '_' => 'Skip polling covered resources', + 'help' => 'Stop polling a resource on a timer while its cloud subscription is healthy. It is still polled if it goes stale, or when refreshed individually.', + ), + 'status' => array( + '_' => 'Subscriptions', + 'active' => 'Active', + 'endpoint' => 'Cloud server', + 'error' => 'Failed', + 'help' => 'One row per resource this instance has discovered a cloud server for. "Renewal attempted" is when registration was last tried, not when it last succeeded; the state column says whether it did.', + 'kind' => array( + '_' => 'Type', + 'feed' => 'Feed', + 'opml' => 'Dynamic OPML', + ), + 'never' => 'Never', + 'none' => 'No cloud servers have been discovered yet. Resources are registered as they are refreshed, so this fills in once feeds advertising a cloud have been fetched at least once.', + 'notified' => 'Last notified', + 'pending' => 'Pending', + 'renewed' => 'Renewal attempted', + 'resource' => 'Resource', + 'stale' => 'Stale', + 'state' => 'State', + 'subscribers' => 'Users', + ), ), ); diff --git a/tests/RssCloud/RegistryTest.php b/tests/RssCloud/RegistryTest.php index e3de648..a218e15 100644 --- a/tests/RssCloud/RegistryTest.php +++ b/tests/RssCloud/RegistryTest.php @@ -32,7 +32,9 @@ protected function tearDown(): void { /** Write a state file straight to disk, bypassing save(), so malformed content can be staged. */ private function writeRaw(string $resourceUrl, string $json): void { $directory = $this->registry->directory($resourceUrl); - mkdir($directory, 0770, true); + if (!is_dir($directory)) { + mkdir($directory, 0770, true); + } if ($json !== '') { file_put_contents($directory . '/!cloud.json', $json); } @@ -114,6 +116,40 @@ public function test_load_treatsMissingErrorFlagAsError(): void { self::assertTrue($state['error']); } + /** Nothing is known about which protocol a server accepts until one has actually worked. */ + public function test_load_defaultsProtocolToUnknown(): void { + $this->writeState('https://j.example/feed', []); + + $state = $this->registry->load('https://j.example/feed'); + + self::assertIsArray($state); + self::assertSame('', $state['protocol']); + } + + /** + * Whatever is stored here is tried first, so a value this extension cannot speak must not + * survive being read back and be advertised to a cloud server. + */ + public function test_load_discardsUnrecognisedProtocol(): void { + foreach (['xml-rpc', 'soap', 'HTTP-POST', 'nonsense', ''] as $stored) { + $this->writeState('https://n.example/feed', ['protocol' => $stored]); + + $state = $this->registry->load('https://n.example/feed'); + + self::assertIsArray($state); + self::assertSame('', $state['protocol'], "stored protocol: {$stored}"); + } + } + + public function test_load_preservesRememberedProtocol(): void { + $this->writeState('https://k.example/feed', ['protocol' => RssCloud_Endpoint::PROTOCOL_HTTP]); + + $state = $this->registry->load('https://k.example/feed'); + + self::assertIsArray($state); + self::assertSame(RssCloud_Endpoint::PROTOCOL_HTTP, $state['protocol']); + } + public function test_all_yieldsOnlyUsableStateAndReportsTheRest(): void { $this->writeState('https://a.example/feed', ['endpoint' => 'https://rpc.example/x']); $this->writeRaw('https://b.example/feed', '{not json'); @@ -187,6 +223,7 @@ public function test_remember_resetsLeaseWhenEndpointChanges(): void { 'endpoint' => 'https://old.example/pleaseNotify', 'lease_start' => 999, 'error' => false, + 'protocol' => RssCloud_Endpoint::PROTOCOL_HTTP, ]); $endpoint = RssCloud_Endpoint::fromUrl('https://new.example/pleaseNotify'); @@ -195,6 +232,8 @@ public function test_remember_resetsLeaseWhenEndpointChanges(): void { self::assertSame(0, $moved['lease_start']); self::assertTrue($moved['error']); + // A different server need not accept what the old one did. + self::assertSame('', $moved['protocol']); } public function test_remember_keepsLeaseWhenEndpointIsUnchanged(): void { diff --git a/tests/RssCloud/SubscriberTest.php b/tests/RssCloud/SubscriberTest.php new file mode 100644 index 0000000..697e701 --- /dev/null +++ b/tests/RssCloud/SubscriberTest.php @@ -0,0 +1,65 @@ +