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 1/2] 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); + } +} From 57f5a79b7cbb01f362e5f86edf88ff148c4965f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Bia=C5=82czak?= Date: Thu, 27 Aug 2026 09:33:59 +0200 Subject: [PATCH 2/2] IBX-12127: Applied changes after code review. --- phpstan-baseline-gte-8.0.neon | 2 +- .../Resources/config/default_settings.yml | 16 ++-- .../Core/URLChecker/Handler/HTTPHandler.php | 78 ++++++++------- .../URLChecker/Handler/HTTPHandlerTest.php | 96 ++++++++++++++++++- 4 files changed, 141 insertions(+), 51 deletions(-) diff --git a/phpstan-baseline-gte-8.0.neon b/phpstan-baseline-gte-8.0.neon index b5b124f651..43466993d0 100644 --- a/phpstan-baseline-gte-8.0.neon +++ b/phpstan-baseline-gte-8.0.neon @@ -69,7 +69,7 @@ parameters: - message: '#^Parameter \#2 \$handle of function curl_multi_add_handle expects CurlHandle, resource given\.$#' identifier: argument.type - count: 3 + count: 2 path: src/bundle/Core/URLChecker/Handler/HTTPHandler.php - diff --git a/src/bundle/Core/Resources/config/default_settings.yml b/src/bundle/Core/Resources/config/default_settings.yml index b29a7c6d61..db2499031a 100644 --- a/src/bundle/Core/Resources/config/default_settings.yml +++ b/src/bundle/Core/Resources/config/default_settings.yml @@ -238,23 +238,19 @@ parameters: connection_timeout: 5 batch_size: 25 ignore_certificate: false - method: HEAD + method: !php/const Ibexa\Bundle\Core\URLChecker\Handler\HTTPHandler::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' + user_agent: !php/const Ibexa\Bundle\Core\URLChecker\Handler\HTTPHandler::DEFAULT_USER_AGENT + headers: !php/const Ibexa\Bundle\Core\URLChecker\Handler\HTTPHandler::DEFAULT_HEADERS ibexa.site_access.config.default.url_handler.https.options: timeout: 10 connection_timeout: 5 batch_size: 25 ignore_certificate: false - method: HEAD + method: !php/const Ibexa\Bundle\Core\URLChecker\Handler\HTTPHandler::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' + user_agent: !php/const Ibexa\Bundle\Core\URLChecker\Handler\HTTPHandler::DEFAULT_USER_AGENT + headers: !php/const Ibexa\Bundle\Core\URLChecker\Handler\HTTPHandler::DEFAULT_HEADERS 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 3b4679b608..dc8fa432a6 100644 --- a/src/bundle/Core/URLChecker/Handler/HTTPHandler.php +++ b/src/bundle/Core/URLChecker/Handler/HTTPHandler.php @@ -13,19 +13,16 @@ class HTTPHandler extends AbstractConfigResolverBasedURLHandler { - private const METHOD_HEAD = 'HEAD'; - private const METHOD_GET = 'GET'; + public const METHOD_HEAD = 'HEAD'; + public const METHOD_GET = 'GET'; - private const DEFAULT_USER_AGENT = 'Mozilla/5.0 (X11; Linux x86_64; rv:128.0) Gecko/20100101 Firefox/128.0'; + public const DEFAULT_USER_AGENT = 'Mozilla/5.0 (X11; Linux x86_64; rv:128.0) Gecko/20100101 Firefox/128.0'; - private const DEFAULT_HEADERS = [ + public 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} - */ public function validate(array $urls): void { $options = $this->getOptions(); @@ -38,59 +35,47 @@ public function validate(array $urls): void $requests = []; $batchSize = min(count($urls), $options['batch_size']); - for ($i = 0; $i < $batchSize; ++$i) { + $queue = array_slice($urls, $batchSize); + foreach (array_slice($urls, 0, $batchSize) as $url) { curl_multi_add_handle( $master, - $this->createCurlHandlerForUrl($urls[$i], $options['method'], $options, $requests) + $this->createCurlHandlerForUrl($url, $options['method'], $options, $requests) ); } do { $status = curl_multi_exec($master, $running); - while ($done = curl_multi_info_read($master)) { + while (($done = curl_multi_info_read($master)) !== false) { $handler = $done['handle']; $request = $requests[(int)$handler]; unset($requests[(int)$handler]); $statusCode = (int)curl_getinfo($handler, CURLINFO_HTTP_CODE); + curl_multi_remove_handle($master, $handler); + curl_close($handler); - if ($this->shouldRetryWithGet($statusCode, $request['method'], $options)) { - // Some servers and WAFs reject HEAD - recheck with GET before marking the URL as invalid + $nextRequest = $this->completeRequest($request, $statusCode, $options, $queue); + if ($nextRequest !== null) { curl_multi_add_handle( $master, - $this->createCurlHandlerForUrl($request['url'], self::METHOD_GET, $options, $requests) + $this->createCurlHandlerForUrl($nextRequest['url'], $nextRequest['method'], $options, $requests) ); - $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); } - if ($running && curl_multi_select($master, 1.0) === -1) { + // handles added mid-loop are not reflected in $running yet + $running = count($requests); + + if ($running > 0 && curl_multi_select($master, 1.0) === -1) { // select failure - back off briefly to avoid busy-looping usleep(250); } - } while ($running && $status === CURLM_OK); + } while ($running > 0 && $status === CURLM_OK); curl_multi_close($master); } - /** - * {@inheritdoc} - */ protected function getOptionsResolver(): OptionsResolver { $resolver = new OptionsResolver(); @@ -120,6 +105,29 @@ protected function getOptionsResolver(): OptionsResolver return $resolver; } + /** + * Records the result of a finished request and returns the request to schedule next, if any. + * + * @param array{url: \Ibexa\Contracts\Core\Repository\Values\URL\URL, method: string} $request + * @param array $options + * @param \Ibexa\Contracts\Core\Repository\Values\URL\URL[] $queue URLs waiting to be checked + * + * @return array{url: \Ibexa\Contracts\Core\Repository\Values\URL\URL, method: string}|null + */ + private function completeRequest(array $request, int $statusCode, array $options, array &$queue): ?array + { + if ($this->shouldRetryWithGet($statusCode, $request['method'], $options)) { + // Some servers and WAFs reject HEAD - recheck with GET before marking the URL as invalid + return ['url' => $request['url'], 'method' => self::METHOD_GET]; + } + + $this->setUrlStatus($request['url'], $this->isSuccessful($statusCode)); + + $nextUrl = array_shift($queue); + + return $nextUrl !== null ? ['url' => $nextUrl, 'method' => $options['method']] : null; + } + /** * Initialize and return a cURL session for given URL. * @@ -159,9 +167,7 @@ private function createCurlHandlerForUrl(URL $url, string $method, array $option } 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; - }); + curl_setopt($handler, CURLOPT_WRITEFUNCTION, static fn (): int => 0); } if ($options['ignore_certificate']) { diff --git a/tests/bundle/Core/URLChecker/Handler/HTTPHandlerTest.php b/tests/bundle/Core/URLChecker/Handler/HTTPHandlerTest.php index d74f4ba175..d2c9d63fb4 100644 --- a/tests/bundle/Core/URLChecker/Handler/HTTPHandlerTest.php +++ b/tests/bundle/Core/URLChecker/Handler/HTTPHandlerTest.php @@ -11,6 +11,7 @@ use Ibexa\Bundle\Core\URLChecker\Handler\HTTPHandler; use Ibexa\Contracts\Core\Repository\URLService; use Ibexa\Contracts\Core\Repository\Values\URL\URL; +use Ibexa\Contracts\Core\Repository\Values\URL\URLUpdateStruct; use Ibexa\Contracts\Core\SiteAccess\ConfigResolverInterface; use PHPUnit\Framework\TestCase; use ReflectionMethod; @@ -23,15 +24,16 @@ final class HTTPHandlerTest extends TestCase { private const PARAMETER_NAME = 'url_handler.http.options'; - /** @var \Ibexa\Contracts\Core\Repository\URLService|\PHPUnit\Framework\MockObject\MockObject */ - private $urlService; + /** @var \Ibexa\Contracts\Core\Repository\URLService&\PHPUnit\Framework\MockObject\MockObject */ + private URLService $urlService; - /** @var \Ibexa\Contracts\Core\SiteAccess\ConfigResolverInterface|\PHPUnit\Framework\MockObject\MockObject */ - private $configResolver; + /** @var \Ibexa\Contracts\Core\SiteAccess\ConfigResolverInterface&\PHPUnit\Framework\MockObject\MockObject */ + private ConfigResolverInterface $configResolver; protected function setUp(): void { $this->urlService = $this->createMock(URLService::class); + $this->urlService->method('createUpdateStruct')->willReturn(new URLUpdateStruct()); $this->configResolver = $this->createMock(ConfigResolverInterface::class); } @@ -165,6 +167,92 @@ public static function provideDataForTestShouldRetryWithGet(): iterable yield 'fallback disabled' => [403, 'HEAD', false, false]; } + public function testCompleteRequestRetriesWithGetWhenHeadFails(): void + { + $url = new URL(['id' => 1, 'url' => 'https://example.com']); + $queue = [new URL(['id' => 2, 'url' => 'https://example.org'])]; + + $this->urlService->expects(self::never())->method('updateUrl'); + + $next = $this->invokeCompleteRequest( + ['url' => $url, 'method' => 'HEAD'], + 403, + ['fallback_to_get' => true, 'method' => 'HEAD'], + $queue + ); + + self::assertSame(['url' => $url, 'method' => 'GET'], $next); + self::assertCount(1, $queue, 'Retry must not consume the queue'); + } + + /** + * @dataProvider provideCompleteRequestFinalResults + */ + public function testCompleteRequestRecordsStatusAndSchedulesNextUrl( + string $method, + int $statusCode, + bool $expectedValid + ): void { + $url = new URL(['id' => 1, 'url' => 'https://example.com']); + $nextUrl = new URL(['id' => 2, 'url' => 'https://example.org']); + $queue = [$nextUrl]; + + $this->urlService + ->expects(self::once()) + ->method('updateUrl') + ->with($url, self::callback(static fn ($struct): bool => $struct->isValid === $expectedValid)); + + $next = $this->invokeCompleteRequest( + ['url' => $url, 'method' => $method], + $statusCode, + ['fallback_to_get' => true, 'method' => 'HEAD'], + $queue + ); + + self::assertSame(['url' => $nextUrl, 'method' => 'HEAD'], $next); + self::assertSame([], $queue); + } + + /** + * @return iterable + */ + public static function provideCompleteRequestFinalResults(): iterable + { + yield 'HEAD succeeded' => ['HEAD', 200, true]; + yield 'GET fallback succeeded' => ['GET', 200, true]; + yield 'GET fallback failed' => ['GET', 404, false]; + } + + public function testCompleteRequestReturnsNullWhenQueueIsEmpty(): void + { + $queue = []; + + $next = $this->invokeCompleteRequest( + ['url' => new URL(['id' => 1, 'url' => 'https://example.com']), 'method' => 'GET'], + 200, + ['fallback_to_get' => true, 'method' => 'HEAD'], + $queue + ); + + self::assertNull($next); + } + + /** + * @param array{url: \Ibexa\Contracts\Core\Repository\Values\URL\URL, method: string} $request + * @param array $options + * @param \Ibexa\Contracts\Core\Repository\Values\URL\URL[] $queue + * + * @return array{url: \Ibexa\Contracts\Core\Repository\Values\URL\URL, method: string}|null + */ + private function invokeCompleteRequest(array $request, int $statusCode, array $options, array &$queue): ?array + { + return $this->invokePrivateMethod( + $this->createHandler([]), + 'completeRequest', + [$request, $statusCode, $options, &$queue] + ); + } + public function testValidateDoesNothingWhenDisabled(): void { $handler = $this->createHandler([