diff --git a/RssCloud/Subscriber.php b/RssCloud/Subscriber.php index ba5069a..3fcb21a 100644 --- a/RssCloud/Subscriber.php +++ b/RssCloud/Subscriber.php @@ -57,6 +57,31 @@ public static function isHealthy(array $state, int $renewAfterSeconds): bool { && $state['lease_start'] > time() - $renewAfterSeconds; } + /** + * The cURL options a `pleaseNotify` POST is sent with. + * + * `CURLOPT_POSTREDIR` is the load-bearing one, and it is not redundant even though it can look + * that way. Most rssCloud endpoints are advertised as `http` in the `` element and answer + * with a permanent redirect to `https`; libcurl's default on 301/302/303 is to follow it as a + * bodyless GET, exactly as `curl -L` does. The server then sees no `url1` and answers + * `No feed for url1.`, which is what made several servers look permanently broken. + * + * On FreshRSS 1.29.x this is the whole ballgame: `httpGet()` sets `CURLOPT_FOLLOWLOCATION` and + * lets libcurl do the following, so without this option the body is lost. Later versions follow + * redirects by hand and happen to keep the body — which is why removing this line would look + * harmless when tested against a development build and still break every released one. + * + * @return array + */ + public static function notifyCurlOptions(string $body): array { + return [ + CURLOPT_POSTFIELDS => $body, + // Keep the POST (and its body) across a 301/302/303 instead of degrading to GET. + CURLOPT_POSTREDIR => CURL_REDIR_POST_ALL, + CURLOPT_MAXREDIRS => 10, + ]; + } + /** * Issue a `pleaseNotify` for one resource. * @@ -85,10 +110,9 @@ public function subscribe(array $state): bool { $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, - ]); + $response = FreshRSS_http_Util::httpGet($endpoint->url, null, 'xml', [], self::notifyCurlOptions( + http_build_query($parameters + ['protocol' => $protocol]), + )); $status = (int)$response['status']; [$success, $message] = self::parseNotifyResult((string)$response['body'], $status); diff --git a/tests/RssCloud/SubscriberTest.php b/tests/RssCloud/SubscriberTest.php index 697e701..deb246f 100644 --- a/tests/RssCloud/SubscriberTest.php +++ b/tests/RssCloud/SubscriberTest.php @@ -51,6 +51,25 @@ public function test_rememberedValueTakesPrecedence(): void { self::assertSame(RssCloud_Endpoint::PROTOCOL_HTTP, $candidates[0]); } + /** + * Most `` elements advertise an http endpoint that permanently redirects to https. Without + * this option libcurl follows that redirect as a bodyless GET, the server sees no `url1`, and + * answers `No feed for url1.` — the failure that made several servers look permanently broken. + */ + public function test_notifyKeepsThePostBodyAcrossARedirect(): void { + $options = RssCloud_Subscriber::notifyCurlOptions('url1=https%3A%2F%2Fexample.com%2Ffeed.xml'); + + self::assertSame(CURL_REDIR_POST_ALL, $options[CURLOPT_POSTREDIR] ?? null, + 'a 301 from http to https must not degrade the pleaseNotify POST into a GET'); + } + + /** The body is what carries url1, so it has to survive the option assembly intact. */ + public function test_notifySendsTheBodyItWasGiven(): void { + $body = 'domain=example.org&port=443&url1=https%3A%2F%2Fexample.com%2Ffeed.xml'; + + self::assertSame($body, RssCloud_Subscriber::notifyCurlOptions($body)[CURLOPT_POSTFIELDS] ?? null); + } + /** Whatever the inputs, the caller always has at least one value to send. */ public function test_alwaysYieldsAtLeastOneCandidate(): void { foreach (['', RssCloud_Endpoint::PROTOCOL_HTTP, RssCloud_Endpoint::PROTOCOL_HTTPS] as $remembered) {