Skip to content

Commit ab62860

Browse files
malbertsclaude
andcommitted
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 <noreply@anthropic.com>
1 parent 7146574 commit ab62860

3 files changed

Lines changed: 93 additions & 5 deletions

File tree

src/EntryPoints/ViewParserFunction.php

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ public function handle( Parser $parser, string ...$args ): string|array {
2525
$parsed = $this->parseArgs( $parser, $args );
2626

2727
if ( is_string( $parsed ) ) {
28-
return $parsed;
28+
return $this->asHtml( $parsed );
2929
}
3030

3131
[ $explicitSubjectId, $layoutName ] = $parsed;
@@ -146,4 +146,17 @@ private function resolveMainSubjectId( Parser $parser ): ?string {
146146
return $subject?->getId()->text;
147147
}
148148

149+
/**
150+
* Hands the HTML to the parser as HTML rather than wikitext. Without this the text is parsed as
151+
* wikitext, which autolinks any URL it happens to hold: the URL is swallowed into a link and its
152+
* trailing quote percent-encoded, corrupting the error box. The error messages echo the offending
153+
* user-supplied argument, which can itself carry a URL, so they need this treatment. The
154+
* success-path placeholder is already armoured the same way.
155+
*
156+
* @return array{0: string, noparse: true, isHTML: true}
157+
*/
158+
private function asHtml( string $html ): array {
159+
return [ $html, 'noparse' => true, 'isHTML' => true ];
160+
}
161+
149162
}
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
<?php
2+
3+
declare( strict_types = 1 );
4+
5+
namespace ProfessionalWiki\NeoWiki\Tests\EntryPoints;
6+
7+
use MediaWiki\Parser\Parser;
8+
use MediaWiki\Parser\ParserOptions;
9+
use MediaWiki\Title\Title;
10+
use MediaWikiIntegrationTestCase;
11+
use ProfessionalWiki\NeoWiki\EntryPoints\ViewParserFunction;
12+
use ProfessionalWiki\NeoWiki\Tests\TestDoubles\InMemorySubjectContentRepository;
13+
14+
/**
15+
* What a reader ends up with, rather than what the parser function returns: the corruption this
16+
* guards against happens in MediaWiki's parser, after the function has handed its text back, so it
17+
* is invisible to a test that only inspects the return value.
18+
*
19+
* @covers \ProfessionalWiki\NeoWiki\EntryPoints\ViewParserFunction
20+
* @group Database
21+
*/
22+
class ViewParserFunctionParsingTest extends MediaWikiIntegrationTestCase {
23+
24+
private const string URL = 'https://example.com/x';
25+
26+
private function parseView(): string {
27+
$parser = $this->getServiceContainer()->getParserFactory()->create();
28+
29+
$parser->setFunctionHook(
30+
'view',
31+
static function ( Parser $parser, string ...$args ): string|array {
32+
return ( new ViewParserFunction( new InMemorySubjectContentRepository() ) )->handle( $parser, ...$args );
33+
}
34+
);
35+
36+
return $parser->parse(
37+
'{{#view: Foo | ' . self::URL . ' }}',
38+
Title::makeTitle( NS_MAIN, 'ViewParsingTest' ),
39+
ParserOptions::newFromAnon()
40+
)->getText();
41+
}
42+
43+
public function testErrorUrlSurvivesParsingUnchanged(): void {
44+
$html = html_entity_decode( $this->parseView() );
45+
46+
$this->assertStringContainsString( self::URL . '</div>', $html );
47+
}
48+
49+
public function testErrorUrlIsNotTurnedIntoLink(): void {
50+
$html = $this->parseView();
51+
52+
$this->assertStringNotContainsString( '<a ', $html );
53+
$this->assertStringNotContainsString( '%22', $html );
54+
}
55+
56+
}

tests/phpunit/EntryPoints/ViewParserFunctionTest.php

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,14 @@ public function testRendersErrorOnArgWithEmptyName(): void {
129129
$this->assertRendersError( $result, 'neowiki-view-error-unknown-arg', '=Finances' );
130130
}
131131

132+
public function testErrorIsArmouredAgainstWikitextTransformation(): void {
133+
$result = $this->callView( self::EXPLICIT_SUBJECT_ID, 'https://example.com' );
134+
135+
$this->assertIsArray( $result, 'Error detail echoes the offending argument, which can carry a URL, so it needs the same armour.' );
136+
$this->assertTrue( $result['isHTML'] );
137+
$this->assertTrue( $result['noparse'] );
138+
}
139+
132140
private function callView( string ...$args ): string|array {
133141
return ( new ViewParserFunction( $this->repositoryWithMainSubject() ) )
134142
->handle( $this->createMockParser(), ...$args );
@@ -179,13 +187,24 @@ private function assertRendersSubject( string|array $result, string $subjectId,
179187
* @param string|array{0: string, noparse: true, isHTML: true} $result
180188
*/
181189
private function assertRendersError( string|array $result, string $messageKey, ?string $insertion = null ): void {
182-
$this->assertIsString( $result, 'Expected an error HTML string; got a placeholder array.' );
183-
$this->assertStringContainsString( 'class="error"', $result );
184-
$this->assertStringContainsString( $messageKey, $result );
190+
$this->assertIsArray( $result, 'Expected an armoured error array; got an error string or empty string.' );
191+
$html = $this->html( $result );
192+
193+
$this->assertStringContainsString( 'class="error"', $html );
194+
$this->assertStringContainsString( $messageKey, $html );
185195

186196
if ( $insertion !== null ) {
187-
$this->assertStringContainsString( $insertion, $result );
197+
$this->assertStringContainsString( $insertion, $html );
188198
}
189199
}
190200

201+
/**
202+
* The HTML the parser function hands back, from either its result or its error shape.
203+
*
204+
* @param array{0: string, noparse: true, isHTML: true} $result
205+
*/
206+
private function html( array $result ): string {
207+
return $result[0];
208+
}
209+
191210
}

0 commit comments

Comments
 (0)