From: daniel <dkinzler@wikimedia.org>
Date: Tue, 21 May 2019 10:16:22 +0000 (+0200)
Subject: Fix empty auto-summaries triggering a fatal error.
X-Git-Tag: 1.34.0-rc.0~1558^2
X-Git-Url: http://git.cyclocoop.org/?a=commitdiff_plain;h=0c4410a6233da6ee3e844f60b06a68f78a27ebd3;p=lhc%2Fweb%2Fwiklou.git

Fix empty auto-summaries triggering a fatal error.

Aka:  Streamline Linker::formatAutocomments() and add tests

This uses the "streamlining" for the code proposed by Thiemo
in I38edc1ad7720. I have squashed the two commits, so it now
has his code in Linker, but still has my tests as well as his.

Thiemo wrote on his patch:
This also changes the output in case there is no fragment to link to.
Before an empty `/* */` in a summary this would have created a link to
the page. I would like to argue this is not what a user expects.

Bug: T222628
Change-Id: I05408ede0e20dfd976f4057fc5baab461d2ef769
---

diff --git a/includes/Linker.php b/includes/Linker.php
index 3f50c97430..eb7a44f6b4 100644
--- a/includes/Linker.php
+++ b/includes/Linker.php
@@ -1237,18 +1237,17 @@ class Linker {
 						// that starts with "#". Before PHP 7 (and still on HHVM) substr() would
 						// return false if the start offset is the end of the string.
 						// On PHP 7+, it gracefully returns empty string instead.
-						if ( $section === false ) {
-							$section = '';
-						}
-						if ( $local ) {
-							$sectionTitle = new TitleValue( NS_MAIN, '', $section );
-						} else {
-							$sectionTitle = $title->createFragmentTarget( $section );
-						}
-						if ( $sectionTitle ) {
+						if ( $section !== '' && $section !== false ) {
+							if ( $local ) {
+								$sectionTitle = new TitleValue( NS_MAIN, '', $section );
+							} else {
+								$sectionTitle = $title->createFragmentTarget( $section );
+							}
 							$auto = Linker::makeCommentLink(
-								$sectionTitle, $wgLang->getArrow() . $wgLang->getDirMark() . $sectionText,
-								$wikiId, 'noclasses'
+								$sectionTitle,
+								$wgLang->getArrow() . $wgLang->getDirMark() . $sectionText,
+								$wikiId,
+								'noclasses'
 							);
 						}
 					}
diff --git a/tests/phpunit/includes/LinkerTest.php b/tests/phpunit/includes/LinkerTest.php
index d3523c3a54..bdbbedd76e 100644
--- a/tests/phpunit/includes/LinkerTest.php
+++ b/tests/phpunit/includes/LinkerTest.php
@@ -279,7 +279,7 @@ class LinkerTest extends MediaWikiLangTestCase {
 				"/* [[linkie?]] */",
 			],
 			[
-				'<span dir="auto"><span class="autocomment"><a href="/wiki/Special:BlankPage" title="Special:BlankPage">→‎</a>: </span> // Edit via via</span>',
+				'<span dir="auto"><span class="autocomment">: </span> // Edit via via</span>',
 				// Regression test for T222857
 				"/*  */ // Edit via via",
 			],
@@ -321,6 +321,36 @@ class LinkerTest extends MediaWikiLangTestCase {
 				"/* autocomment */",
 				null
 			],
+			[
+				'',
+				"/* */",
+				false, true
+			],
+			[
+				'',
+				"/* */",
+				null
+			],
+			[
+				'<span dir="auto"><span class="autocomment">[[</span></span>',
+				"/* [[ */",
+				false, true
+			],
+			[
+				'<span dir="auto"><span class="autocomment">[[</span></span>',
+				"/* [[ */",
+				null
+			],
+			[
+				"foo <span dir=\"auto\"><span class=\"autocomment\"><a href=\"#.23\">→‎&#91;[#_\t_]]</a></span></span>",
+				"foo /* [[#_\t_]] */",
+				false, true
+			],
+			[
+				"foo <span dir=\"auto\"><span class=\"autocomment\"><a href=\"#_.09\">#_\t_</a></span></span>",
+				"foo /* [[#_\t_]] */",
+				null
+			],
 			[
 				'<span dir="auto"><span class="autocomment"><a href="/wiki/Special:BlankPage#autocomment" title="Special:BlankPage">→‎autocomment</a></span></span>',
 				"/* autocomment */",
diff --git a/tests/phpunit/includes/title/MediaWikiTitleCodecTest.php b/tests/phpunit/includes/title/MediaWikiTitleCodecTest.php
index b8bf087a72..fdd200b6d6 100644
--- a/tests/phpunit/includes/title/MediaWikiTitleCodecTest.php
+++ b/tests/phpunit/includes/title/MediaWikiTitleCodecTest.php
@@ -336,6 +336,14 @@ class MediaWikiTitleCodecTest extends MediaWikiTestCase {
 				'X' . str_repeat( 'x', 251 ) ],
 			// Test decoding and normalization
 			[ '&quot;n&#x303;&#34;', NS_MAIN, 'en', new TitleValue( NS_MAIN, '"ñ"' ) ],
+			[ 'X#n&#x303;', NS_MAIN, 'en', new TitleValue( NS_MAIN, 'X', 'ñ' ) ],
+			// target section parsing
+			'empty fragment' => [ 'X#', NS_MAIN, 'en', new TitleValue( NS_MAIN, 'X' ) ],
+			'double hash' => [ 'X##', NS_MAIN, 'en', new TitleValue( NS_MAIN, 'X', '#' ) ],
+			'fragment with hash' => [ 'X#z#z', NS_MAIN, 'en', new TitleValue( NS_MAIN, 'X', 'z#z' ) ],
+			'fragment with space' => [ 'X#z z', NS_MAIN, 'en', new TitleValue( NS_MAIN, 'X', 'z z' ) ],
+			'fragment with percent' => [ 'X#z%z', NS_MAIN, 'en', new TitleValue( NS_MAIN, 'X', 'z%z' ) ],
+			'fragment with amp' => [ 'X#z&z', NS_MAIN, 'en', new TitleValue( NS_MAIN, 'X', 'z&z' ) ],
 		];
 	}