From 1df81839968dfa587dc04979ee20f65d2cb1eaf1 Mon Sep 17 00:00:00 2001 From: Morne Alberts Date: Fri, 17 Jul 2026 19:58:40 +0200 Subject: [PATCH] Render view error output as HTML instead of wikitext The {{#view}} error box was returned as a bare parser-function string, which MediaWiki treats as wikitext. When an error message interpolates the offending user-supplied argument and that argument holds a URL, the parser autolinks it: the URL is wrapped in an anchor, and a trailing quote is swallowed into the link and percent-encoded, corrupting the surrounding markup. Wrap the error output in the same isHTML/noparse array the success placeholder and cypher_raw already use, so the parser leaves it alone. The wrapping happens at the handle() boundary rather than inside renderError(): the internal parseArgs()/classifyArgs() plumbing distinguishes an error string from a parsed argument tuple by type, so an armoured array there would be indistinguishable from the tuple. Co-Authored-By: Claude Opus 4.8 --- src/EntryPoints/ViewParserFunction.php | 21 +++++-- .../ViewParserFunctionParsingTest.php | 56 +++++++++++++++++++ .../EntryPoints/ViewParserFunctionTest.php | 29 ++++++++-- 3 files changed, 95 insertions(+), 11 deletions(-) create mode 100644 tests/phpunit/EntryPoints/ViewParserFunctionParsingTest.php diff --git a/src/EntryPoints/ViewParserFunction.php b/src/EntryPoints/ViewParserFunction.php index 2ec611bc8..dbfbbb21d 100644 --- a/src/EntryPoints/ViewParserFunction.php +++ b/src/EntryPoints/ViewParserFunction.php @@ -25,7 +25,7 @@ public function handle( Parser $parser, string ...$args ): string|array { $parsed = $this->parseArgs( $parser, $args ); if ( is_string( $parsed ) ) { - return $parsed; + return $this->asHtml( $parsed ); } [ $explicitSubjectId, $layoutName ] = $parsed; @@ -36,11 +36,7 @@ public function handle( Parser $parser, string ...$args ): string|array { return ''; } - return [ - ViewHtmlBuilder::viewPlaceholderHtml( $resolvedSubjectId, $layoutName ), - 'noparse' => true, - 'isHTML' => true, - ]; + return $this->asHtml( ViewHtmlBuilder::viewPlaceholderHtml( $resolvedSubjectId, $layoutName ) ); } /** @@ -146,4 +142,17 @@ private function resolveMainSubjectId( Parser $parser ): ?string { return $subject?->getId()->text; } + /** + * Hands the HTML to the parser as HTML rather than wikitext. Without this the text is parsed as + * wikitext, which autolinks any URL it happens to hold: the URL is swallowed into a link and its + * trailing quote percent-encoded, corrupting the error box. The error messages echo the offending + * user-supplied argument, which can itself carry a URL, so they need this treatment. The + * subject placeholder is armoured the same way. + * + * @return array{0: string, noparse: true, isHTML: true} + */ + private function asHtml( string $html ): array { + return [ $html, 'noparse' => true, 'isHTML' => true ]; + } + } diff --git a/tests/phpunit/EntryPoints/ViewParserFunctionParsingTest.php b/tests/phpunit/EntryPoints/ViewParserFunctionParsingTest.php new file mode 100644 index 000000000..ad168717d --- /dev/null +++ b/tests/phpunit/EntryPoints/ViewParserFunctionParsingTest.php @@ -0,0 +1,56 @@ +getServiceContainer()->getParserFactory()->create(); + + $parser->setFunctionHook( + 'view', + static function ( Parser $parser, string ...$args ): string|array { + return ( new ViewParserFunction( new InMemorySubjectContentRepository() ) )->handle( $parser, ...$args ); + } + ); + + return $parser->parse( + '{{#view: Foo | "' . self::URL . '" }}', + Title::makeTitle( NS_MAIN, 'ViewParsingTest' ), + ParserOptions::newFromAnon() + )->getText(); + } + + public function testErrorUrlSurvivesParsingUnchanged(): void { + $html = html_entity_decode( $this->parseView() ); + + $this->assertStringContainsString( '"' . self::URL . '"', $html ); + } + + public function testErrorUrlIsNotTurnedIntoLink(): void { + $html = $this->parseView(); + + $this->assertStringNotContainsString( 'assertStringNotContainsString( '%22', $html ); + } + +} diff --git a/tests/phpunit/EntryPoints/ViewParserFunctionTest.php b/tests/phpunit/EntryPoints/ViewParserFunctionTest.php index ee15062ce..2b52c8588 100644 --- a/tests/phpunit/EntryPoints/ViewParserFunctionTest.php +++ b/tests/phpunit/EntryPoints/ViewParserFunctionTest.php @@ -129,6 +129,14 @@ public function testRendersErrorOnArgWithEmptyName(): void { $this->assertRendersError( $result, 'neowiki-view-error-unknown-arg', '=Finances' ); } + public function testErrorIsArmouredAgainstWikitextTransformation(): void { + $result = $this->callView( self::EXPLICIT_SUBJECT_ID, 'https://example.com' ); + + $this->assertIsArray( $result, 'Error detail echoes the offending argument, which can carry a URL, so it needs the same armour.' ); + $this->assertTrue( $result['isHTML'] ); + $this->assertTrue( $result['noparse'] ); + } + private function callView( string ...$args ): string|array { return ( new ViewParserFunction( $this->repositoryWithMainSubject() ) ) ->handle( $this->createMockParser(), ...$args ); @@ -165,7 +173,7 @@ private function assertRendersSubject( string|array $result, string $subjectId, $this->assertTrue( $result['isHTML'] ); $this->assertTrue( $result['noparse'] ); - $html = $result[0]; + $html = $this->html( $result ); $this->assertStringContainsString( 'data-mw-neowiki-subject-id="' . $subjectId . '"', $html ); if ( $layoutName === null ) { @@ -179,13 +187,24 @@ private function assertRendersSubject( string|array $result, string $subjectId, * @param string|array{0: string, noparse: true, isHTML: true} $result */ private function assertRendersError( string|array $result, string $messageKey, ?string $insertion = null ): void { - $this->assertIsString( $result, 'Expected an error HTML string; got a placeholder array.' ); - $this->assertStringContainsString( 'class="error"', $result ); - $this->assertStringContainsString( $messageKey, $result ); + $this->assertIsArray( $result, 'Expected an armoured error array; got an error string or empty string.' ); + $html = $this->html( $result ); + + $this->assertStringContainsString( 'class="error"', $html ); + $this->assertStringContainsString( $messageKey, $html ); if ( $insertion !== null ) { - $this->assertStringContainsString( $insertion, $result ); + $this->assertStringContainsString( $insertion, $html ); } } + /** + * The HTML the parser function hands back, from either its result or its error shape. + * + * @param array{0: string, noparse: true, isHTML: true} $result + */ + private function html( array $result ): string { + return $result[0]; + } + }