Merge "Detect and monitor against multiple Parser invocation during edit requests"

autoload.php

@@ -1085,6 +1085,7 @@ $wgAutoloadLocalClasses = [
 	'MediaWiki\\Languages\\LanguageNameUtils' => __DIR__ . '/includes/language/LanguageNameUtils.php',
 	'MediaWiki\\Parser\\ParserCacheFactory' => __DIR__ . '/includes/parser/ParserCacheFactory.php',
 	'MediaWiki\\Parser\\ParserCacheMetadata' => __DIR__ . '/includes/parser/ParserCacheMetadata.php',
+	'MediaWiki\\Parser\\ParserObserver' => __DIR__ . '/includes/parser/ParserObserver.php',
 	'MediaWiki\\Parser\\RevisionOutputCache' => __DIR__ . '/includes/parser/RevisionOutputCache.php',
 	'MediaWiki\\ProcOpenError' => __DIR__ . '/includes/exception/ProcOpenError.php',
 	'MediaWiki\\ShellDisabledError' => __DIR__ . '/includes/exception/ShellDisabledError.php',

includes/ServiceWiring.php

@@ -102,6 +102,7 @@ use MediaWiki\Page\ParserOutputAccess;
 use MediaWiki\Page\RollbackPageFactory;
 use MediaWiki\Page\WikiPageFactory;
 use MediaWiki\Parser\ParserCacheFactory;
+use MediaWiki\Parser\ParserObserver;
 use MediaWiki\Permissions\GrantsInfo;
 use MediaWiki\Permissions\GrantsLocalization;
 use MediaWiki\Permissions\GroupPermissionsLookup;
@@ -1939,6 +1940,10 @@ return [
 		);
 	},
 
+	'_ParserObserver' => static function ( MediaWikiServices $services ): ParserObserver {
+		return new ParserObserver( LoggerFactory::getInstance( 'DuplicateParse' ) );
+	},
+
 	'_SqlBlobStore' => static function ( MediaWikiServices $services ): SqlBlobStore {
 		return $services->getBlobStoreFactory()->newSqlBlobStore();
 	},

includes/content/AbstractContent.php

@@ -535,24 +535,30 @@ abstract class AbstractContent implements Content {
 			$options = ParserOptions::newCanonical( 'canonical' );
 		}
 
-		$po = new ParserOutput();
+		$output = new ParserOutput();
-		$options->registerWatcher( [ $po, 'recordOption' ] );
+		$options->registerWatcher( [ $output, 'recordOption' ] );
 
 		if ( Hooks::runner()->onContentGetParserOutput(
-			$this, $title, $revId, $options, $generateHtml, $po )
+			$this, $title, $revId, $options, $generateHtml, $output )
 		) {
 			// Save and restore the old value, just in case something is reusing
 			// the ParserOptions object in some weird way.
 			$oldRedir = $options->getRedirectTarget();
 			$options->setRedirectTarget( $this->getRedirectTarget() );
-			$this->fillParserOutput( $title, $revId, $options, $generateHtml, $po );
+			$this->fillParserOutput( $title, $revId, $options, $generateHtml, $output );
+			MediaWikiServices::getInstance()->get( '_ParserObserver' )->notifyParse(
+				$title,
+				$revId,
+				$options,
+				$output
+			);
 			$options->setRedirectTarget( $oldRedir );
 		}
 
-		Hooks::runner()->onContentAlterParserOutput( $this, $title, $po );
+		Hooks::runner()->onContentAlterParserOutput( $this, $title, $output );
 		$options->registerWatcher( null );
 
-		return $po;
+		return $output;
 	}
 
 	/**

includes/content/WikitextContent.php

@@ -25,7 +25,6 @@
  * @author Daniel Kinzler
  */
 
-use MediaWiki\Logger\LoggerFactory;
 use MediaWiki\MediaWikiServices;
 
 /**
@@ -42,11 +41,6 @@ class WikitextContent extends TextContent {
 	 */
 	private $preSaveTransformFlags = [];
 
-	/**
-	 * @var string|null Stack trace of the previous parse
-	 */
-	private $previousParseStackTrace = null;
-
 	/**
 	 * @stable to call
 	 *
@@ -300,28 +294,6 @@ class WikitextContent extends TextContent {
 	protected function fillParserOutput( Title $title, $revId,
 			ParserOptions $options, $generateHtml, ParserOutput &$output
 	) {
-		$stackTrace = ( new RuntimeException() )->getTraceAsString();
-		if ( $this->previousParseStackTrace ) {
-			// NOTE: there may be legitimate changes to re-parse the same WikiText content,
-			// e.g. if predicted revision ID for the REVISIONID magic word mismatched.
-			// But that should be rare.
-			$logger = LoggerFactory::getInstance( 'DuplicateParse' );
-			$logger->debug(
-				__METHOD__ . ': Possibly redundant parse!',
-				[
-					'title' => $title->getPrefixedDBkey(),
-					'rev' => $revId,
-					'options-hash' => $options->optionsHash(
-						ParserOptions::allCacheVaryingOptions(),
-						$title
-					),
-					'trace' => $stackTrace,
-					'previous-trace' => $this->previousParseStackTrace,
-				]
-			);
-		}
-		$this->previousParseStackTrace = $stackTrace;
-
 		list( $redir, $text ) = $this->getRedirectTargetAndText();
 		$output = MediaWikiServices::getInstance()->getParser()
 			->parse( $text, $title, $options, true, true, $revId );

includes/parser/ParserObserver.php

@@ -0,0 +1,114 @@
+<?php
+
+/**
+ * Observer to detect parser behaviors such as duplicate parses
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @since 1.38
+ *
+ * @file
+ * @ingroup Parser
+ *
+ * @author Cindy Cicalese
+ */
+
+namespace MediaWiki\Parser;
+
+use MediaWiki\Cache\CacheKeyHelper;
+use MediaWiki\Page\PageReference;
+use ParserOptions;
+use ParserOutput;
+use Psr\Log\LoggerInterface;
+use RuntimeException;
+use Title;
+
+/**
+ * For observing and detecting parser behaviors, such as duplicate parses
+ * @internal
+ * @package MediaWiki\Parser
+ */
+class ParserObserver {
+	/**
+	 * @var LoggerInterface
+	 */
+	private $logger;
+
+	/**
+	 * @var array
+	 */
+	private $previousParseStackTraces;
+
+	/**
+	 * @param LoggerInterface $logger
+	 */
+	public function __construct( LoggerInterface $logger ) {
+		$this->logger = $logger;
+		$this->previousParseStackTraces = [];
+	}
+
+	/**
+	 * @param PageReference $page
+	 * @param int|null $revId
+	 * @param ParserOptions $options
+	 * @param ParserOutput $output
+	 */
+	public function notifyParse(
+		PageReference $page, ?int $revId, ParserOptions $options, ParserOutput $output
+	) {
+		$pageKey = CacheKeyHelper::getKeyForPage( $page );
+
+		$optionsHash = $options->optionsHash(
+			$output->getUsedOptions(),
+			Title::castFromPageReference( $page )
+		);
+
+		$index = $this->getParseId( $pageKey, $revId, $optionsHash );
+
+		$stackTrace = ( new RuntimeException() )->getTraceAsString();
+		if ( array_key_exists( $index, $this->previousParseStackTraces ) ) {
+
+			// NOTE: there may be legitimate changes to re-parse the same WikiText content,
+			// e.g. if predicted revision ID for the REVISIONID magic word mismatched.
+			// But that should be rare.
+			$this->logger->debug(
+				__METHOD__ . ': Possibly redundant parse!',
+				[
+					'page' => $pageKey,
+					'rev' => $revId,
+					'options-hash' => $optionsHash,
+					'trace' => $stackTrace,
+					'previous-trace' => $this->previousParseStackTraces[$index],
+				]
+			);
+		}
+		$this->previousParseStackTraces[$index] = $stackTrace;
+	}
+
+	/**
+	 * @param string $titleStr
+	 * @param int|null $revId
+	 * @param string $optionsHash
+	 * @return string
+	 */
+	private function getParseId( string $titleStr, ?int $revId, string $optionsHash ): string {
+		// $revId may be null when previewing a new page
+		$revIdStr = $revId ?? "";
+
+		return "$titleStr.$revIdStr.$optionsHash";
+	}
+
+}

tests/phpunit/integration/includes/parser/ParserObserverIntegrationTest.php

@@ -0,0 +1,36 @@
+<?php
+
+use MediaWiki\Parser\ParserObserver;
+
+/**
+ * @covers \MediaWiki\Parser\ParserObserver
+ * @group Database
+ */
+class ParserObserverIntegrationTest extends MediaWikiIntegrationTestCase {
+
+	/**
+	 * @param bool $duplicate
+	 * @param int $count
+	 *
+	 * @dataProvider provideDuplicateParse
+	 */
+	public function testDuplicateParse( bool $duplicate, int $count ) {
+		$logger = new TestLogger( true );
+		$observer = new ParserObserver( $logger );
+		$this->setService( '_ParserObserver', $observer );
+
+		// Create a test page. Parse it twice if a duplicate is desired, or once otherwise.
+		$page = $this->getExistingTestPage();
+		$page->getContent()->getParserOutput( $page->getTitle() );
+		if ( $duplicate ) {
+			$page->getContent()->getParserOutput( $page->getTitle() );
+		}
+
+		$this->assertCount( $count, $logger->getBuffer() );
+	}
+
+	public function provideDuplicateParse() {
+		yield [ true, 1 ];
+		yield [ false, 0 ];
+	}
+}

tests/phpunit/unit/includes/parser/ParserObserverTest.php

@@ -0,0 +1,53 @@
+<?php
+
+use MediaWiki\Parser\ParserObserver;
+
+/**
+ * @covers \MediaWiki\Parser\ParserObserver
+ */
+class ParserObserverTest extends MediaWikiUnitTestCase {
+	/**
+	 * @param string $hashOne
+	 * @param string $hashTwo
+	 * @param array $expects
+	 *
+	 * @dataProvider provideDuplicateParse
+	 */
+	public function testDuplicateParse( string $hashOne, string $hashTwo, array $expects ) {
+		$logger = new TestLogger( true );
+
+		// ::makeTitle allows us to create a title without needing any services
+		$title = Title::makeTitle(
+			NS_PROJECT,
+			'Duplicate Parse Test'
+		);
+
+		$options = $this->createNoOpMock( ParserOptions::class, [ 'optionsHash' ] );
+		$options->method( 'optionsHash' )->willReturnOnConsecutiveCalls( $hashOne, $hashTwo );
+
+		$output = new ParserOutput();
+		$observer = new ParserObserver( $logger );
+		$observer->notifyParse( $title, null, $options, $output );
+		$observer->notifyParse( $title, null, $options, $output );
+
+		$this->assertArrayEquals( $expects, $logger->getBuffer() );
+	}
+
+	public function provideDuplicateParse() {
+		yield [
+			'foo',
+			'bar',
+			[]
+		];
+		yield [
+			'foo',
+			'foo',
+			[
+				[
+					'debug',
+					'MediaWiki\Parser\ParserObserver::notifyParse: Possibly redundant parse!'
+				]
+			]
+		];
+	}
+}