Move Linker::makeExternalLink() to the LinkRenderer service

Move Linker::makeExternalLink to the LinkRenderer service, as has been
done with the other static methods of Linker.

In order to allow phan's SecurityCheckPlugin to perform a more accurate
analysis of taintedness, tweak the API of Linker::makeExternalLink to
clearly indicate via the type system whether the link text has already
been escaped or not: a `string` argument will always be escaped, and
if the argument is already escaped it should be passed as an HtmlArmor
object.  In refactoring, `Message` arguments were also common, and accept
them as-is to avoid the caller having to think about whether to call
Message::text() or Message::escaped().

This allows us to provide a more precise taint type to the $text argument,
avoids an opaque boolean argument, and avoids spurious errors from
SecurityCheck.

We also require the caller to explicitly pass a Title context, instead
of implicitly relying on the global $wgTitle.  This works cleanly
everywhere except for CommentParser, which has a $selfLinkTarget which
generally works as the title context for the external link, but which
is nullable.  The original Linker::makeExternalLink() used $wgTitle as
a fallback, but $wgTitle can also be null in some circumstances.  The
title context only determines how $wgNoFollowNsExceptions is handled,
so existing code basically just ignored $wgNoFollowNsExceptions when
$wgTitle was null, which isn't terrible.  A future refactor could/should
clean up CommentParser to ensure that there is always a non-null title
context that can be used.

Change-Id: I9bcf4780f388ba639a9cc882dd9dd42eda5736ae
This commit is contained in:
C. Scott Ananian 2024-02-09 15:19:38 -05:00 committed by Bartosz Dziewoński
parent 0afc5f3280
commit b855c62f66
13 changed files with 166 additions and 101 deletions

View file

@ -239,6 +239,9 @@ because of Phabricator reports.
* SpecialBlock::getTargetAndType, deprecated since 1.36, has been removed.
* ApiQueryBlockInfoTrait::addBlockInfoToQuery(), deprecated since 1.42, has been
removed.
* Linker::makeExternalLink() has been deprecated in favor of
LinkRenderer::makeExternalLink(), which has a improved API to help phan's
SecurityCheckPlugin provide more accurate checks for escaping issues.
* Linker::makeHeadline(), Linker::generateTOC(), Linker::tocIndent(),
Linker::tocUnindent(), Linker::tocLine(), Linker::tocLineEnd(), and
Linker::tocList(), deprecated in 1.42, have been removed.

View file

@ -15,6 +15,7 @@ use MediaWiki\Linker\LinkRenderer;
use MediaWiki\Linker\LinkTarget;
use MediaWiki\Parser\Parser;
use MediaWiki\Parser\Sanitizer;
use MediaWiki\SpecialPage\SpecialPage;
use MediaWiki\Title\MalformedTitleException;
use MediaWiki\Title\NamespaceInfo;
use MediaWiki\Title\Title;
@ -260,7 +261,8 @@ class CommentParser {
$auto = $this->makeSectionLink(
$sectionTitle,
$this->userLang->getArrow() . $this->userLang->getDirMark() . $sectionText,
$wikiId
$wikiId,
$selfLinkTarget
);
}
}
@ -292,14 +294,15 @@ class CommentParser {
* @param string $text
* @param string|false|null $wikiId Id of the wiki to link to (if not the local wiki),
* as used by WikiMap.
* @param LinkTarget $contextTitle
*
* @return string HTML link
*/
private function makeSectionLink(
LinkTarget $target, $text, $wikiId
LinkTarget $target, $text, $wikiId, LinkTarget $contextTitle
) {
if ( $wikiId !== null && $wikiId !== false && !$target->isExternal() ) {
return Linker::makeExternalLink(
return $this->linkRenderer->makeExternalLink(
WikiMap::getForeignURL(
$wikiId,
$target->getNamespace() === 0
@ -308,8 +311,8 @@ class CommentParser {
':' . $target->getDBkey(),
$target->getFragment()
),
$text,
/* escape = */ false // Already escaped
new HtmlArmor( $text ), // Already escaped
$contextTitle
);
}
return $this->linkRenderer->makePreloadedLink( $target, new HtmlArmor( $text ), '' );
@ -415,7 +418,16 @@ class CommentParser {
$target = $selfLinkTarget->createFragmentTarget( $target->getFragment() );
}
$linkMarker = $this->addPageLink( $target, $linkText . $inside, $wikiId );
// We should deprecate `null` as a valid value for
// $selfLinkTarget to ensure that we can use it as
// the title context for the external link.
global $wgTitle;
$linkMarker = $this->addPageLink(
$target,
$linkText . $inside,
$wikiId,
$selfLinkTarget ?? $wgTitle ?? SpecialPage::getTitleFor( 'Badtitle' )
);
$linkMarker .= $trail;
} catch ( MalformedTitleException $e ) {
// Fall through
@ -462,12 +474,13 @@ class CommentParser {
* @param LinkTarget $target
* @param string $text
* @param string|false|null $wikiId
* @param LinkTarget $contextTitle
* @return string
*/
private function addPageLink( LinkTarget $target, $text, $wikiId ) {
private function addPageLink( LinkTarget $target, $text, $wikiId, LinkTarget $contextTitle ) {
if ( $wikiId !== null && $wikiId !== false && !$target->isExternal() ) {
// Handle links from a foreign wiki ID
return Linker::makeExternalLink(
return $this->linkRenderer->makeExternalLink(
WikiMap::getForeignURL(
$wikiId,
$target->getNamespace() === 0
@ -476,8 +489,8 @@ class CommentParser {
':' . $target->getDBkey(),
$target->getFragment()
),
$text,
/* escape = */ false // Already escaped
new HtmlArmor( $text ), // Already escaped
$contextTitle
);
} elseif ( $this->linkCache->getGoodLinkID( $target ) ||
Title::newFromLinkTarget( $target )->isAlwaysKnown()

View file

@ -20,9 +20,9 @@
namespace MediaWiki\WikiMap;
use MediaWiki\Linker\Linker;
use MediaWiki\MediaWikiServices;
use MediaWiki\Site\MediaWikiSite;
use MediaWiki\SpecialPage\SpecialPage;
use Wikimedia\Rdbms\DatabaseDomain;
/**
@ -149,6 +149,7 @@ class WikiMap {
* @return string|false HTML link or false if the wiki was not found
*/
public static function makeForeignLink( $wikiID, $page, $text = null ) {
global $wgTitle;
if ( !$text ) {
$text = $page;
}
@ -158,7 +159,12 @@ class WikiMap {
return false;
}
return Linker::makeExternalLink( $url, $text );
$linkRenderer = MediaWikiServices::getInstance()->getLinkRenderer();
return $linkRenderer->makeExternalLink(
$url,
$text,
$wgTitle ?? SpecialPage::getTitleFor( 'Badtitle' )
);
}
/**

View file

@ -27,7 +27,9 @@ use MediaWiki\Config\ServiceOptions;
use MediaWiki\HookContainer\HookContainer;
use MediaWiki\HookContainer\HookRunner;
use MediaWiki\Html\Html;
use MediaWiki\Message\Message;
use MediaWiki\Page\PageReference;
use MediaWiki\Parser\Parser;
use MediaWiki\Parser\Sanitizer;
use MediaWiki\SpecialPage\SpecialPageFactory;
use MediaWiki\Title\Title;
@ -359,6 +361,65 @@ class LinkRenderer {
return $this->buildAElement( $target, $text, $attribs, false );
}
/**
* Make an external link
*
* @since 1.43
* @param string $url URL to link to
* @param-taint $url escapes_html
* @param string|HtmlArmor|Message $text Text of link; will be escaped if
* a string.
* @param-taint $text escapes_html
* @param LinkTarget $title LinkTarget object used for title specific link attributes
* @param-taint $title none
* @param string $linktype Type of external link. Gets added to the classes
* @param-taint $linktype escapes_html
* @param array $attribs Array of extra attributes to <a>
* @param-taint $attribs escapes_html
* @return string
*/
public function makeExternalLink(
string $url, $text, LinkTarget $title, $linktype = '', $attribs = []
) {
$class = 'external';
if ( $linktype ) {
$class .= " $linktype";
}
if ( isset( $attribs['class'] ) && $attribs['class'] ) {
$class .= " {$attribs['class']}";
}
$attribs['class'] = $class;
if ( $text instanceof Message ) {
$text = $text->escaped();
} else {
$text = HtmlArmor::getHtml( $text );
// Tell phan that $text is non-null after ::getHtml()
'@phan-var string $text';
}
$newRel = Parser::getExternalLinkRel( $url, $title );
if ( !isset( $attribs['rel'] ) || $attribs['rel'] === '' ) {
$attribs['rel'] = $newRel;
} elseif ( $newRel !== null ) {
// Merge the rel attributes.
$newRels = explode( ' ', $newRel );
$oldRels = explode( ' ', $attribs['rel'] );
$combined = array_unique( array_merge( $newRels, $oldRels ) );
$attribs['rel'] = implode( ' ', $combined );
}
$link = '';
$success = $this->hookRunner->onLinkerMakeExternalLink(
$url, $text, $link, $attribs, $linktype );
if ( !$success ) {
wfDebug( "Hook LinkerMakeExternalLink changed the output of link "
. "with url {$url} and text {$text} to {$link}" );
return $link;
}
$attribs['href'] = $url;
return Html::rawElement( 'a', $attribs, $text );
}
/**
* Return the HTML for the top of a redirect page
*

View file

@ -1137,47 +1137,21 @@ class Linker {
* @param LinkTarget|null $title LinkTarget object used for title specific link attributes
* @param-taint $title none
* @return string
* @deprecated since 1.43; use LinkRenderer::makeExternalLink(), passing
* in an HtmlArmor instance if $escape was false.
*/
public static function makeExternalLink( $url, $text, $escape = true,
$linktype = '', $attribs = [], $title = null
) {
global $wgTitle;
$class = 'external';
if ( $linktype ) {
$class .= " $linktype";
}
if ( isset( $attribs['class'] ) && $attribs['class'] ) {
$class .= " {$attribs['class']}";
}
$attribs['class'] = $class;
if ( $escape ) {
$text = htmlspecialchars( $text, ENT_COMPAT );
}
if ( !$title ) {
$title = $wgTitle;
}
$newRel = Parser::getExternalLinkRel( $url, $title );
if ( !isset( $attribs['rel'] ) || $attribs['rel'] === '' ) {
$attribs['rel'] = $newRel;
} elseif ( $newRel !== null ) {
// Merge the rel attributes.
$newRels = explode( ' ', $newRel );
$oldRels = explode( ' ', $attribs['rel'] );
$combined = array_unique( array_merge( $newRels, $oldRels ) );
$attribs['rel'] = implode( ' ', $combined );
}
$link = '';
$success = ( new HookRunner( MediaWikiServices::getInstance()->getHookContainer() ) )->onLinkerMakeExternalLink(
$url, $text, $link, $attribs, $linktype );
if ( !$success ) {
wfDebug( "Hook LinkerMakeExternalLink changed the output of link "
. "with url {$url} and text {$text} to {$link}" );
return $link;
}
$attribs['href'] = $url;
return Html::rawElement( 'a', $attribs, $text );
$linkRenderer = MediaWikiServices::getInstance()->getLinkRenderer();
return $linkRenderer->makeExternalLink(
$url,
$escape ? $text : new HtmlArmor( $text ),
$title ?? $wgTitle,
$linktype,
$attribs
);
}
/**

View file

@ -1190,7 +1190,7 @@ class Article implements Page {
// This is an externally redirected view, from some other wiki.
// If it was reported from a trusted site, supply a backlink.
if ( $redirectSources && preg_match( $redirectSources, $rdfrom ) ) {
$redir = Linker::makeExternalLink( $rdfrom, $rdfrom );
$redir = $this->linkRenderer->makeExternalLink( $rdfrom, $rdfrom, $this->getTitle() );
$outputPage->addSubtitle( "<span class=\"mw-redirectedfrom\">" .
$context->msg( 'redirectedfrom' )->rawParams( $redir )->parse()
. "</span>" );

View file

@ -492,7 +492,6 @@ class ImagePage extends Article {
if ( $isMulti ) {
$linkPrev = $linkNext = '';
$count = $this->displayImg->pageCount();
$linkRenderer = MediaWikiServices::getInstance()->getLinkRenderer();
if ( !$enableLegacyMediaDOM ) {
$out->addModules( 'mediawiki.page.media' );
}
@ -501,7 +500,7 @@ class ImagePage extends Article {
$label = $context->msg( 'imgmultipageprev' )->text();
// on the client side, this link is generated in ajaxifyPageNavigation()
// in the mediawiki.page.image.pagination module
$linkPrev = $linkRenderer->makeKnownLink(
$linkPrev = $this->linkRenderer->makeKnownLink(
$this->getTitle(),
$label,
[],
@ -521,7 +520,7 @@ class ImagePage extends Article {
if ( $page < $count ) {
$label = $context->msg( 'imgmultipagenext' )->text();
$linkNext = $linkRenderer->makeKnownLink(
$linkNext = $this->linkRenderer->makeKnownLink(
$this->getTitle(),
$label,
[],
@ -820,9 +819,10 @@ EOT
$this->getFile() )
) {
// "Upload a new version of this file" link
$ulink = Linker::makeExternalLink(
$ulink = $this->linkRenderer->makeExternalLink(
$this->getUploadUrl(),
$this->getContext()->msg( 'uploadnewversion-linktext' )->text()
$this->getContext()->msg( 'uploadnewversion-linktext' ),
$this->getTitle()
);
$attrs = [ 'class' => 'plainlinks', 'id' => 'mw-imagepage-reupload-link' ];
$linkPara = Html::rawElement( 'p', $attrs, $ulink );
@ -941,8 +941,6 @@ EOT
// Sort the list by namespace:title
usort( $rows, [ $this, 'compare' ] );
$linkRenderer = MediaWikiServices::getInstance()->getLinkRenderer();
// Create links for every element
$currentCount = 0;
foreach ( $rows as $element ) {
@ -951,7 +949,7 @@ EOT
break;
}
$link = $linkRenderer->makeKnownLink(
$link = $this->linkRenderer->makeKnownLink(
Title::makeTitle( $element->page_namespace, $element->page_title ),
null,
[],
@ -975,7 +973,7 @@ EOT
break;
}
$link2 = $linkRenderer->makeKnownLink(
$link2 = $this->linkRenderer->makeKnownLink(
Title::makeTitle( $row->page_namespace, $row->page_title ) );
$li .= Html::rawElement(
'li',
@ -1025,18 +1023,19 @@ EOT
);
$out->addHTML( "<ul class='mw-imagepage-duplicates'>\n" );
$linkRenderer = MediaWikiServices::getInstance()->getLinkRenderer();
/**
* @var File $file
*/
foreach ( $dupes as $file ) {
$fromSrc = '';
if ( $file->isLocal() ) {
$link = $linkRenderer->makeKnownLink( $file->getTitle() );
$link = $this->linkRenderer->makeKnownLink( $file->getTitle() );
} else {
$link = Linker::makeExternalLink( $file->getDescriptionUrl(),
$file->getTitle()->getPrefixedText() );
$link = $this->linkRenderer->makeExternalLink(
$file->getDescriptionUrl(),
$file->getTitle()->getPrefixedText(),
$this->getTitle()
);
$fromSrc = $this->getContext()->msg(
'shared-repo-from',
$file->getRepo()->getDisplayName()

View file

@ -1850,13 +1850,12 @@ class Parser {
}
$url = wfMessage( $urlmsg, $id )->inContentLanguage()->text();
$this->addTrackingCategory( $trackingCat );
return Linker::makeExternalLink(
return $this->getLinkRenderer()->makeExternalLink(
$url,
"{$keyword} {$id}",
true,
$this->getTitle(),
$cssClass,
[],
$this->getTitle()
[]
);
} elseif ( isset( $m[6] ) && $m[6] !== ''
&& $this->mOptions->getMagicISBNLinks()
@ -1949,13 +1948,12 @@ class Parser {
$text = $this->maybeMakeExternalImage( $url );
if ( $text === false ) {
# Not an image, make a link
$text = Linker::makeExternalLink(
$text = $this->getLinkRenderer()->makeExternalLink(
$url,
$this->getTargetLanguageConverter()->markNoConversion( $url ),
true,
$this->getTitle(),
'free',
$this->getExternalLinkAttribs( $url ),
$this->getTitle()
$this->getExternalLinkAttribs( $url )
);
# Register it in the output object...
$this->mOutput->addExternalLink( $url );
@ -2248,9 +2246,14 @@ class Parser {
# This means that users can paste URLs directly into the text
# Funny characters like ö aren't valid in URLs anyway
# This was changed in August 2004
// @phan-suppress-next-line SecurityCheck-DoubleEscaped
$s .= Linker::makeExternalLink( $url, $text, false, $linktype,
$this->getExternalLinkAttribs( $url ), $this->getTitle() ) . $dtrail . $trail;
$s .= $this->getLinkRenderer()->makeExternalLink(
$url,
// @phan-suppress-next-line SecurityCheck-XSS
new HtmlArmor( $text ),
$this->getTitle(),
$linktype,
$this->getExternalLinkAttribs( $url )
) . $dtrail . $trail;
# Register link in the output object.
$this->mOutput->addExternalLink( $url );

View file

@ -4,7 +4,6 @@ namespace MediaWiki\Skin;
use HtmlArmor;
use MediaWiki\Config\Config;
use MediaWiki\Linker\Linker;
use MediaWiki\MainConfigNames;
use MediaWiki\MediaWikiServices;
use MediaWiki\Title\Title;
@ -72,8 +71,11 @@ class SkinComponentCopyright implements SkinComponent {
new HtmlArmor( $config->get( MainConfigNames::RightsText ) ?: $title->getText() )
);
} elseif ( $config->get( MainConfigNames::RightsUrl ) ) {
$link = Linker::makeExternalLink( $config->get( MainConfigNames::RightsUrl ),
$config->get( MainConfigNames::RightsText ) );
$link = $linkRenderer->makeExternalLink(
$config->get( MainConfigNames::RightsUrl ),
$config->get( MainConfigNames::RightsText ),
$title
);
} elseif ( $config->get( MainConfigNames::RightsText ) ) {
$link = $config->get( MainConfigNames::RightsText );
} else {

View file

@ -23,7 +23,6 @@ namespace MediaWiki\Specials;
use MediaWiki\Cache\LinkBatchFactory;
use MediaWiki\ExternalLinks\LinkFilter;
use MediaWiki\HTMLForm\HTMLForm;
use MediaWiki\Linker\Linker;
use MediaWiki\MainConfigNames;
use MediaWiki\Parser\Parser;
use MediaWiki\SpecialPage\QueryPage;
@ -247,7 +246,7 @@ class SpecialLinkSearch extends QueryPage {
$pageLink = $this->getLinkRenderer()->makeLink( $title );
$url = LinkFilter::reverseIndexes( $result->urldomain ) . $result->urlpath;
$urlLink = Linker::makeExternalLink( $url, $url );
$urlLink = $this->getLinkRenderer()->makeExternalLink( $url, $url, $this->getFullTitle() );
return $this->msg( 'linksearch-line' )->rawParams( $urlLink, $pageLink )->escaped();
}

View file

@ -30,7 +30,6 @@ use MediaWiki\Config\Config;
use MediaWiki\HookContainer\HookRunner;
use MediaWiki\Html\Html;
use MediaWiki\Language\RawMessage;
use MediaWiki\Linker\Linker;
use MediaWiki\MainConfigNames;
use MediaWiki\MediaWikiServices;
use MediaWiki\Message\Message;
@ -748,9 +747,11 @@ class SpecialVersion extends SpecialPage {
. Html::rawElement(
'td',
[],
Linker::makeExternalLink(
"https://packagist.org/packages/$name", $name,
true, '',
$this->getLinkRenderer()->makeExternalLink(
"https://packagist.org/packages/$name",
$name,
$this->getFullTitle(),
'',
[ 'class' => 'mw-version-library-name' ]
)
)
@ -830,9 +831,11 @@ class SpecialVersion extends SpecialPage {
. Html::rawElement(
'td',
[],
Linker::makeExternalLink(
$info['homepage'], $info['name'],
true, '',
$this->getLinkRenderer()->makeExternalLink(
$info['homepage'],
$info['name'],
$this->getFullTitle(),
'',
[ 'class' => 'mw-version-library-name' ]
)
)
@ -867,9 +870,10 @@ class SpecialVersion extends SpecialPage {
Html::rawElement(
'span',
[ 'class' => 'plainlinks' ],
Linker::makeExternalLink(
$this->getLinkRenderer()->makeExternalLink(
'https://www.mediawiki.org/wiki/Special:MyLanguage/Manual:Tag_extensions',
$this->msg( 'version-parser-extensiontags' )->text()
$this->msg( 'version-parser-extensiontags' ),
$this->getFullTitle()
)
)
);
@ -910,9 +914,10 @@ class SpecialVersion extends SpecialPage {
Html::rawElement(
'span',
[ 'class' => 'plainlinks' ],
Linker::makeExternalLink(
$this->getLinkRenderer()->makeExternalLink(
'https://www.mediawiki.org/wiki/Special:MyLanguage/Manual:Parser_functions',
$this->msg( 'version-parser-function-hooks' )->text()
$this->msg( 'version-parser-function-hooks' ),
$this->getFullTitle()
)
)
);
@ -1010,19 +1015,19 @@ class SpecialVersion extends SpecialPage {
// ... such as extension names and links
if ( isset( $extension['namemsg'] ) ) {
// Localized name of extension
$extensionName = $this->msg( $extension['namemsg'] )->text();
$extensionName = $this->msg( $extension['namemsg'] );
} elseif ( isset( $extension['name'] ) ) {
// Non localized version
$extensionName = $extension['name'];
} else {
$extensionName = $this->msg( 'version-no-ext-name' )->text();
$extensionName = $this->msg( 'version-no-ext-name' );
}
if ( isset( $extension['url'] ) ) {
$extensionNameLink = Linker::makeExternalLink(
$extensionNameLink = $this->getLinkRenderer()->makeExternalLink(
$extension['url'],
$extensionName,
true,
$this->getFullTitle(),
'',
[ 'class' => 'mw-version-ext-name' ]
);
@ -1081,10 +1086,10 @@ class SpecialVersion extends SpecialPage {
if ( $vcsVersion ) {
if ( $vcsLink ) {
$vcsVerString = Linker::makeExternalLink(
$vcsVerString = $this->getLinkRenderer()->makeExternalLink(
$vcsLink,
$this->msg( 'version-version', $vcsVersion )->text(),
true,
$this->msg( 'version-version', $vcsVersion ),
$this->getFullTitle(),
'',
[ 'class' => 'mw-version-ext-vcs-version' ]
);

View file

@ -27,7 +27,6 @@ use MediaWiki\Context\IContextSource;
use MediaWiki\Html\FormOptions;
use MediaWiki\Html\Html;
use MediaWiki\Languages\LanguageFactory;
use MediaWiki\Linker\Linker;
use MediaWiki\Linker\LinkRenderer;
use MediaWiki\MediaWikiServices;
use MediaWiki\Parser\Sanitizer;
@ -292,7 +291,7 @@ class AllMessagesTablePager extends TablePager {
$title = Title::makeTitle( NS_MEDIAWIKI, $value . $this->suffix );
$talk = Title::makeTitle( NS_MEDIAWIKI_TALK, $value . $this->suffix );
$message = $this->msg( $value )->inLanguage( $this->lang )->useDatabase( false )->plain();
$translation = Linker::makeExternalLink(
$translation = $linkRenderer->makeExternalLink(
'https://translatewiki.net/w/i.php?' . wfArrayToCgi( [
'title' => 'Special:SearchTranslations',
'group' => 'mediawiki',
@ -300,7 +299,8 @@ class AllMessagesTablePager extends TablePager {
'language' => $this->lang->getCode(),
'query' => $value . ' ' . $message
] ),
$this->msg( 'allmessages-filter-translate' )->text()
$this->msg( 'allmessages-filter-translate' ),
$this->getTitle()
);
$talkLink = $this->msg( 'talkpagelinktext' )->text();

View file

@ -321,7 +321,7 @@ External links: Free with trailing quotes (T113666)
news:'a'b''c''d e
!! html/php
<p><b>News:</b> Stuff here
</p><p><a rel="nofollow" class="external free" href="news:&#39;a&#39;b">news:'a'b</a><i>c</i>d e
</p><p><a rel="nofollow" class="external free" href="news:&#39;a&#39;b">news:&#39;a&#39;b</a><i>c</i>d e
</p>
!! html/parsoid
<p><b>News:</b> Stuff here</p>