Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 28 additions & 4 deletions RssCloud/Subscriber.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<cloud>` 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<int,mixed>
*/
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.
*
Expand Down Expand Up @@ -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);

Expand Down
19 changes: 19 additions & 0 deletions tests/RssCloud/SubscriberTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,25 @@ public function test_rememberedValueTakesPrecedence(): void {
self::assertSame(RssCloud_Endpoint::PROTOCOL_HTTP, $candidates[0]);
}

/**
* Most `<cloud>` 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) {
Expand Down