From 3fc0d75afcdbf05c547494b9578280f8d54942e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Bia=C5=82czak?= Date: Thu, 20 Aug 2026 08:18:36 +0200 Subject: [PATCH] IBX-12127: Fixed valid URLs being marked as invalid by ibexa:check-urls --- phpstan-baseline-gte-8.0.neon | 8 +- phpstan-baseline.neon | 36 --- .../Resources/config/default_settings.yml | 12 + .../Core/URLChecker/Handler/HTTPHandler.php | 141 ++++++++---- .../URLChecker/Handler/HTTPHandlerTest.php | 215 ++++++++++++++++++ 5 files changed, 319 insertions(+), 93 deletions(-) create mode 100644 tests/bundle/Core/URLChecker/Handler/HTTPHandlerTest.php diff --git a/phpstan-baseline-gte-8.0.neon b/phpstan-baseline-gte-8.0.neon index 874cf8f42c..b5b124f651 100644 --- a/phpstan-baseline-gte-8.0.neon +++ b/phpstan-baseline-gte-8.0.neon @@ -66,16 +66,10 @@ parameters: count: 1 path: src/bundle/Core/URLChecker/Handler/HTTPHandler.php - - - message: '#^Parameter \#1 \$handle of function curl_getinfo expects CurlHandle, resource given\.$#' - identifier: argument.type - count: 1 - path: src/bundle/Core/URLChecker/Handler/HTTPHandler.php - - message: '#^Parameter \#2 \$handle of function curl_multi_add_handle expects CurlHandle, resource given\.$#' identifier: argument.type - count: 2 + count: 3 path: src/bundle/Core/URLChecker/Handler/HTTPHandler.php - diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 6ac13d46c4..2b1cfd55c5 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -4668,42 +4668,6 @@ parameters: count: 1 path: src/bundle/Core/URLChecker/Handler/HTTPHandler.php - - - message: '#^Method Ibexa\\Bundle\\Core\\URLChecker\\Handler\\HTTPHandler\:\:createCurlHandlerForUrl\(\) has parameter \$handlers with no value type specified in iterable type array\.$#' - identifier: missingType.iterableValue - count: 1 - path: src/bundle/Core/URLChecker/Handler/HTTPHandler.php - - - - message: '#^Method Ibexa\\Bundle\\Core\\URLChecker\\Handler\\HTTPHandler\:\:doValidate\(\) has no return type specified\.$#' - identifier: missingType.return - count: 1 - path: src/bundle/Core/URLChecker/Handler/HTTPHandler.php - - - - message: '#^Method Ibexa\\Bundle\\Core\\URLChecker\\Handler\\HTTPHandler\:\:getOptions\(\) return type has no value type specified in iterable type array\.$#' - identifier: missingType.iterableValue - count: 1 - path: src/bundle/Core/URLChecker/Handler/HTTPHandler.php - - - - message: '#^Method Ibexa\\Bundle\\Core\\URLChecker\\Handler\\HTTPHandler\:\:isSuccessful\(\) has no return type specified\.$#' - identifier: missingType.return - count: 1 - path: src/bundle/Core/URLChecker/Handler/HTTPHandler.php - - - - message: '#^Method Ibexa\\Bundle\\Core\\URLChecker\\Handler\\HTTPHandler\:\:isSuccessful\(\) has parameter \$statusCode with no type specified\.$#' - identifier: missingType.parameter - count: 1 - path: src/bundle/Core/URLChecker/Handler/HTTPHandler.php - - - - message: '#^Method Ibexa\\Bundle\\Core\\URLChecker\\Handler\\HTTPHandler\:\:validate\(\) has no return type specified\.$#' - identifier: missingType.return - count: 1 - path: src/bundle/Core/URLChecker/Handler/HTTPHandler.php - - message: '#^Access to protected property Ibexa\\Contracts\\Core\\Repository\\Values\\URL\\URL\:\:\$url\.$#' identifier: property.protected diff --git a/src/bundle/Core/Resources/config/default_settings.yml b/src/bundle/Core/Resources/config/default_settings.yml index 6dc43995e5..b29a7c6d61 100644 --- a/src/bundle/Core/Resources/config/default_settings.yml +++ b/src/bundle/Core/Resources/config/default_settings.yml @@ -238,11 +238,23 @@ parameters: connection_timeout: 5 batch_size: 25 ignore_certificate: false + method: HEAD + fallback_to_get: true + user_agent: 'Mozilla/5.0 (X11; Linux x86_64; rv:128.0) Gecko/20100101 Firefox/128.0' + headers: + Accept: 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8' + Accept-Language: 'en-US,en;q=0.5' ibexa.site_access.config.default.url_handler.https.options: timeout: 10 connection_timeout: 5 batch_size: 25 ignore_certificate: false + method: HEAD + fallback_to_get: true + user_agent: 'Mozilla/5.0 (X11; Linux x86_64; rv:128.0) Gecko/20100101 Firefox/128.0' + headers: + Accept: 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8' + Accept-Language: 'en-US,en;q=0.5' ibexa.site_access.config.default.url_handler.mailto.options: {} ### diff --git a/src/bundle/Core/URLChecker/Handler/HTTPHandler.php b/src/bundle/Core/URLChecker/Handler/HTTPHandler.php index 50e1140807..3b4679b608 100644 --- a/src/bundle/Core/URLChecker/Handler/HTTPHandler.php +++ b/src/bundle/Core/URLChecker/Handler/HTTPHandler.php @@ -13,12 +13,20 @@ class HTTPHandler extends AbstractConfigResolverBasedURLHandler { + private const METHOD_HEAD = 'HEAD'; + private const METHOD_GET = 'GET'; + + private const DEFAULT_USER_AGENT = 'Mozilla/5.0 (X11; Linux x86_64; rv:128.0) Gecko/20100101 Firefox/128.0'; + + private const DEFAULT_HEADERS = [ + 'Accept' => 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8', + 'Accept-Language' => 'en-US,en;q=0.5', + ]; + /** * {@inheritdoc} - * - * Based on https://www.onlineaspect.com/2009/01/26/how-to-use-curl_multi-without-blocking/ */ - public function validate(array $urls) + public function validate(array $urls): void { $options = $this->getOptions(); @@ -27,51 +35,55 @@ public function validate(array $urls) } $master = curl_multi_init(); - $handlers = []; + $requests = []; - // Batch size can't be larger then number of urls $batchSize = min(count($urls), $options['batch_size']); for ($i = 0; $i < $batchSize; ++$i) { curl_multi_add_handle( $master, - $this->createCurlHandlerForUrl( - $urls[$i], - $handlers, - $options['connection_timeout'], - $options['timeout'] - ) + $this->createCurlHandlerForUrl($urls[$i], $options['method'], $options, $requests) ); } do { - while (($execrun = curl_multi_exec($master, $running)) == CURLM_CALL_MULTI_PERFORM); - - if ($execrun != CURLM_OK) { - break; - } + $status = curl_multi_exec($master, $running); while ($done = curl_multi_info_read($master)) { $handler = $done['handle']; + $request = $requests[(int)$handler]; + unset($requests[(int)$handler]); - $this->doValidate($handlers[(int)$handler], $handler); + $statusCode = (int)curl_getinfo($handler, CURLINFO_HTTP_CODE); - if ($i < count($urls)) { + if ($this->shouldRetryWithGet($statusCode, $request['method'], $options)) { + // Some servers and WAFs reject HEAD - recheck with GET before marking the URL as invalid curl_multi_add_handle( $master, - $this->createCurlHandlerForUrl( - $urls[$i], - $handlers, - $options['connection_timeout'], - $options['timeout'] - ) + $this->createCurlHandlerForUrl($request['url'], self::METHOD_GET, $options, $requests) ); - ++$i; + $running = 1; // handles added mid-loop are not reflected in $running yet + } else { + $this->setUrlStatus($request['url'], $this->isSuccessful($statusCode)); + + if ($i < count($urls)) { + curl_multi_add_handle( + $master, + $this->createCurlHandlerForUrl($urls[$i], $options['method'], $options, $requests) + ); + ++$i; + $running = 1; // as above + } } curl_multi_remove_handle($master, $handler); curl_close($handler); } - } while ($running); + + if ($running && curl_multi_select($master, 1.0) === -1) { + // select failure - back off briefly to avoid busy-looping + usleep(250); + } + } while ($running && $status === CURLM_OK); curl_multi_close($master); } @@ -88,6 +100,10 @@ protected function getOptionsResolver(): OptionsResolver 'connection_timeout' => 5, 'batch_size' => 10, 'ignore_certificate' => false, + 'method' => self::METHOD_HEAD, + 'fallback_to_get' => true, + 'user_agent' => self::DEFAULT_USER_AGENT, + 'headers' => self::DEFAULT_HEADERS, ]); $resolver->setAllowedTypes('enabled', 'bool'); @@ -95,30 +111,25 @@ protected function getOptionsResolver(): OptionsResolver $resolver->setAllowedTypes('connection_timeout', 'int'); $resolver->setAllowedTypes('batch_size', 'int'); $resolver->setAllowedTypes('ignore_certificate', 'bool'); + $resolver->setAllowedTypes('method', 'string'); + $resolver->setAllowedValues('method', [self::METHOD_HEAD, self::METHOD_GET]); + $resolver->setAllowedTypes('fallback_to_get', 'bool'); + $resolver->setAllowedTypes('user_agent', 'string'); + $resolver->setAllowedTypes('headers', 'array'); return $resolver; } - public function getOptions(): array - { - $options = $this->configResolver->getParameter('url_handler.http.options'); - - return $this->getOptionsResolver()->resolve($options); - } - /** * Initialize and return a cURL session for given URL. * - * @param \Ibexa\Contracts\Core\Repository\Values\URL\URL $url - * @param array $handlers - * @param int $connectionTimeout - * @param int $timeout + * @param array $options + * @param array $requests * * @return resource */ - private function createCurlHandlerForUrl(URL $url, array &$handlers, int $connectionTimeout, int $timeout) + private function createCurlHandlerForUrl(URL $url, string $method, array $options, array &$requests) { - $options = $this->getOptions(); $handler = curl_init(); if ($handler === false) { throw new RuntimeException('Unable to initialize cURL handler.'); @@ -134,36 +145,66 @@ private function createCurlHandlerForUrl(URL $url, array &$handlers, int $connec CURLOPT_URL => $urlString, CURLOPT_RETURNTRANSFER => false, CURLOPT_FOLLOWLOCATION => true, - CURLOPT_CONNECTTIMEOUT => $connectionTimeout, - CURLOPT_TIMEOUT => $timeout, + CURLOPT_MAXREDIRS => 10, + CURLOPT_CONNECTTIMEOUT => $options['connection_timeout'], + CURLOPT_TIMEOUT => $options['timeout'], CURLOPT_FAILONERROR => true, - CURLOPT_NOBODY => true, + CURLOPT_USERAGENT => $options['user_agent'], + CURLOPT_HTTPHEADER => $this->buildRequestHeaders($options['headers']), + CURLOPT_ENCODING => '', ]); - if (!empty($options['ignore_certificate'])) { + if ($method === self::METHOD_HEAD) { + curl_setopt($handler, CURLOPT_NOBODY, true); + } else { + // Abort on the first body chunk - the final (post-redirect) status code is already known + // and the body must not be streamed to the output (CURLOPT_RETURNTRANSFER is disabled). + curl_setopt($handler, CURLOPT_WRITEFUNCTION, static function ($handler, string $data): int { + return 0; + }); + } + + if ($options['ignore_certificate']) { curl_setopt_array($handler, [ CURLOPT_SSL_VERIFYPEER => false, CURLOPT_SSL_VERIFYHOST => 0, ]); } - $handlers[(int) $handler] = $url; + $requests[(int) $handler] = [ + 'url' => $url, + 'method' => $method, + ]; return $handler; } /** - * Validate single response. + * @param array $options + */ + private function shouldRetryWithGet(int $statusCode, string $requestMethod, array $options): bool + { + return $requestMethod === self::METHOD_HEAD + && $options['fallback_to_get'] + && !$this->isSuccessful($statusCode); + } + + /** + * @param array $headers * - * @param \Ibexa\Contracts\Core\Repository\Values\URL\URL $url - * @param resource $handler CURL handler + * @return string[] */ - private function doValidate(URL $url, $handler) + private function buildRequestHeaders(array $headers): array { - $this->setUrlStatus($url, $this->isSuccessful(curl_getinfo($handler, CURLINFO_HTTP_CODE))); + $lines = []; + foreach ($headers as $name => $value) { + $lines[] = is_int($name) ? $value : sprintf('%s: %s', $name, $value); + } + + return $lines; } - private function isSuccessful($statusCode) + private function isSuccessful(int $statusCode): bool { return $statusCode >= 200 && $statusCode < 300; } diff --git a/tests/bundle/Core/URLChecker/Handler/HTTPHandlerTest.php b/tests/bundle/Core/URLChecker/Handler/HTTPHandlerTest.php new file mode 100644 index 0000000000..d74f4ba175 --- /dev/null +++ b/tests/bundle/Core/URLChecker/Handler/HTTPHandlerTest.php @@ -0,0 +1,215 @@ +urlService = $this->createMock(URLService::class); + $this->configResolver = $this->createMock(ConfigResolverInterface::class); + } + + public function testGetOptionsDefaults(): void + { + $handler = $this->createHandler([]); + + $options = $handler->getOptions(); + + self::assertTrue($options['enabled']); + self::assertSame(10, $options['timeout']); + self::assertSame(5, $options['connection_timeout']); + self::assertSame(10, $options['batch_size']); + self::assertFalse($options['ignore_certificate']); + self::assertSame('HEAD', $options['method']); + self::assertTrue($options['fallback_to_get']); + self::assertNotEmpty($options['user_agent']); + self::assertArrayHasKey('Accept', $options['headers']); + } + + public function testGetOptionsUsesInjectedParameterName(): void + { + $this->configResolver + ->expects(self::once()) + ->method('getParameter') + ->with('url_handler.https.options', null, null) + ->willReturn([]); + + $handler = new HTTPHandler( + $this->urlService, + $this->configResolver, + 'url_handler.https.options' + ); + + $handler->getOptions(); + } + + public function testGetOptionsRejectsInvalidMethod(): void + { + $handler = $this->createHandler([ + 'method' => 'POST', + ]); + + $this->expectException(InvalidOptionsException::class); + + $handler->getOptions(); + } + + public function testBuildRequestHeaders(): void + { + $handler = $this->createHandler([]); + + $headers = $this->invokePrivateMethod($handler, 'buildRequestHeaders', [ + [ + 'Accept' => 'text/html', + 'Accept-Language' => 'en', + 'X-Raw-Header: 1', + ], + ]); + + self::assertSame( + [ + 'Accept: text/html', + 'Accept-Language: en', + 'X-Raw-Header: 1', + ], + $headers + ); + } + + /** + * @dataProvider provideDataForTestIsSuccessful + */ + public function testIsSuccessful(int $statusCode, bool $expected): void + { + $handler = $this->createHandler([]); + + self::assertSame( + $expected, + $this->invokePrivateMethod($handler, 'isSuccessful', [$statusCode]) + ); + } + + /** + * @return iterable + */ + public static function provideDataForTestIsSuccessful(): iterable + { + yield 'curl error' => [0, false]; + yield 'status 199' => [199, false]; + yield 'status 200' => [200, true]; + yield 'status 204' => [204, true]; + yield 'status 299' => [299, true]; + yield 'status 300' => [300, false]; + yield 'status 403' => [403, false]; + yield 'status 404' => [404, false]; + yield 'status 500' => [500, false]; + } + + /** + * @dataProvider provideDataForTestShouldRetryWithGet + */ + public function testShouldRetryWithGet( + int $statusCode, + string $requestMethod, + bool $fallbackToGet, + bool $expected + ): void { + $handler = $this->createHandler([]); + + self::assertSame( + $expected, + $this->invokePrivateMethod($handler, 'shouldRetryWithGet', [ + $statusCode, + $requestMethod, + ['fallback_to_get' => $fallbackToGet], + ]) + ); + } + + /** + * @return iterable + */ + public static function provideDataForTestShouldRetryWithGet(): iterable + { + yield 'HEAD blocked by WAF' => [403, 'HEAD', true, true]; + yield 'HEAD not allowed' => [405, 'HEAD', true, true]; + yield 'HEAD curl error' => [0, 'HEAD', true, true]; + yield 'HEAD succeeded' => [200, 'HEAD', true, false]; + yield 'GET is final' => [403, 'GET', true, false]; + yield 'fallback disabled' => [403, 'HEAD', false, false]; + } + + public function testValidateDoesNothingWhenDisabled(): void + { + $handler = $this->createHandler([ + 'enabled' => false, + ]); + + $this->urlService + ->expects(self::never()) + ->method('updateUrl'); + + $handler->validate([ + new URL([ + 'id' => 1, + 'url' => 'http://127.0.0.1:1/', + ]), + ]); + } + + /** + * @param array $options + */ + private function createHandler(array $options): HTTPHandler + { + $this->configResolver + ->method('getParameter') + ->with(self::PARAMETER_NAME, null, null) + ->willReturn($options); + + return new HTTPHandler( + $this->urlService, + $this->configResolver, + self::PARAMETER_NAME + ); + } + + /** + * @param array $arguments + * + * @return mixed + */ + private function invokePrivateMethod(HTTPHandler $handler, string $method, array $arguments) + { + $reflection = new ReflectionMethod(HTTPHandler::class, $method); + $reflection->setAccessible(true); + + return $reflection->invokeArgs($handler, $arguments); + } +}