Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Make ParserOptionsTest work on PHP<7.4.9 with opcache enabled

Browse source 
Instead of refusing to run MediaWiki at all, just fix the test which
fails on 7.4.3.

* Add ParserOptions::clearStaticCache(), which resets static cached
  values back to their pre-hook values.
* Add accessors for these private static variables to protect against
  access before initialisation.
* Rename "inCache" to "cacheVaryingOptionsHash" for clarity.
* In ParserOptions::match(), filter static properties by detecting them
  with ReflectionProperty, instead of making a big list of them.
* Update ParserOptionsTest for the fact that stubthreshold is no longer
  a cache-varying option. This was hidden by
  ParserOptionsTest::clearCache() injecting the wrong array into
  ParserOptions.

Bug: T270228
Change-Id: I6b5ba022e1b889a10f9fbe93da63da9831504df8
This commit is contained in:
Tim Starling 2021-03-01 16:20:42 +11:00
parent 720edebbba
commit f28578f9da
3 changed files with 111 additions and 65 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
includes/PHPVersionCheck.php
View file

@ -94,7 +94,6 @@ class PHPVersionCheck {
* Remember to drop irrelevant ranges when bumping $minimumVersion.
*/
$knownBad = array(
'T270228' => '7.4.0 - 7.4.8',
'CVE-2019-11048' => '7.3.0 - 7.3.18',
);

108
includes/parser/ParserOptions.php
View file

@ -52,9 +52,15 @@ class ParserOptions {
/**
* Lazy-loaded options
* @var callable[]|null
*/
private static $lazyOptions = null;
/**
* Initial lazy-loaded options (before hook)
* @var callable[]
*/
private static $lazyOptions = [
private static $initialLazyOptions = [
'dateformat' => [ __CLASS__, 'initDateFormat' ],
'speculativeRevId' => [ __CLASS__, 'initSpeculativeRevId' ],
'speculativePageId' => [ __CLASS__, 'initSpeculativePageId' ],
@ -62,9 +68,15 @@ class ParserOptions {
/**
* Specify options that are included in the cache key
* @var array|null
*/
private static $cacheVaryingOptionsHash = null;
/**
* Initial inCacheKey options (before hook)
* @var array
*/
private static $inCacheKey = [
private static $initialCacheVaryingOptionsHash = [
'dateformat' => true,
'numberheadings' => true,
'thumbsize' => true,
@ -145,11 +157,44 @@ class ParserOptions {
* @param string $name Lazy load option without tracking usage
*/
private function lazyLoadOption( $name ) {
if ( isset( self::$lazyOptions[$name] ) && $this->options[$name] === null ) {
$this->options[$name] = call_user_func( self::$lazyOptions[$name], $this, $name );
$lazyOptions = self::getLazyOptions();
if ( isset( $lazyOptions[$name] ) && $this->options[$name] === null ) {
$this->options[$name] = call_user_func( $lazyOptions[$name], $this, $name );
}
}
/**
* Get lazy-loaded options.
*
* This array should be initialised by the constructor. The return type
* hint is used as an assertion to ensure this has happened and to coerce
* the type for static analysis.
*
* @internal Public for testing only
*
* @return array
*/
public static function getLazyOptions(): array {
return self::$lazyOptions;
}
/**
* Get cache varying options, with the name of the option in the key, and a
* boolean in the value which indicates whether the cache is indeed varied.
*
* @see self::allCacheVaryingOptions()
*
* @return array
*/
private static function getCacheVaryingOptionsHash(): array {
// Trigger a call to the 'ParserOptionsRegister' hook if it hasn't
// already been called.
if ( self::$cacheVaryingOptionsHash === null ) {
self::getDefaults();
}
return self::$cacheVaryingOptionsHash;
}
/**
* Set an option, generically
* @since 1.30
@ -1158,6 +1203,19 @@ class ParserOptions {
return $ret;
}
/**
* Reset static caches
* @internal For testing
*/
public static function clearStaticCache() {
if ( !defined( 'MW_PHPUNIT_TEST' ) ) {
throw new RuntimeException( __METHOD__ . ' is just for testing' );
}
self::$defaults = null;
self::$lazyOptions = null;
self::$cacheVaryingOptionsHash = null;
}
/**
* Get default option values
* @warning If you change the default for an existing option (unless it's
@ -1198,13 +1256,16 @@ class ParserOptions {
'speculativePageId' => null,
];
self::$cacheVaryingOptionsHash = self::$initialCacheVaryingOptionsHash;
self::$lazyOptions = self::$initialLazyOptions;
Hooks::runner()->onParserOptionsRegister(
self::$defaults,
self::$inCacheKey,
self::$cacheVaryingOptionsHash,
self::$lazyOptions
);
ksort( self::$inCacheKey );
ksort( self::$cacheVaryingOptionsHash );
}
// Unit tests depend on being able to modify the globals at will
@ -1295,16 +1356,18 @@ class ParserOptions {
}
// Compare most other fields
$fields = array_keys( get_class_vars( __CLASS__ ) );
$fields = array_diff( $fields, [
'defaults', // static
'lazyOptions', // static
'inCacheKey', // static
'callbacks', // static
'options', // Already checked above
'onAccessCallback', // only used for ParserOutput option tracking
] );
foreach ( $fields as $field ) {
foreach ( ( new ReflectionClass( $this ) )->getProperties() as $property ) {
$field = $property->getName();
if ( $property->isStatic() ) {
continue;
}
if ( in_array( $field, [
'options', // Already checked above
'onAccessCallback', // only used for ParserOutput option tracking
] ) ) {
continue;
}
if ( !is_object( $this->$field ) && $this->$field !== $other->$field ) {
return false;
}
@ -1363,12 +1426,7 @@ class ParserOptions {
* @return string[]
*/
public static function allCacheVaryingOptions() {
// Trigger a call to the 'ParserOptionsRegister' hook if it hasn't
// already been called.
if ( self::$defaults === null ) {
self::getDefaults();
}
return array_keys( array_filter( self::$inCacheKey ) );
return array_keys( array_filter( self::getCacheVaryingOptionsHash() ) );
}
/**
@ -1410,7 +1468,8 @@ class ParserOptions {
$inCacheKey = self::allCacheVaryingOptions();
// Resolve any lazy options
$lazyOpts = array_intersect( $forOptions, $inCacheKey, array_keys( self::$lazyOptions ) );
$lazyOpts = array_intersect( $forOptions,
$inCacheKey, array_keys( self::getLazyOptions() ) );
foreach ( $lazyOpts as $k ) {
$this->lazyLoadOption( $k );
}
@ -1465,9 +1524,10 @@ class ParserOptions {
*/
public function isSafeToCache( array $usedOptions = null ) {
$defaults = self::getCanonicalOverrides() + self::getDefaults();
$inCacheKey = self::getCacheVaryingOptionsHash();
$usedOptions = $usedOptions ?? array_keys( $this->options );
foreach ( $usedOptions as $option ) {
if ( empty( self::$inCacheKey[$option] ) && empty( self::$callbacks[$option] ) ) {
if ( empty( $inCacheKey[$option] ) && empty( self::$callbacks[$option] ) ) {
$v = $this->optionToString( $this->options[$option] ?? null );
$d = $this->optionToString( $defaults[$option] ?? null );
if ( $v !== $d ) {

67
tests/phpunit/includes/parser/ParserOptionsTest.php
View file

@ -2,33 +2,14 @@
use MediaWiki\MediaWikiServices;
use Wikimedia\ScopedCallback;
use Wikimedia\TestingAccessWrapper;
/**
* @covers ParserOptions
*/
class ParserOptionsTest extends MediaWikiLangTestCase {
private static function clearCache() {
$wrap = TestingAccessWrapper::newFromClass( ParserOptions::class );
$wrap->defaults = null;
$wrap->lazyOptions = [
'dateformat' => [ ParserOptions::class, 'initDateFormat' ],
'speculativeRevId' => [ ParserOptions::class, 'initSpeculativeRevId' ],
];
$wrap->inCacheKey = [
'dateformat' => true,
'numberheadings' => true,
'thumbsize' => true,
'stubthreshold' => true,
'printable' => true,
'userlang' => true,
];
}
protected function setUp() : void {
parent::setUp();
self::clearCache();
$this->setMwGlobals( [
'wgRenderHashAppend' => '',
@ -40,7 +21,7 @@ class ParserOptionsTest extends MediaWikiLangTestCase {
}
protected function tearDown() : void {
self::clearCache();
ParserOptions::clearStaticCache();
parent::tearDown();
}
@ -191,11 +172,9 @@ class ParserOptionsTest extends MediaWikiLangTestCase {
public static function provideOptionsHash() {
$used = [ 'thumbsize', 'printable' ];
$classWrapper = TestingAccessWrapper::newFromClass( ParserOptions::class );
$classWrapper->getDefaults();
$allUsableOptions = array_diff(
array_keys( $classWrapper->inCacheKey ),
array_keys( $classWrapper->lazyOptions )
ParserOptions::allCacheVaryingOptions(),
array_keys( ParserOptions::getLazyOptions() )
);
return [
@ -248,7 +227,7 @@ class ParserOptionsTest extends MediaWikiLangTestCase {
}
);
self::clearCache();
ParserOptions::clearStaticCache();
$popt = ParserOptions::newCanonical( 'canonical' );
$popt->registerWatcher( function () {
@ -279,14 +258,6 @@ class ParserOptionsTest extends MediaWikiLangTestCase {
}
public function testMatches() {
$classWrapper = TestingAccessWrapper::newFromClass( ParserOptions::class );
$oldDefaults = $classWrapper->defaults;
$oldLazy = $classWrapper->lazyOptions;
$reset = new ScopedCallback( static function () use ( $classWrapper, $oldDefaults, $oldLazy ) {
$classWrapper->defaults = $oldDefaults;
$classWrapper->lazyOptions = $oldLazy;
} );
$popt1 = ParserOptions::newCanonical( 'canonical' );
$popt2 = ParserOptions::newCanonical( 'canonical' );
$this->assertTrue( $popt1->matches( $popt2 ) );
@ -299,10 +270,16 @@ class ParserOptionsTest extends MediaWikiLangTestCase {
$this->assertFalse( $popt1->matches( $popt2 ) );
$ctr = 0;
$classWrapper->defaults += [ __METHOD__ => null ];
$classWrapper->lazyOptions += [ __METHOD__ => static function () use ( &$ctr ) {
return ++$ctr;
} ];
$this->setTemporaryHook( 'ParserOptionsRegister',
function ( &$defaults, &$inCacheKey, &$lazyOptions ) use ( &$ctr ) {
$defaults['testMatches'] = null;
$lazyOptions['testMatches'] = static function () use ( &$ctr ) {
return ++$ctr;
};
}
);
ParserOptions::clearStaticCache();
$popt1 = ParserOptions::newCanonical( 'canonical' );
$popt2 = ParserOptions::newCanonical( 'canonical' );
$this->assertFalse( $popt1->matches( $popt2 ) );
@ -310,6 +287,16 @@ class ParserOptionsTest extends MediaWikiLangTestCase {
ScopedCallback::consume( $reset );
}
/**
* This test fails if tearDown() does not call ParserOptions::clearStaticCache(),
* because the lazy option from the hook in the previous test remains active.
*/
public function testTeardownClearedCache() {
$popt1 = ParserOptions::newCanonical( 'canonical' );
$popt2 = ParserOptions::newCanonical( 'canonical' );
$this->assertTrue( $popt1->matches( $popt2 ) );
}
public function testMatchesForCacheKey() {
$this->hideDeprecated( 'ParserOptions::newCanonical with no user' );
$cOpts = ParserOptions::newCanonical( null, 'en' );
@ -340,11 +327,11 @@ class ParserOptionsTest extends MediaWikiLangTestCase {
public function testAllCacheVaryingOptions() {
$this->setTemporaryHook( 'ParserOptionsRegister', null );
$this->assertSame( [
'dateformat', 'numberheadings', 'printable', 'stubthreshold',
'dateformat', 'numberheadings', 'printable',
'thumbsize', 'userlang'
], ParserOptions::allCacheVaryingOptions() );
self::clearCache();
ParserOptions::clearStaticCache();
$this->setTemporaryHook( 'ParserOptionsRegister', static function ( &$defaults, &$inCacheKey ) {
$defaults += [
@ -358,7 +345,7 @@ class ParserOptionsTest extends MediaWikiLangTestCase {
];
} );
$this->assertSame( [
'dateformat', 'foo', 'numberheadings', 'printable', 'stubthreshold',
'dateformat', 'foo', 'numberheadings', 'printable',
'thumbsize', 'userlang'
], ParserOptions::allCacheVaryingOptions() );
}