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

Move serializability validation from ParserOutput to ParserCache

Browse source 
Bug: T263579
Change-Id: Iac2dbc817c2e7af4a6d112f01bd380a04354db22
This commit is contained in:
Petr Pchelko 2020-10-14 07:54:56 -07:00
parent 7702eafcd8
commit 09c14b9dd0
3 changed files with 82 additions and 36 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

44
includes/parser/ParserCache.php
View file

@ -482,16 +482,8 @@ class ParserCache {
$msg = "Saved in parser cache with key $parserOutputKey" .
" and timestamp $cacheTime" .
" and revision id $revId";
$parserOutput->addCacheMessage( $msg );
$this->logger->debug( 'Saved in parser cache', [
'name' => $this->name,
'key' => $parserOutputKey,
'cache_time' => $cacheTime,
'rev_id' => $revId
] );
$pageKey = $this->makeMetadataKey( $wikiPage );
if ( $this->writeJson ) {
@ -517,6 +509,13 @@ class ParserCache {
$this->hookRunner->onParserCacheSaveComplete(
$this, $parserOutput, $wikiPage->getTitle(), $popts, $revId );
$this->logger->debug( 'Saved in parser cache', [
'name' => $this->name,
'key' => $parserOutputKey,
'cache_time' => $cacheTime,
'rev_id' => $revId
] );
}
} elseif ( $expire <= 0 ) {
$this->logger->debug(
@ -606,15 +605,32 @@ class ParserCache {
*/
private function encodeAsJson( CacheTime $obj, string $key ) {
$data = $obj->jsonSerialize();
$json = FormatJson::encode( $data );
$json = FormatJson::encode( $data );
if ( !$json ) {
$this->logger->error(
"JSON encoding failed!",
[ 'cache_key' => $key, 'json_error' => json_last_error(), ]
);
$this->logger->error( "JSON encoding failed", [
'name' => $this->name,
'cache_key' => $key,
'json_error' => json_last_error(),
] );
return null;
}
return $json ?: null;
// Detect if the array contained any properties non-serializable
// to json. We will not be able to deserialize the value correctly
// anyway, so return null. This is done after calling FormatJson::encode
// to avoid walking over circular structures.
$unserializablePath = FormatJson::detectNonSerializableData( $data );
if ( $unserializablePath ) {
$objClass = get_class( $obj );
$this->logger->error( "Non-serializable {$objClass} property set", [
'name' => $this->name,
'cache_key' => $key,
'path' => $unserializablePath,
] );
return null;
}
return $json;
}
}

29
includes/parser/ParserOutput.php
View file

@ -1140,20 +1140,15 @@ class ParserOutput extends CacheTime {
* @code
* $parser->getOutput()->my_ext_foo = '...';
* @endcode
*
* @note Only scalar values, e.g. numbers, strings, arrays are supported
* as a value. Attempt to set a class instance as a page property will
* break ParserCache for the page.
*
* @param string $name
* @param mixed $value
*/
public function setProperty( $name, $value ) {
$unserializablePath = FormatJson::detectNonSerializableData( $value );
if ( $unserializablePath ) {
LoggerFactory::getInstance( 'ParserOutput' )->warning(
'Non-serializable page property set',
[
'name' => $name,
'path' => $unserializablePath,
]
);
}
$this->mProperties[$name] = $value;
}
@ -1241,6 +1236,10 @@ class ParserOutput extends CacheTime {
* $parser->getOutput()->my_ext_foo = '...';
* @endcode
*
* @note Only scalar values, e.g. numbers, strings, arrays are supported
* as a value. Attempt to set a class instance as a extension data will
* break ParserCache for the page.
*
* @since 1.21
*
* @param string $key The key for accessing the data. Extensions should take care to avoid
@ -1253,16 +1252,6 @@ class ParserOutput extends CacheTime {
if ( $value === null ) {
unset( $this->mExtensionData[$key] );
} else {
$unserializablePath = FormatJson::detectNonSerializableData( $value );
if ( $unserializablePath ) {
LoggerFactory::getInstance( 'ParserOutput' )->warning(
'Non-serializable extension data set',
[
'key' => $key,
'path' => $unserializablePath,
]
);
}
$this->mExtensionData[$key] = $value;
}
}

45
tests/phpunit/includes/parser/ParserCacheTest.php
View file

@ -12,8 +12,12 @@ use NullStatsdDataFactory;
use ParserCache;
use ParserOptions;
use ParserOutput;
use Psr\Log\LoggerInterface;
use Psr\Log\LogLevel;
use Psr\Log\NullLogger;
use TestLogger;
use Title;
use User;
use Wikimedia\TestingAccessWrapper;
use WikiPage;
@ -62,11 +66,13 @@ class ParserCacheTest extends MediaWikiIntegrationTestCase {
/**
* @param HookContainer|null $hookContainer
* @param BagOStuff|null $storage
* @param LoggerInterface|null $logger
* @return ParserCache
*/
private function createParserCache(
HookContainer $hookContainer = null,
BagOStuff $storage = null
BagOStuff $storage = null,
LoggerInterface $logger = null
): ParserCache {
return new ParserCache(
'test',
@ -74,7 +80,7 @@ class ParserCacheTest extends MediaWikiIntegrationTestCase {
'19900220000000',
$hookContainer ?: $this->createHookContainer( [] ),
new NullStatsdDataFactory(),
new NullLogger()
$logger ?: new NullLogger()
);
}
@ -622,4 +628,39 @@ class ParserCacheTest extends MediaWikiIntegrationTestCase {
$this->assertEquals( $parserOutput2, $cachedOutput );
}
/**
* @covers ParserCache::encodeAsJson
*/
public function testNonSerializableJsonIsReported() {
$testLogger = new TestLogger( true );
$cache = $this->createParserCache( null, null, $testLogger );
$cache->setJsonSupport( true, true );
$parserOutput = $this->createDummyParserOutput();
$parserOutput->setExtensionData( 'test', new User() );
$cache->save( $parserOutput, $this->page, ParserOptions::newFromAnon() );
$this->assertArrayEquals(
[ [ LogLevel::ERROR, 'Non-serializable ParserOutput property set' ] ],
$testLogger->getBuffer()
);
}
/**
* @covers ParserCache::encodeAsJson
*/
public function testCyclicStructuresDoNotBlowUpInJson() {
$testLogger = new TestLogger( true );
$cache = $this->createParserCache( null, null, $testLogger );
$cache->setJsonSupport( true, true );
$parserOutput = $this->createDummyParserOutput();
$cyclicArray = [ 'a' => 'b' ];
$cyclicArray['c'] = &$cyclicArray;
$parserOutput->setExtensionData( 'test', $cyclicArray );
$cache->save( $parserOutput, $this->page, ParserOptions::newFromAnon() );
$this->assertArrayEquals(
[ [ LogLevel::ERROR, 'JSON encoding failed' ] ],
$testLogger->getBuffer()
);
}
}