From 237937b01da6da818c64db0ff356634a5993c3de Mon Sep 17 00:00:00 2001 From: Andrew Shell Date: Fri, 7 Aug 2026 08:46:46 -0500 Subject: [PATCH] fix(subscriber): keep the POST body across the http to https redirect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit rssCloud endpoints are usually advertised as `http` in the `` element and answer with a permanent redirect to `https`. libcurl's default on 301/302/303 is to follow that as a bodyless GET, exactly as `curl -L` does, so the server received no `url1` and answered `No feed for url1.` — which is what made many servers show as permanently failed in the admin screen. On FreshRSS 1.29.0 and 1.29.1 this is the whole story: `httpGet()` sets `CURLOPT_FOLLOWLOCATION` and lets libcurl follow the redirect, so the body was always lost. Development builds follow redirects by hand and happen to preserve it, which is why the bug does not reproduce when tested against `edge` — and why the option below looks redundant there. Set `CURLOPT_POSTREDIR` so the POST survives the redirect, and extract the option assembly into `notifyCurlOptions()` so the behaviour can be pinned by a test rather than rediscovered. Verified against a redirecting echo server: without the option the request arrives as a GET with an empty body; with it, `url1` arrives intact. Co-Authored-By: Claude Opus 5 --- RssCloud/Subscriber.php | 32 +++++++++++++++++++++++++++---- tests/RssCloud/SubscriberTest.php | 19 ++++++++++++++++++ 2 files changed, 47 insertions(+), 4 deletions(-) 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) {