From 21984c2c38c51e2524329cecaa50b426db5bd09e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tinjo=20Sch=C3=B6ni?= <32767367+tscni@users.noreply.github.com> Date: Fri, 23 Aug 2024 16:37:03 +0200 Subject: [PATCH 1/3] Improve curl_init() return type analysis --- src/Type/Php/CurlInitReturnTypeExtension.php | 58 ++++++++++++++++++- .../Analyser/LegacyNodeScopeResolverTest.php | 36 ++++++++++++ 2 files changed, 92 insertions(+), 2 deletions(-) diff --git a/src/Type/Php/CurlInitReturnTypeExtension.php b/src/Type/Php/CurlInitReturnTypeExtension.php index e9564278fe..3cbd39ecb9 100644 --- a/src/Type/Php/CurlInitReturnTypeExtension.php +++ b/src/Type/Php/CurlInitReturnTypeExtension.php @@ -4,17 +4,32 @@ use PhpParser\Node; use PHPStan\Analyser\Scope; +use PHPStan\Php\PhpVersion; use PHPStan\Reflection\FunctionReflection; use PHPStan\Reflection\ParametersAcceptorSelector; +use PHPStan\ShouldNotHappenException; use PHPStan\Type\Constant\ConstantBooleanType; use PHPStan\Type\DynamicFunctionReturnTypeExtension; +use PHPStan\Type\NeverType; use PHPStan\Type\Type; use PHPStan\Type\TypeCombinator; use function count; +use function is_string; +use function parse_url; +use function str_contains; +use function strcasecmp; +use function strlen; class CurlInitReturnTypeExtension implements DynamicFunctionReturnTypeExtension { + /** @see https://github.com/curl/curl/blob/curl-8_9_1/lib/urldata.h#L135 */ + private const CURL_MAX_INPUT_LENGTH = 8000000; + + public function __construct(private PhpVersion $phpVersion) + { + } + public function isFunctionSupported(FunctionReflection $functionReflection): bool { return $functionReflection->getName() === 'curl_init'; @@ -26,10 +41,49 @@ public function getTypeFromFunctionCall( Scope $scope, ): Type { - $argsCount = count($functionCall->getArgs()); + $args = $functionCall->getArgs(); + $argsCount = count($args); $returnType = ParametersAcceptorSelector::selectSingle($functionReflection->getVariants())->getReturnType(); + $notFalseReturnType = TypeCombinator::remove($returnType, new ConstantBooleanType(false)); if ($argsCount === 0) { - return TypeCombinator::remove($returnType, new ConstantBooleanType(false)); + return $notFalseReturnType; + } + + $urlArgType = $scope->getType($args[0]->value); + if ($urlArgType->isNull()->yes()) { + return $notFalseReturnType; + } + if ($urlArgType->isString()->yes()) { + $urlArgValue = $urlArgType->getConstantScalarValues()[0] ?? null; + if ($urlArgValue !== null) { + if (!is_string($urlArgValue)) { + throw new ShouldNotHappenException(); + } + if (str_contains($urlArgValue, "\0")) { + if (!$this->phpVersion->throwsValueErrorForInternalFunctions()) { + // https://github.com/php/php-src/blob/php-7.4.33/ext/curl/interface.c#L112-L115 + return new ConstantBooleanType(false); + } + // https://github.com/php/php-src/blob/php-8.0.0/ext/curl/interface.c#L104-L107 + return new NeverType(); + } + if ($this->phpVersion->getVersionId() < 80000) { + // Before PHP 8.0 an unparsable URL or a file:// scheme would fail if open_basedir is used + // Since we can't detect open_basedir properly, we'll always consider a failure possible if these + // conditions are given + // https://github.com/php/php-src/blob/php-7.4.33/ext/curl/interface.c#L139-L158 + $parsedUrlArg = parse_url($urlArgValue); + if ($parsedUrlArg === false || (isset($parsedUrlArg['scheme']) && strcasecmp($parsedUrlArg['scheme'], 'file') === 0)) { + return $returnType; + } + } + if (strlen($urlArgValue) > self::CURL_MAX_INPUT_LENGTH) { + // Since libcurl 7.65.0 this would always fail, but no current PHP version requires it at the moment + // https://github.com/curl/curl/commit/5fc28510a4664f46459d9a40187d81cc08571e60 + return $returnType; + } + return $notFalseReturnType; + } } return $returnType; diff --git a/tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php b/tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php index cf6c1c1b3b..4f9f2bc5d4 100644 --- a/tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php +++ b/tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php @@ -3087,6 +3087,42 @@ public function dataBinaryOperations(): array PHP_VERSION_ID < 80000 ? 'resource|false' : 'CurlHandle|false', 'curl_init($string)', ], + [ + PHP_VERSION_ID < 80000 ? 'resource' : 'CurlHandle', + 'curl_init(null)', + ], + [ + PHP_VERSION_ID < 80000 ? 'resource' : 'CurlHandle', + 'curl_init(\'https://phpstan.org\')', + ], + [ + PHP_VERSION_ID < 80000 ? 'false' : '*NEVER*', + "curl_init('\0')", + ], + [ + PHP_VERSION_ID < 80000 ? 'false' : '*NEVER*', + "curl_init('https://phpstan.org\0')", + ], + [ + PHP_VERSION_ID < 80000 ? 'resource' : 'CurlHandle', + 'curl_init(\'\')', + ], + [ + PHP_VERSION_ID < 80000 ? 'resource|false' : 'CurlHandle', + 'curl_init(\':\')', + ], + [ + PHP_VERSION_ID < 80000 ? 'resource|false' : 'CurlHandle', + 'curl_init(\'file://host/text.txt\')', + ], + [ + PHP_VERSION_ID < 80000 ? 'resource|false' : 'CurlHandle', + 'curl_init(\'FIle://host/text.txt\')', + ], + [ + PHP_VERSION_ID < 80000 ? 'resource' : 'CurlHandle', + 'curl_init(\'host/text.txt\')', + ], [ 'string', 'sprintf($string, $string, 1)', From 2d47da6c816bd820149cd0ced41af145e14ad341 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tinjo=20Sch=C3=B6ni?= <32767367+tscni@users.noreply.github.com> Date: Sat, 24 Aug 2024 16:37:18 +0200 Subject: [PATCH 2/3] Fix handling of multiple constant values --- src/Type/Php/CurlInitReturnTypeExtension.php | 76 +++++++++++-------- .../Analyser/LegacyNodeScopeResolverTest.php | 44 ----------- .../Php/CurlInitReturnTypeExtensionTest.php | 32 ++++++++ .../PHPStan/Type/Php/data/curl-init-php-7.php | 65 ++++++++++++++++ .../PHPStan/Type/Php/data/curl-init-php-8.php | 69 +++++++++++++++++ 5 files changed, 209 insertions(+), 77 deletions(-) create mode 100644 tests/PHPStan/Type/Php/CurlInitReturnTypeExtensionTest.php create mode 100644 tests/PHPStan/Type/Php/data/curl-init-php-7.php create mode 100644 tests/PHPStan/Type/Php/data/curl-init-php-8.php diff --git a/src/Type/Php/CurlInitReturnTypeExtension.php b/src/Type/Php/CurlInitReturnTypeExtension.php index 3cbd39ecb9..77071c82ba 100644 --- a/src/Type/Php/CurlInitReturnTypeExtension.php +++ b/src/Type/Php/CurlInitReturnTypeExtension.php @@ -11,8 +11,12 @@ use PHPStan\Type\Constant\ConstantBooleanType; use PHPStan\Type\DynamicFunctionReturnTypeExtension; use PHPStan\Type\NeverType; +use PHPStan\Type\NullType; +use PHPStan\Type\StringType; use PHPStan\Type\Type; use PHPStan\Type\TypeCombinator; +use PHPStan\Type\UnionType; +use function array_map; use function count; use function is_string; use function parse_url; @@ -50,43 +54,49 @@ public function getTypeFromFunctionCall( } $urlArgType = $scope->getType($args[0]->value); - if ($urlArgType->isNull()->yes()) { + if ($urlArgType->isConstantScalarValue()->yes() && (new UnionType([new NullType(), new StringType()]))->isSuperTypeOf($urlArgType)->yes()) { + $urlArgReturnTypes = array_map( + fn ($value) => $this->getUrlArgValueReturnType($value, $returnType, $notFalseReturnType), + $urlArgType->getConstantScalarValues(), + ); + return TypeCombinator::union(...$urlArgReturnTypes); + } + + return $returnType; + } + + private function getUrlArgValueReturnType(mixed $urlArgValue, Type $returnType, Type $notFalseReturnType): Type + { + if ($urlArgValue === null) { return $notFalseReturnType; } - if ($urlArgType->isString()->yes()) { - $urlArgValue = $urlArgType->getConstantScalarValues()[0] ?? null; - if ($urlArgValue !== null) { - if (!is_string($urlArgValue)) { - throw new ShouldNotHappenException(); - } - if (str_contains($urlArgValue, "\0")) { - if (!$this->phpVersion->throwsValueErrorForInternalFunctions()) { - // https://github.com/php/php-src/blob/php-7.4.33/ext/curl/interface.c#L112-L115 - return new ConstantBooleanType(false); - } - // https://github.com/php/php-src/blob/php-8.0.0/ext/curl/interface.c#L104-L107 - return new NeverType(); - } - if ($this->phpVersion->getVersionId() < 80000) { - // Before PHP 8.0 an unparsable URL or a file:// scheme would fail if open_basedir is used - // Since we can't detect open_basedir properly, we'll always consider a failure possible if these - // conditions are given - // https://github.com/php/php-src/blob/php-7.4.33/ext/curl/interface.c#L139-L158 - $parsedUrlArg = parse_url($urlArgValue); - if ($parsedUrlArg === false || (isset($parsedUrlArg['scheme']) && strcasecmp($parsedUrlArg['scheme'], 'file') === 0)) { - return $returnType; - } - } - if (strlen($urlArgValue) > self::CURL_MAX_INPUT_LENGTH) { - // Since libcurl 7.65.0 this would always fail, but no current PHP version requires it at the moment - // https://github.com/curl/curl/commit/5fc28510a4664f46459d9a40187d81cc08571e60 - return $returnType; - } - return $notFalseReturnType; + if (!is_string($urlArgValue)) { + throw new ShouldNotHappenException(); + } + if (str_contains($urlArgValue, "\0")) { + if (!$this->phpVersion->throwsValueErrorForInternalFunctions()) { + // https://github.com/php/php-src/blob/php-7.4.33/ext/curl/interface.c#L112-L115 + return new ConstantBooleanType(false); } + // https://github.com/php/php-src/blob/php-8.0.0/ext/curl/interface.c#L104-L107 + return new NeverType(); } - - return $returnType; + if ($this->phpVersion->getVersionId() < 80000) { + // Before PHP 8.0 an unparsable URL or a file:// scheme would fail if open_basedir is used + // Since we can't detect open_basedir properly, we'll always consider a failure possible if these + // conditions are given + // https://github.com/php/php-src/blob/php-7.4.33/ext/curl/interface.c#L139-L158 + $parsedUrlArgValue = parse_url($urlArgValue); + if ($parsedUrlArgValue === false || (isset($parsedUrlArgValue['scheme']) && strcasecmp($parsedUrlArgValue['scheme'], 'file') === 0)) { + return $returnType; + } + } + if (strlen($urlArgValue) > self::CURL_MAX_INPUT_LENGTH) { + // Since libcurl 7.65.0 this would always fail, but no current PHP version requires it at the moment + // https://github.com/curl/curl/commit/5fc28510a4664f46459d9a40187d81cc08571e60 + return $returnType; + } + return $notFalseReturnType; } } diff --git a/tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php b/tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php index 4f9f2bc5d4..4d0ca33581 100644 --- a/tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php +++ b/tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php @@ -3079,50 +3079,6 @@ public function dataBinaryOperations(): array 'bool', 'array_key_exists(\'foo\', $generalArray)', ], - [ - PHP_VERSION_ID < 80000 ? 'resource' : 'CurlHandle', - 'curl_init()', - ], - [ - PHP_VERSION_ID < 80000 ? 'resource|false' : 'CurlHandle|false', - 'curl_init($string)', - ], - [ - PHP_VERSION_ID < 80000 ? 'resource' : 'CurlHandle', - 'curl_init(null)', - ], - [ - PHP_VERSION_ID < 80000 ? 'resource' : 'CurlHandle', - 'curl_init(\'https://phpstan.org\')', - ], - [ - PHP_VERSION_ID < 80000 ? 'false' : '*NEVER*', - "curl_init('\0')", - ], - [ - PHP_VERSION_ID < 80000 ? 'false' : '*NEVER*', - "curl_init('https://phpstan.org\0')", - ], - [ - PHP_VERSION_ID < 80000 ? 'resource' : 'CurlHandle', - 'curl_init(\'\')', - ], - [ - PHP_VERSION_ID < 80000 ? 'resource|false' : 'CurlHandle', - 'curl_init(\':\')', - ], - [ - PHP_VERSION_ID < 80000 ? 'resource|false' : 'CurlHandle', - 'curl_init(\'file://host/text.txt\')', - ], - [ - PHP_VERSION_ID < 80000 ? 'resource|false' : 'CurlHandle', - 'curl_init(\'FIle://host/text.txt\')', - ], - [ - PHP_VERSION_ID < 80000 ? 'resource' : 'CurlHandle', - 'curl_init(\'host/text.txt\')', - ], [ 'string', 'sprintf($string, $string, 1)', diff --git a/tests/PHPStan/Type/Php/CurlInitReturnTypeExtensionTest.php b/tests/PHPStan/Type/Php/CurlInitReturnTypeExtensionTest.php new file mode 100644 index 0000000000..7749b327f1 --- /dev/null +++ b/tests/PHPStan/Type/Php/CurlInitReturnTypeExtensionTest.php @@ -0,0 +1,32 @@ +assertFileAsserts($assertType, $file, ...$args); + } + +} diff --git a/tests/PHPStan/Type/Php/data/curl-init-php-7.php b/tests/PHPStan/Type/Php/data/curl-init-php-7.php new file mode 100644 index 0000000000..9778b786e0 --- /dev/null +++ b/tests/PHPStan/Type/Php/data/curl-init-php-7.php @@ -0,0 +1,65 @@ + Date: Sat, 24 Aug 2024 18:24:03 +0200 Subject: [PATCH 3/3] Add PhpVersion::isCurloptUrlCheckingFileSchemeWithOpenBasedir() --- src/Php/PhpVersion.php | 8 ++++++++ src/Type/Php/CurlInitReturnTypeExtension.php | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Php/PhpVersion.php b/src/Php/PhpVersion.php index aaa08cf178..8aa5e2435b 100644 --- a/src/Php/PhpVersion.php +++ b/src/Php/PhpVersion.php @@ -327,4 +327,12 @@ public function hasDateTimeExceptions(): bool return $this->versionId >= 80300; } + public function isCurloptUrlCheckingFileSchemeWithOpenBasedir(): bool + { + // Before PHP 8.0, when setting CURLOPT_URL, an unparsable URL or a file:// scheme would fail if open_basedir is used + // https://github.com/php/php-src/blob/php-7.4.33/ext/curl/interface.c#L139-L158 + // https://github.com/php/php-src/blob/php-8.0.0/ext/curl/interface.c#L128-L130 + return $this->versionId < 80000; + } + } diff --git a/src/Type/Php/CurlInitReturnTypeExtension.php b/src/Type/Php/CurlInitReturnTypeExtension.php index 77071c82ba..813b220159 100644 --- a/src/Type/Php/CurlInitReturnTypeExtension.php +++ b/src/Type/Php/CurlInitReturnTypeExtension.php @@ -81,7 +81,7 @@ private function getUrlArgValueReturnType(mixed $urlArgValue, Type $returnType, // https://github.com/php/php-src/blob/php-8.0.0/ext/curl/interface.c#L104-L107 return new NeverType(); } - if ($this->phpVersion->getVersionId() < 80000) { + if ($this->phpVersion->isCurloptUrlCheckingFileSchemeWithOpenBasedir()) { // Before PHP 8.0 an unparsable URL or a file:// scheme would fail if open_basedir is used // Since we can't detect open_basedir properly, we'll always consider a failure possible if these // conditions are given