Skip to content

Commit 262d732

Browse files
committed
Keep the openssl cipher probe's warnings inside the probe
openssl_get_cipher_methods() reports algorithms openssl_cipher_iv_length() rejects on PHP 8.0-8.4 (php/php-src#19994) - 40 of 248 on PHP 8.4.23 - so the provider filters them out by probing each one, and each rejection warns. Neither @ nor Composer's Silencer settles that: both lower what is reported, and PHP calls a user error handler for a diagnostic regardless of error_reporting(), so a handler that does not consult it still sees all 40. Measured with such a handler installed: @ leaks 40, an error_reporting mask leaks 40, a handler of our own on top of the stack leaks none. Extracted as Internal\Silencer so the next call site does not have to rediscover which of the three actually works. The reported cipher list, and so the result cache key, is unchanged.
1 parent d6adcb4 commit 262d732

3 files changed

Lines changed: 81 additions & 3 deletions

File tree

src/Internal/Silencer.php

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Internal;
4+
5+
use Closure;
6+
use function restore_error_handler;
7+
use function set_error_handler;
8+
9+
/**
10+
* Runs something that predictably emits diagnostics which are none of the user's business, and keeps
11+
* them to itself.
12+
*
13+
* Not the `@` operator, and not Composer's `Silencer` either: both work by lowering what is reported,
14+
* and PHP calls a user error handler for a diagnostic regardless of `error_reporting()`. A handler that
15+
* does not consult it therefore still sees everything `@` was meant to hide, and Xdebug's `scream`
16+
* disables `@` outright. A handler of our own, on top of the stack for the duration of the call, is
17+
* called instead of theirs and reports nothing - and theirs is back in place afterwards.
18+
*
19+
* Use it only where the diagnostics are expected and meaningless, never to hide a failure that should
20+
* be handled: the callable's own return value still says whether it worked.
21+
*/
22+
final class Silencer
23+
{
24+
25+
/**
26+
* @template T
27+
* @param Closure(): T $callback
28+
* @return T
29+
*/
30+
public static function call(Closure $callback)
31+
{
32+
set_error_handler(static fn (): bool => true);
33+
34+
try {
35+
return $callback();
36+
} finally {
37+
restore_error_handler();
38+
}
39+
}
40+
41+
}

src/Type/Php/OpenSslCipherMethodsProvider.php

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use PHPStan\Analyser\ResultCache\ResultCacheMetaExtension;
66
use PHPStan\DependencyInjection\AutowiredService;
7+
use PHPStan\Internal\Silencer;
78
use function array_filter;
89
use function array_map;
910
use function array_values;
@@ -50,10 +51,14 @@ private function getSupportedCipherMethods(): array
5051
// openssl_get_cipher_methods() reports algorithms that are not actually
5152
// supported on PHP 8.0-8.4 due to https://github.com/php/php-src/issues/19994
5253
// Filter by actually testing each algorithm with openssl_cipher_iv_length().
53-
$methods = array_values(array_filter(
54+
//
55+
// Probing an unsupported algorithm warns, and @ is not enough on its own: a user error
56+
// handler that does not consult error_reporting() is still called for a suppressed
57+
// diagnostic, and whatever installs one is out of our hands. See Silencer.
58+
$methods = Silencer::call(static fn (): array => array_values(array_filter(
5459
openssl_get_cipher_methods(true),
55-
static fn (string $algorithm): bool => @openssl_cipher_iv_length($algorithm) !== false,
56-
));
60+
static fn (string $algorithm): bool => openssl_cipher_iv_length($algorithm) !== false,
61+
)));
5762
}
5863

5964
$this->supportedCipherMethods = array_map('strtolower', $methods);

tests/PHPStan/Type/Php/OpenSslCipherMethodsProviderTest.php

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
namespace PHPStan\Type\Php;
44

55
use PHPStan\Testing\PHPStanTestCase;
6+
use function restore_error_handler;
7+
use function set_error_handler;
68

79
class OpenSslCipherMethodsProviderTest extends PHPStanTestCase
810
{
@@ -28,6 +30,36 @@ public function testHashIgnoresTheOrderTheCiphersAreReportedIn(): void
2830
);
2931
}
3032

33+
/**
34+
* Reading the ciphers out of the runtime means probing each one, and on PHP 8.0-8.4
35+
* openssl_get_cipher_methods() reports algorithms openssl_cipher_iv_length() rejects with a
36+
* warning (php/php-src#19994) - 40 of 248 on PHP 8.4.23. `@` does not settle that: a user error
37+
* handler that does not consult error_reporting() is still called for a suppressed diagnostic.
38+
* See phpstan/phpstan#15176.
39+
*
40+
* Vacuous on a PHP where nothing is rejected, which is why the count is not asserted - only that
41+
* whatever the probe does stays inside it.
42+
*/
43+
public function testProbingTheRuntimeLeaksNoWarningThroughAnUnsuppressedHandler(): void
44+
{
45+
$leaked = [];
46+
set_error_handler(static function (int $errno, string $errstr) use (&$leaked): bool {
47+
// deliberately does not check error_reporting(), so the @ operator does not hide anything
48+
$leaked[] = $errstr;
49+
50+
return true;
51+
});
52+
53+
try {
54+
$hash = (new OpenSslCipherMethodsProvider())->getHash();
55+
} finally {
56+
restore_error_handler();
57+
}
58+
59+
$this->assertSame([], $leaked, 'Probing the runtime for supported ciphers must not emit warnings.');
60+
$this->assertNotSame('', $hash);
61+
}
62+
3163
/**
3264
* @param list<string> $supportedCipherMethods
3365
*/

0 commit comments

Comments
 (0)