Skip to content

Improve SimplePie redirects (POST to GET, remove authentication cross-origin) #78

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Alkarex merged 16 commits into from
Jun 27, 2026
Merged

Improve SimplePie redirects (POST to GET, remove authentication cross-origin) #78

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
cff0d36
Improve SimplePie redirects (POST to GET, remove authentication cross…
Inverle May 30, 2026
0c3b3d7
PHP 7.2+ compatibility
Inverle May 30, 2026
0d69814
Normalize URLs to lowercase when parsing
Inverle May 30, 2026
4c8a012
Fix wrong URL printed in error message
Inverle May 30, 2026
f956c3f
Merge branch 'freshrss' into improve-sp-redirects
Alkarex Jun 11, 2026
010a7dd
Same logic for fsockopen
Alkarex Jun 11, 2026
19a9420
Merge branch 'freshrss' into improve-sp-redirects
Inverle Jun 13, 2026
55d17de
Also unset `CURLOPT_USERPWD` during redirects
Inverle Jun 13, 2026
56ecb13
Add infinite redirects for the `fsockopen()` path too
Inverle Jun 13, 2026
dca64c0
Merge branch 'freshrss' into improve-sp-redirects
Alkarex Jun 13, 2026
10bff0a
Draft rework CURLOPT_FOLLOWLOCATION
Alkarex Jun 21, 2026
171a122
Merge branch 'freshrss' into pr/Inverle/78
Alkarex Jun 27, 2026
1313590
A bit of documentation
Alkarex Jun 27, 2026
c4e0a97
Minor doc
Alkarex Jun 27, 2026
d253211
Move curl_close
Alkarex Jun 27, 2026
c698b4b
composer run fix
Alkarex Jun 27, 2026
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
135 changes: 119 additions & 16 deletions src/File.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,11 @@ class File implements Response
* @param ?array<string, string> $headers
* @param ?string $useragent
* @param bool $force_fsockopen
* @param array<int, mixed> $curl_options
* @param array<int, mixed> $curl_options Specify cURL options.
* Special case for HTTP redirect handling:
* * CURLOPT_FOLLOWLOCATION not set or null: SimplePie PHP handling of HTTP redirections with security checks (default, recommended).
* * CURLOPT_FOLLOWLOCATION = truthy: Native cURL handling of HTTP redirections _without_ SimplePie security checks;
* * CURLOPT_FOLLOWLOCATION = falsy: No HTTP redirections at all;
*/
public function __construct(string $url, int $timeout = 10, int $redirects = 5, ?array $headers = null, ?string $useragent = null, bool $force_fsockopen = false, array $curl_options = [])
{
Expand Down Expand Up @@ -163,27 +167,91 @@ public function __construct(string $url, int $timeout = 10, int $redirects = 5,
$responseHeaders = \SimplePie\HTTP\Parser::prepareHeaders($responseHeaders);
}
$this->on_http_response($responseHeaders . $responseBody, $curl_options);
if (\PHP_VERSION_ID < 80000) {
curl_close($fp);
}
$parser = new \SimplePie\HTTP\Parser($responseHeaders, true);
if ($parser->parse()) {
$this->set_headers($parser->headers);
$this->body = $responseBody;
if ((in_array($this->status_code, [300, 301, 302, 303, 307]) || $this->status_code > 307 && $this->status_code < 400) &&
($locationHeader = $this->get_header_line('location')) !== '' && ($this->redirects < $redirects || $redirects === -1)) { // FreshRSS: added infinite redirects for -1
$this->redirects++;
$location = \SimplePie\Misc::absolutize_url($locationHeader, $url);
if ($location === false) {
$this->error = "Invalid redirect location, trying to base “{$locationHeader}” onto “{$url}”";
$this->success = false;
// Handling of HTTP redirections:
$followLocation = $curl_options[CURLOPT_FOLLOWLOCATION] ?? null;

if ($followLocation) {
// Native cURL handling of HTTP redirections
$finalUrl = curl_getinfo($fp, CURLINFO_EFFECTIVE_URL);
if (is_string($finalUrl) && $finalUrl !== '') {
$this->url = $finalUrl;
}
} elseif ($followLocation === null) {
// SimplePie PHP handling of HTTP redirections (default)
if ((in_array($this->status_code, [300, 301, 302, 303, 307]) || $this->status_code > 307 && $this->status_code < 400) &&
($locationHeader = $this->get_header_line('location')) !== '' && ($this->redirects < $redirects || $redirects === -1)) { // FreshRSS: added infinite redirects for -1
$this->redirects++;
$location = \SimplePie\Misc::absolutize_url($locationHeader, $url);
if ($location === false) {
$this->error = "Invalid redirect location, trying to base “{$locationHeader}” onto “{$url}”";
$this->success = false;
return;
}

// FreshRSS: POST to GET on redirect
if (isset($curl_options[CURLOPT_POST]) && in_array($this->status_code, [301, 302, 303], true)) { // Not for 307 and 308, which must not change the HTTP method
unset($curl_options[CURLOPT_POST]);
unset($curl_options[CURLOPT_POSTFIELDS]);
if (is_array($curl_options[CURLOPT_HTTPHEADER] ?? null)) {
$curl_options[CURLOPT_HTTPHEADER] = array_filter(
$curl_options[CURLOPT_HTTPHEADER],
function ($header) {
return is_string($header) && substr(strtolower(trim($header)), 0, 13) !== 'content-type:';
}
);
}
}
// FreshRSS: cross-origin authentication headers removal
if (($url_parts_from = parse_url(strtolower($url))) === false) {
throw new \InvalidArgumentException('Malformed URL: ' . $url);
}
if (($url_parts_to = parse_url(strtolower($location))) === false) {
$this->error = "Invalid redirect location: malformed URL “{$location}”";
$this->success = false;
return;
}
foreach ([&$url_parts_from, &$url_parts_to] as &$url_parts) {
if (!isset($url_parts['port']) && isset($url_parts['scheme'])) {
if ($url_parts['scheme'] === 'http') {
$url_parts['port'] = 80;
} elseif ($url_parts['scheme'] === 'https') {
$url_parts['port'] = 443;
}
}
}
unset($url_parts);
$sameOriginRedirect =
($url_parts_from['scheme'] ?? '') === ($url_parts_to['scheme'] ?? '') &&
($url_parts_from['host'] ?? '') === ($url_parts_to['host'] ?? '') &&
($url_parts_from['port'] ?? '') === ($url_parts_to['port'] ?? '');
if (!$sameOriginRedirect) {
unset($curl_options[CURLOPT_COOKIE]);
unset($curl_options[CURLOPT_USERPWD]);
if (is_array($curl_options[CURLOPT_HTTPHEADER] ?? null)) {
$curl_options[CURLOPT_HTTPHEADER] = array_filter(
$curl_options[CURLOPT_HTTPHEADER],
function ($header) {
return is_string($header) && !preg_match('/^(Cookie|Authorization)\s*:/i', $header);
}
);
}
}

$this->permanentUrlMutable = $this->permanentUrlMutable && ($this->status_code == 301 || $this->status_code == 308);
$this->__construct($location, $timeout, $redirects, $headers, $useragent, $force_fsockopen, $curl_options);
return;
}
$this->permanentUrlMutable = $this->permanentUrlMutable && ($this->status_code == 301 || $this->status_code == 308);
$this->__construct($location, $timeout, $redirects, $headers, $useragent, $force_fsockopen, $curl_options);
return;
// } elseif ($followLocation == false) {
// No HTTP redirections at all
}
}
if (\PHP_VERSION_ID < 80000) {
curl_close($fp);
}
}
} else {
$this->method = \SimplePie\SimplePie::FILE_SOURCE_REMOTE | \SimplePie\SimplePie::FILE_SOURCE_FSOCKOPEN;
Expand Down Expand Up @@ -247,7 +315,8 @@ public function __construct(string $url, int $timeout = 10, int $redirects = 5,
$this->body = $parser->body;
$this->status_code = $parser->status_code;
$this->on_http_response($responseHeaders);
if ((in_array($this->status_code, [300, 301, 302, 303, 307]) || $this->status_code > 307 && $this->status_code < 400) && ($locationHeader = $this->get_header_line('location')) !== '' && $this->redirects < $redirects) {
if ((in_array($this->status_code, [300, 301, 302, 303, 307]) || $this->status_code > 307 && $this->status_code < 400) &&
($locationHeader = $this->get_header_line('location')) !== '' && ($this->redirects < $redirects || $redirects === -1)) { // FreshRSS: added infinite redirects for -1
$this->redirects++;
$location = \SimplePie\Misc::absolutize_url($locationHeader, $url);
$this->permanentUrlMutable = $this->permanentUrlMutable && ($this->status_code == 301 || $this->status_code == 308);
Expand All @@ -256,6 +325,41 @@ public function __construct(string $url, int $timeout = 10, int $redirects = 5,
$this->success = false;
return;
}

// FreshRSS: POST to GET on redirect is not applicable here as fsockopen only ever performs GET requests.
// FreshRSS: cross-origin authentication headers removal
if (($url_parts_from = parse_url(strtolower($url))) === false) {
throw new \InvalidArgumentException('Malformed URL: ' . $url);
}
if (($url_parts_to = parse_url(strtolower($location))) === false) {
$this->error = "Invalid redirect location: malformed URL “{$location}”";
$this->success = false;
return;
}
foreach ([&$url_parts_from, &$url_parts_to] as &$url_parts) {
if (!isset($url_parts['port']) && isset($url_parts['scheme'])) {
if ($url_parts['scheme'] === 'http') {
$url_parts['port'] = 80;
} elseif ($url_parts['scheme'] === 'https') {
$url_parts['port'] = 443;
}
}
}
unset($url_parts);
$sameOriginRedirect =
($url_parts_from['scheme'] ?? '') === ($url_parts_to['scheme'] ?? '') &&
($url_parts_from['host'] ?? '') === ($url_parts_to['host'] ?? '') &&
($url_parts_from['port'] ?? '') === ($url_parts_to['port'] ?? '');
if (!$sameOriginRedirect) {
$headers = array_filter(
$headers,
function (string $key) {
return !preg_match('/^(Cookie|Authorization)$/i', $key);
},
ARRAY_FILTER_USE_KEY
);
}

$this->__construct($location, $timeout, $redirects, $headers, $useragent, $force_fsockopen, $curl_options);
return;
}
Expand Down Expand Up @@ -522,7 +626,6 @@ private static function curlInit(
foreach ($curl_options as $curl_param => $curl_value) {
curl_setopt($fp, $curl_param, $curl_value);
}
curl_setopt($fp, CURLOPT_FOLLOWLOCATION, false); // FreshRSS

return $fp;
}
Expand Down
115 changes: 115 additions & 0 deletions tests/Integration/HTTP/ClientsTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,121 @@ public function testRedirectsChain(Client $client): void
self::assertSame($finalLocation, $response->get_final_requested_uri());
}

/**
* Test native cURL redirect handling when CURLOPT_FOLLOWLOCATION is enabled
* This test verifies that redirects are followed by cURL natively instead of custom handling
*/
public function testNativeCurlRedirectHandling(): void
{
$client = new FileClient(new Registry(), [
'redirects' => 10,
'force_fsockopen' => false,
'curl_options' => [
CURLOPT_FOLLOWLOCATION => true, // Enable native cURL redirect handling
],
]);

$server = new MockWebServer();
$server->start();

$server->setDefaultResponse(new NotFoundResponse());
$url = $server->setResponseOfPath(
'/redirect-start',
new MockWebServerResponse('', ['Location: /redirect-end'], 302)
);
$finalLocation = $server->setResponseOfPath(
'/redirect-end',
new MockWebServerResponse('OK', [], 200)
);

$response = $client->request(Client::METHOD_GET, $url);

$server->stop();

self::assertSame(200, $response->get_status_code());
self::assertSame('OK', $response->get_body_content());
// With native cURL handling, permanent_uri should be the final URL (cURL doesn't track permanent redirects separately)
self::assertSame($finalLocation, $response->get_final_requested_uri());
}

/**
* Test that native cURL redirect handling bypasses custom security features
* This is expected behavior - when using CURLOPT_FOLLOWLOCATION, cURL handles everything
*/
public function testNativeCurlRedirectHandlingBypassesSecurityFeatures(): void
{
// This test documents that native cURL handling bypasses security features
// In a real scenario, you would test that auth headers are NOT removed on cross-origin redirects

$client = new FileClient(new Registry(), [
'redirects' => 10,
'force_fsockopen' => false,
'curl_options' => [
CURLOPT_FOLLOWLOCATION => true, // Native handling bypasses security features
],
]);

$server = new MockWebServer();
$server->start();

$server->setDefaultResponse(new NotFoundResponse());
// Create a redirect chain
$url = $server->setResponseOfPath(
'/start',
new MockWebServerResponse('', ['Location: /end'], 302)
);
$finalLocation = $server->setResponseOfPath(
'/end',
new MockWebServerResponse('OK', [], 200)
);

$response = $client->request(Client::METHOD_GET, $url);
$server->stop();

// Verify redirect was followed
self::assertSame(200, $response->get_status_code());
self::assertSame('OK', $response->get_body_content());
self::assertSame($finalLocation, $response->get_final_requested_uri());
}

/**
* Test that CURLOPT_FOLLOWLOCATION = false prevents all redirect handling
* This verifies the "no redirect handling" mode where the client is responsible for redirects
*/
public function testNoRedirectHandling(): void
{
$client = new FileClient(new Registry(), [
'redirects' => 10,
'force_fsockopen' => false,
'curl_options' => [
CURLOPT_FOLLOWLOCATION => false, // Disable all redirect handling
],
]);

$server = new MockWebServer();
$server->start();

$server->setDefaultResponse(new NotFoundResponse());
$url = $server->setResponseOfPath(
'/redirect-start',
new MockWebServerResponse('Redirecting', ['Location: /redirect-end'], 302)
);
$server->setResponseOfPath(
'/redirect-end',
new MockWebServerResponse('Final', [], 200)
);

$response = $client->request(Client::METHOD_GET, $url);
$server->stop();

// Verify redirect was NOT followed - should return the 302 response
self::assertSame(302, $response->get_status_code());
self::assertSame('Redirecting', $response->get_body_content());
self::assertStringContainsString('/redirect-start', $response->get_final_requested_uri());
// The location header should be present for the client to handle
self::assertNotEmpty($response->get_header_line('location'));
}

/**
* @return iterable<array{client: Client, kind: "remote"|"remote-gzip"|"local", endianness: "le"|"be"}>
*/
Expand Down
Loading