diff --git a/src/File.php b/src/File.php index 13a50afdf..cf22f0633 100644 --- a/src/File.php +++ b/src/File.php @@ -84,7 +84,11 @@ class File implements Response * @param ?array $headers * @param ?string $useragent * @param bool $force_fsockopen - * @param array $curl_options + * @param array $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 = []) { @@ -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; @@ -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); @@ -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; } @@ -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; } diff --git a/tests/Integration/HTTP/ClientsTest.php b/tests/Integration/HTTP/ClientsTest.php index 77ff455ad..4a8041e59 100644 --- a/tests/Integration/HTTP/ClientsTest.php +++ b/tests/Integration/HTTP/ClientsTest.php @@ -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 */