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

Don't use RequestContext in CommentParserFactory construction

Browse source 
Why:
* ServiceWiring.php is documented to say that "Services MUST NOT
  vary their behaviour on the global state, especially not ...
  RequestContext ... or ... "current" user"
** However, the constructor of the CommentParserFactory calls
   `RequestContext::getMain()->getLanguage()` which is in
   violation of this rule by both using the RequestContext
   and being controlled by the state of the "current" user.
* This has caused issues with premature access to the session
  user as demonstrated in T397900.
** Specifically, the call to ::getLanguage will load the request
   user's preferences and then as part of this checks if the
   user is named (which will load the User object).
* Instead of using the incorrect method of getting the user's
  language, it should instead be fetched in
  CommentParserFactory::create.
** This will also allow the Language associated with the main
   request to change without leaving the service with an
   outdated and stale version of the user's Language object.

What:
* Update CommentParserFactory to call `RequestContext::getMain()
  ->getLanguage()` in the ::create method instead of getting it
  from the constructor.
* Remove the call to `RequestContext::getMain()->getLanguage()`
  in ServiceWiring.php as no longer needed.
* Update the unit test to instead be an integration test due to
  ::create now calling code which uses the service container.

Bug: T397900
Change-Id: I36c9d8650eb5040f94626fa50f90b8026d3c3fe9
(cherry picked from commit 536f41bce51ca67733c4879d17992ee0b0db1de8)
This commit is contained in:
Dreamy Jazz 2025-08-06 20:09:40 +01:00
parent 258a26bb6f
commit 05631d4fe4
3 changed files with 6 additions and 30 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
includes/CommentFormatter/CommentParserFactory.php
View file

@ -4,6 +4,7 @@ namespace MediaWiki\CommentFormatter;
use MediaWiki\Cache\LinkBatchFactory;
use MediaWiki\Cache\LinkCache;
use MediaWiki\Context\RequestContext;
use MediaWiki\HookContainer\HookContainer;
use MediaWiki\Language\Language;
use MediaWiki\Linker\LinkRenderer;
@ -24,8 +25,6 @@ class CommentParserFactory {
/** @var RepoGroup */
private $repoGroup;
/** @var Language */
private $userLang;
/** @var Language */
private $contLang;
/** @var TitleParser */
private $titleParser;
@ -39,7 +38,6 @@ class CommentParserFactory {
* @param LinkBatchFactory $linkBatchFactory
* @param LinkCache $linkCache
* @param RepoGroup $repoGroup
* @param Language $userLang
* @param Language $contLang
* @param TitleParser $titleParser
* @param NamespaceInfo $namespaceInfo
@ -50,7 +48,6 @@ class CommentParserFactory {
LinkBatchFactory $linkBatchFactory,
LinkCache $linkCache,
RepoGroup $repoGroup,
Language $userLang,
Language $contLang,
TitleParser $titleParser,
NamespaceInfo $namespaceInfo,
@ -60,7 +57,6 @@ class CommentParserFactory {
$this->linkBatchFactory = $linkBatchFactory;
$this->linkCache = $linkCache;
$this->repoGroup = $repoGroup;
$this->userLang = $userLang;
$this->contLang = $contLang;
$this->titleParser = $titleParser;
$this->namespaceInfo = $namespaceInfo;
@ -76,7 +72,7 @@ class CommentParserFactory {
$this->linkBatchFactory,
$this->linkCache,
$this->repoGroup,
$this->userLang,
RequestContext::getMain()->getLanguage(),
$this->contLang,
$this->titleParser,
$this->namespaceInfo,

1
includes/ServiceWiring.php
View file

@ -575,7 +575,6 @@ return [
$services->getLinkBatchFactory(),
$services->getLinkCache(),
$services->getRepoGroup(),
RequestContext::getMain()->getLanguage(),
$services->getContentLanguage(),
$services->getTitleParser(),
$services->getNamespaceInfo(),

27
tests/phpunit/unit/includes/CommentFormatter/CommentParserFactoryTest.php → tests/phpunit/integration/includes/CommentFormatter/CommentParserFactoryTest.php
View file

@ -18,38 +18,19 @@
* @file
*/
namespace MediaWiki\Tests\Unit\CommentFormatter;
namespace MediaWiki\Tests\Integration\CommentFormatter;
use MediaWiki\Cache\LinkBatchFactory;
use MediaWiki\Cache\LinkCache;
use MediaWiki\CommentFormatter\CommentParser;
use MediaWiki\CommentFormatter\CommentParserFactory;
use MediaWiki\HookContainer\HookContainer;
use MediaWiki\Language\Language;
use MediaWiki\Linker\LinkRenderer;
use MediaWiki\Title\NamespaceInfo;
use MediaWiki\Title\TitleParser;
use MediaWikiUnitTestCase;
use RepoGroup;
use MediaWikiIntegrationTestCase;
/**
* @group CommentFormatter
* @covers \MediaWiki\CommentFormatter\CommentParserFactory
*/
class CommentParserFactoryTest extends MediaWikiUnitTestCase {
class CommentParserFactoryTest extends MediaWikiIntegrationTestCase {
public function testCreate() {
$factory = new CommentParserFactory(
$this->createMock( LinkRenderer::class ),
$this->createMock( LinkBatchFactory::class ),
$this->createMock( LinkCache::class ),
$this->createMock( RepoGroup::class ),
$this->createMock( Language::class ),
$this->createMock( Language::class ),
$this->createMock( TitleParser::class ),
$this->createMock( NamespaceInfo::class ),
$this->createMock( HookContainer::class )
);
$factory = $this->getServiceContainer()->getCommentParserFactory();
$this->assertInstanceOf( CommentParser::class, $factory->create() );
}