From: jenkins-bot Date: Thu, 6 Apr 2017 23:30:01 +0000 (+0000) Subject: Merge "phpunit: Avoid use of deprecated getMock for PHPUnit 5 compat" X-Git-Tag: 1.31.0-rc.0~3569 X-Git-Url: https://git.heureux-cyclage.org/?p=lhc%2Fweb%2Fwiklou.git;a=commitdiff_plain;h=424251a2cb5842727756d96f877c787c443ea056;hp=447ce7e39ab22806cbdea1244d77480c5753dad2 Merge "phpunit: Avoid use of deprecated getMock for PHPUnit 5 compat" --- diff --git a/RELEASE-NOTES-1.29 b/RELEASE-NOTES-1.29 index 3d3f9ca468..4b7de886a9 100644 --- a/RELEASE-NOTES-1.29 +++ b/RELEASE-NOTES-1.29 @@ -35,6 +35,8 @@ production. * (T156983) $wgRateLimitsExcludedIPs now accepts CIDR ranges as well as single IPs. * $wgDummyLanguageCodes is deprecated. Additional language code mappings may be added to $wgExtraLanguageCodes instead. +* (T161453) LocalisationCache will no longer use the temporary directory in it's + fallback chain when trying to work out where to write the cache. === New features in 1.29 === * (T5233) A cookie can now be set when a user is autoblocked, to track that user @@ -86,6 +88,23 @@ production. * (T157035) "new mw.Uri()" was ignoring options when using default URI. * Special:Allpages can no longer be filtered by redirect in miser mode. * (T160519) CACHE_ANYTHING will not be CACHE_ACCEL if no accelerator is installed. +* (T109140) (T122209) SECURITY: Special:UserLogin and Special:Search allow redirect + to interwiki links. +* (T144845) SECURITY: XSS in SearchHighlighter::highlightText() when + $wgAdvancedSearchHighlighting is true. +* (T125177) SECURITY: API parameters may now be marked as "sensitive" to keep + their values out of the logs. +* (T150044) SECURITY: "Mark all pages visited" on the watchlist now requires a CSRF + token. +* (T156184) SECURITY: Escape content model/format url parameter in message. +* (T151735) SECURITY: SVG filter evasion using default attribute values in DTD + declaration. +* (T161453) SECURITY: LocalisationCache will no longer use the temporary directory + in it's fallback chain when trying to work out where to write the cache. +* (T48143) SECURITY: Spam blacklist ineffective on encoded URLs inside file inclusion + syntax's link parameter. +* (T108138) SECURITY: Sysops can undelete pages, although the page is protected against + it. === Action API changes in 1.29 === * Submitting sensitive authentication request parameters to action=login, @@ -146,6 +165,8 @@ production. various methods now take a module path rather than a module name. * ApiMessageTrait::getApiCode() now strips 'apierror-' and 'apiwarn-' prefixes from the message key, and maps some message keys for backwards compatibility. +* API parameters may now be marked as "sensitive" to keep their values out of + the logs. === Languages updated in 1.29 === diff --git a/autoload.php b/autoload.php index 21f75d12cf..3816485089 100644 --- a/autoload.php +++ b/autoload.php @@ -1340,6 +1340,7 @@ $wgAutoloadLocalClasses = [ 'SpecialExpandTemplates' => __DIR__ . '/includes/specials/SpecialExpandTemplates.php', 'SpecialExport' => __DIR__ . '/includes/specials/SpecialExport.php', 'SpecialFilepath' => __DIR__ . '/includes/specials/SpecialFilepath.php', + 'SpecialGoToInterwiki' => __DIR__ . '/includes/specials/SpecialGoToInterwiki.php', 'SpecialImport' => __DIR__ . '/includes/specials/SpecialImport.php', 'SpecialJavaScriptTest' => __DIR__ . '/includes/specials/SpecialJavaScriptTest.php', 'SpecialLinkAccounts' => __DIR__ . '/includes/specials/SpecialLinkAccounts.php', diff --git a/includes/CategoryViewer.php b/includes/CategoryViewer.php index 31369b0187..0205d708ca 100644 --- a/includes/CategoryViewer.php +++ b/includes/CategoryViewer.php @@ -742,7 +742,13 @@ class CategoryViewer extends ContextSource { $totalcnt = $rescnt; $category = $this->cat; DeferredUpdates::addCallableUpdate( function () use ( $category ) { - $category->refreshCounts(); + # Avoid excess contention on the same category (T162121) + $dbw = wfGetDB( DB_MASTER ); + $name = __METHOD__ . ':' . md5( $this->mName ); + $scopedLock = $dbw->getScopedLockAndFlush( $name, __METHOD__, 1 ); + if ( $scopedLock ) { + $category->refreshCounts(); + } } ); } else { // Case 3: hopeless. Don't give a total count at all. diff --git a/includes/EditPage.php b/includes/EditPage.php index e4d217c979..2153b8c02b 100644 --- a/includes/EditPage.php +++ b/includes/EditPage.php @@ -1027,7 +1027,7 @@ class EditPage { throw new ErrorPageError( 'editpage-invalidcontentmodel-title', 'editpage-invalidcontentmodel-text', - [ $this->contentModel ] + [ wfEscapeWikiText( $this->contentModel ) ] ); } @@ -1035,7 +1035,10 @@ class EditPage { throw new ErrorPageError( 'editpage-notsupportedcontentformat-title', 'editpage-notsupportedcontentformat-text', - [ $this->contentFormat, ContentHandler::getLocalizedName( $this->contentModel ) ] + [ + wfEscapeWikiText( $this->contentFormat ), + wfEscapeWikiText( ContentHandler::getLocalizedName( $this->contentModel ) ) + ] ); } diff --git a/includes/OutputPage.php b/includes/OutputPage.php index d3e137330d..53802647b5 100644 --- a/includes/OutputPage.php +++ b/includes/OutputPage.php @@ -2727,7 +2727,9 @@ class OutputPage extends ContextSource { } else { $titleObj = Title::newFromText( $returnto ); } - if ( !is_object( $titleObj ) ) { + // We don't want people to return to external interwiki. That + // might potentially be used as part of a phishing scheme + if ( !is_object( $titleObj ) || $titleObj->isExternal() ) { $titleObj = Title::newMainPage(); } diff --git a/includes/Title.php b/includes/Title.php index f16f0c5311..0db4094f7f 100644 --- a/includes/Title.php +++ b/includes/Title.php @@ -1683,6 +1683,33 @@ class Title implements LinkTarget { return $url; } + /** + * Get a url appropriate for making redirects based on an untrusted url arg + * + * This is basically the same as getFullUrl(), but in the case of external + * interwikis, we send the user to a landing page, to prevent possible + * phishing attacks and the like. + * + * @note Uses current protocol by default, since technically relative urls + * aren't allowed in redirects per HTTP spec, so this is not suitable for + * places where the url gets cached, as might pollute between + * https and non-https users. + * @see self::getLocalURL for the arguments. + * @param array|string $query + * @param string $proto Protocol type to use in URL + * @return String. A url suitable to use in an HTTP location header. + */ + public function getFullUrlForRedirect( $query = '', $proto = PROTO_CURRENT ) { + $target = $this; + if ( $this->isExternal() ) { + $target = SpecialPage::getTitleFor( + 'GoToInterwiki', + $this->getPrefixedDBKey() + ); + } + return $target->getFullUrl( $query, false, $proto ); + } + /** * Get a URL with no fragment or server name (relative URL) from a Title object. * If this page is generated with action=render, however, @@ -2289,6 +2316,17 @@ class Title implements LinkTarget { ) { $errors[] = [ 'delete-toobig', $wgLang->formatNum( $wgDeleteRevisionsLimit ) ]; } + } elseif ( $action === 'undelete' ) { + if ( count( $this->getUserPermissionsErrorsInternal( 'edit', $user, $rigor, true ) ) ) { + // Undeleting implies editing + $errors[] = [ 'undelete-cantedit' ]; + } + if ( !$this->exists() + && count( $this->getUserPermissionsErrorsInternal( 'create', $user, $rigor, true ) ) + ) { + // Undeleting where nothing currently exists implies creating + $errors[] = [ 'undelete-cantcreate' ]; + } } return $errors; } diff --git a/includes/api/ApiAuthManagerHelper.php b/includes/api/ApiAuthManagerHelper.php index d037c365bd..8862cc7f9f 100644 --- a/includes/api/ApiAuthManagerHelper.php +++ b/includes/api/ApiAuthManagerHelper.php @@ -169,6 +169,7 @@ class ApiAuthManagerHelper { $this->module->getMain()->markParamsUsed( array_keys( $data ) ); if ( $sensitive ) { + $this->module->getMain()->markParamsSensitive( array_keys( $sensitive ) ); $this->module->requirePostedParameters( array_keys( $sensitive ), 'noprefix' ); } diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php index fec4234cf0..b698ceffbc 100644 --- a/includes/api/ApiBase.php +++ b/includes/api/ApiBase.php @@ -188,6 +188,13 @@ abstract class ApiBase extends ContextSource { */ const PARAM_EXTRA_NAMESPACES = 18; + /* + * (boolean) Is the parameter sensitive? Note 'password'-type fields are + * always sensitive regardless of the value of this field. + * @since 1.29 + */ + const PARAM_SENSITIVE = 19; + /**@}*/ const ALL_DEFAULT_STRING = '*'; @@ -1025,6 +1032,10 @@ abstract class ApiBase extends ContextSource { } else { $type = 'NULL'; // allow everything } + + if ( $type == 'password' || !empty( $paramSettings[self::PARAM_SENSITIVE] ) ) { + $this->getMain()->markParamsSensitive( $encParamName ); + } } if ( $type == 'boolean' ) { @@ -2030,6 +2041,7 @@ abstract class ApiBase extends ContextSource { $params['token'] = [ ApiBase::PARAM_TYPE => 'string', ApiBase::PARAM_REQUIRED => true, + ApiBase::PARAM_SENSITIVE => true, ApiBase::PARAM_HELP_MSG => [ 'api-help-param-token', $this->needsToken(), diff --git a/includes/api/ApiCheckToken.php b/includes/api/ApiCheckToken.php index 3cc7a8a058..480915e60c 100644 --- a/includes/api/ApiCheckToken.php +++ b/includes/api/ApiCheckToken.php @@ -73,6 +73,7 @@ class ApiCheckToken extends ApiBase { 'token' => [ ApiBase::PARAM_TYPE => 'string', ApiBase::PARAM_REQUIRED => true, + ApiBase::PARAM_SENSITIVE => true, ], 'maxtokenage' => [ ApiBase::PARAM_TYPE => 'integer', diff --git a/includes/api/ApiLogin.php b/includes/api/ApiLogin.php index e3513da80d..41bec355a7 100644 --- a/includes/api/ApiLogin.php +++ b/includes/api/ApiLogin.php @@ -250,6 +250,7 @@ class ApiLogin extends ApiBase { 'token' => [ ApiBase::PARAM_TYPE => 'string', ApiBase::PARAM_REQUIRED => false, // for BC + ApiBase::PARAM_SENSITIVE => true, ApiBase::PARAM_HELP_MSG => [ 'api-help-param-token', 'login' ], ], ]; diff --git a/includes/api/ApiMain.php b/includes/api/ApiMain.php index a1fac0cc3d..b5eff7056e 100644 --- a/includes/api/ApiMain.php +++ b/includes/api/ApiMain.php @@ -161,6 +161,7 @@ class ApiMain extends ApiBase { private $mCacheMode = 'private'; private $mCacheControl = []; private $mParamsUsed = []; + private $mParamsSensitive = []; /** @var bool|null Cached return value from self::lacksSameOriginSecurity() */ private $lacksSameOriginSecurity = null; @@ -1602,13 +1603,17 @@ class ApiMain extends ApiBase { " {$logCtx['ip']} " . "T={$logCtx['timeSpentBackend']}ms"; + $sensitive = array_flip( $this->getSensitiveParams() ); foreach ( $this->getParamsUsed() as $name ) { $value = $request->getVal( $name ); if ( $value === null ) { continue; } - if ( strlen( $value ) > 256 ) { + if ( isset( $sensitive[$name] ) ) { + $value = '[redacted]'; + $encValue = '[redacted]'; + } elseif ( strlen( $value ) > 256 ) { $value = substr( $value, 0, 256 ); $encValue = $this->encodeRequestLogValue( $value ) . '[...]'; } else { @@ -1658,6 +1663,24 @@ class ApiMain extends ApiBase { $this->mParamsUsed += array_fill_keys( (array)$params, true ); } + /** + * Get the request parameters that should be considered sensitive + * @since 1.29 + * @return array + */ + protected function getSensitiveParams() { + return array_keys( $this->mParamsSensitive ); + } + + /** + * Mark parameters as sensitive + * @since 1.29 + * @param string|string[] $params + */ + public function markParamsSensitive( $params ) { + $this->mParamsSensitive += array_fill_keys( (array)$params, true ); + } + /** * Get a request value, and register the fact that it was used, for logging. * @param string $name diff --git a/includes/api/ApiQueryDeletedrevs.php b/includes/api/ApiQueryDeletedrevs.php index 2bb4d03efa..b68a8682c5 100644 --- a/includes/api/ApiQueryDeletedrevs.php +++ b/includes/api/ApiQueryDeletedrevs.php @@ -250,7 +250,7 @@ class ApiQueryDeletedrevs extends ApiQueryBase { $this->addOption( 'LIMIT', $limit + 1 ); $this->addOption( 'USE INDEX', - [ 'archive' => ( $mode == 'user' ? 'usertext_timestamp' : 'name_title_timestamp' ) ] + [ 'archive' => ( $mode == 'user' ? 'ar_usertext_timestamp' : 'name_title_timestamp' ) ] ); if ( $mode == 'all' ) { if ( $params['unique'] ) { diff --git a/includes/api/ApiQueryWatchlist.php b/includes/api/ApiQueryWatchlist.php index fee0b78c29..f8f6e7d8a1 100644 --- a/includes/api/ApiQueryWatchlist.php +++ b/includes/api/ApiQueryWatchlist.php @@ -475,7 +475,8 @@ class ApiQueryWatchlist extends ApiQueryGeneratorBase { ApiBase::PARAM_TYPE => 'user' ], 'token' => [ - ApiBase::PARAM_TYPE => 'string' + ApiBase::PARAM_TYPE => 'string', + ApiBase::PARAM_SENSITIVE => true, ], 'continue' => [ ApiBase::PARAM_HELP_MSG => 'api-help-param-continue', diff --git a/includes/api/ApiQueryWatchlistRaw.php b/includes/api/ApiQueryWatchlistRaw.php index 116f21928d..b0b1cde92b 100644 --- a/includes/api/ApiQueryWatchlistRaw.php +++ b/includes/api/ApiQueryWatchlistRaw.php @@ -170,7 +170,8 @@ class ApiQueryWatchlistRaw extends ApiQueryGeneratorBase { ApiBase::PARAM_TYPE => 'user' ], 'token' => [ - ApiBase::PARAM_TYPE => 'string' + ApiBase::PARAM_TYPE => 'string', + ApiBase::PARAM_SENSITIVE => true, ], 'dir' => [ ApiBase::PARAM_DFLT => 'ascending', diff --git a/includes/api/ApiUndelete.php b/includes/api/ApiUndelete.php index 952e0087c8..3aa7b608dc 100644 --- a/includes/api/ApiUndelete.php +++ b/includes/api/ApiUndelete.php @@ -33,7 +33,6 @@ class ApiUndelete extends ApiBase { $this->useTransactionalTimeLimit(); $params = $this->extractRequestParams(); - $this->checkUserRightsAny( 'undelete' ); $user = $this->getUser(); if ( $user->isBlocked() ) { @@ -45,6 +44,10 @@ class ApiUndelete extends ApiBase { $this->dieWithError( [ 'apierror-invalidtitle', wfEscapeWikiText( $params['title'] ) ] ); } + if ( !$titleObj->userCan( 'undelete', $user, 'secure' ) ) { + $this->dieWithError( 'permdenied-undelete' ); + } + // Check if user can add tags if ( !is_null( $params['tags'] ) ) { $ableToTag = ChangeTags::canAddTagsAccompanyingChange( $params['tags'], $user ); diff --git a/includes/cache/MessageCache.php b/includes/cache/MessageCache.php index 70e1d9a75d..8a42a9a72d 100644 --- a/includes/cache/MessageCache.php +++ b/includes/cache/MessageCache.php @@ -485,7 +485,8 @@ class MessageCache { } else { # Effectively disallows use of '/' character in NS_MEDIAWIKI for uses # other than language code. - $conds[] = 'page_title NOT' . $dbr->buildLike( $dbr->anyString(), '/', $dbr->anyString() ); + $conds[] = 'page_title NOT' . + $dbr->buildLike( $dbr->anyString(), '/', $dbr->anyString() ); } # Conditions to fetch oversized pages to ignore them @@ -551,7 +552,7 @@ class MessageCache { /** * Updates cache as necessary when message page is changed * - * @param string $title Message cache key with initial uppercase letter. + * @param string $title Message cache key with initial uppercase letter * @param string|bool $text New contents of the page (false if deleted) */ public function replace( $title, $text ) { @@ -596,9 +597,8 @@ class MessageCache { $page->loadPageData( $page::READ_LATEST ); $text = $this->getMessageTextFromContent( $page->getContent() ); // Check if an individual cache key should exist and update cache accordingly - $titleKey = $this->wanCache->makeKey( - 'messages-big', $this->mCache[$code]['HASH'], $title ); if ( is_string( $text ) && strlen( $text ) > $wgMaxMsgCacheEntrySize ) { + $titleKey = $this->bigMessageCacheKey( $this->mCache[$code]['HASH'], $title ); $this->wanCache->set( $titleKey, ' ' . $text, $this->mExpiry ); } // Mark this cache as definitely being "latest" (non-volatile) so @@ -967,8 +967,8 @@ class MessageCache { * some callers require this behavior. LanguageConverter::parseCachedTable() * and self::get() are some examples in core. * - * @param string $title Message cache key with initial uppercase letter. - * @param string $code Code denoting the language to try. + * @param string $title Message cache key with initial uppercase letter + * @param string $code Code denoting the language to try * @return string|bool The message, or false if it does not exist or on error */ public function getMsgFromNamespace( $title, $code ) { @@ -995,8 +995,8 @@ class MessageCache { return false; } - // Try the individual message cache - $titleKey = $this->wanCache->makeKey( 'messages-big', $this->mCache[$code]['HASH'], $title ); + // Individual message cache key + $titleKey = $this->bigMessageCacheKey( $this->mCache[$code]['HASH'], $title ); if ( $this->mCacheVolatile[$code] ) { $entry = false; @@ -1005,6 +1005,7 @@ class MessageCache { __METHOD__ . ': loading volatile key \'{titleKey}\'', [ 'titleKey' => $titleKey, 'code' => $code ] ); } else { + // Try the individual message cache $entry = $this->wanCache->get( $titleKey ); } @@ -1057,7 +1058,8 @@ class MessageCache { $message = false; // negative caching } - if ( $message === false ) { // negative caching + if ( $message === false ) { + // Negative caching in case a "too big" message is no longer available (deleted) $this->mCache[$code][$title] = '!NONEXISTENT'; $this->wanCache->set( $titleKey, '!NONEXISTENT', $this->mExpiry, $cacheOpts ); } @@ -1301,4 +1303,13 @@ class MessageCache { return $msgText; } + + /** + * @param string $hash Hash for this version of the entire key/value overrides map + * @param string $title Message cache key with initial uppercase letter + * @return string + */ + private function bigMessageCacheKey( $hash, $title ) { + return $this->wanCache->makeKey( 'messages-big', $hash, $title ); + } } diff --git a/includes/cache/localisation/LocalisationCache.php b/includes/cache/localisation/LocalisationCache.php index cbff113761..d499340d0e 100644 --- a/includes/cache/localisation/LocalisationCache.php +++ b/includes/cache/localisation/LocalisationCache.php @@ -212,19 +212,17 @@ class LocalisationCache { case 'detect': if ( !empty( $conf['storeDirectory'] ) ) { $storeClass = 'LCStoreCDB'; + } elseif ( $wgCacheDirectory ) { + $storeConf['directory'] = $wgCacheDirectory; + $storeClass = 'LCStoreCDB'; } else { - $cacheDir = $wgCacheDirectory ?: wfTempDir(); - if ( $cacheDir ) { - $storeConf['directory'] = $cacheDir; - $storeClass = 'LCStoreCDB'; - } else { - $storeClass = 'LCStoreDB'; - } + $storeClass = 'LCStoreDB'; } break; default: throw new MWException( - 'Please set $wgLocalisationCacheConf[\'store\'] to something sensible.' ); + 'Please set $wgLocalisationCacheConf[\'store\'] to something sensible.' + ); } } diff --git a/includes/libs/mime/XmlTypeCheck.php b/includes/libs/mime/XmlTypeCheck.php index 7f2bf5e81b..e48cf62346 100644 --- a/includes/libs/mime/XmlTypeCheck.php +++ b/includes/libs/mime/XmlTypeCheck.php @@ -73,19 +73,36 @@ class XmlTypeCheck { */ private $parserOptions = [ 'processing_instruction_handler' => '', + 'external_dtd_handler' => '', + 'dtd_handler' => '', + 'require_safe_dtd' => true ]; /** + * Allow filtering an XML file. + * + * Filters should return either true or a string to indicate something + * is wrong with the file. $this->filterMatch will store if the + * file failed validation (true = failed validation). + * $this->filterMatchType will contain the validation error. + * $this->wellFormed will contain whether the xml file is well-formed. + * + * @note If multiple filters are hit, only one of them will have the + * result stored in $this->filterMatchType. + * * @param string $input a filename or string containing the XML element * @param callable $filterCallback (optional) * Function to call to do additional custom validity checks from the * SAX element handler event. This gives you access to the element * namespace, name, attributes, and text contents. - * Filter should return 'true' to toggle on $this->filterMatch + * Filter should return a truthy value describing the error. * @param bool $isFile (optional) indicates if the first parameter is a * filename (default, true) or if it is a string (false) * @param array $options list of additional parsing options: * processing_instruction_handler: Callback for xml_set_processing_instruction_handler + * external_dtd_handler: Callback for the url of external dtd subset + * dtd_handler: Callback given the full text of the filterCallback = $filterCallback; @@ -186,6 +203,9 @@ class XmlTypeCheck { if ( $reader->nodeType === XMLReader::PI ) { $this->processingInstructionHandler( $reader->name, $reader->value ); } + if ( $reader->nodeType === XMLReader::DOC_TYPE ) { + $this->DTDHandler( $reader ); + } } while ( $reader->nodeType != XMLReader::ELEMENT ); // Process the rest of the document @@ -234,8 +254,13 @@ class XmlTypeCheck { $reader->value ); break; + case XMLReader::DOC_TYPE: + // We should never see a doctype after first + // element. + $this->wellFormed = false; + break; default: - // One of DOC, DOC_TYPE, ENTITY, END_ENTITY, + // One of DOC, ENTITY, END_ENTITY, // NOTATION, or XML_DECLARATION // xml_parse didn't send these to the filter, so we won't. } @@ -339,4 +364,140 @@ class XmlTypeCheck { $this->filterMatchType = $callbackReturn; } } + /** + * Handle coming across a parserOptions['external_dtd_handler']; + $generalCallback = $this->parserOptions['dtd_handler']; + $checkIfSafe = $this->parserOptions['require_safe_dtd']; + if ( !$externalCallback && !$generalCallback && !$checkIfSafe ) { + return; + } + $dtd = $reader->readOuterXML(); + $callbackReturn = false; + + if ( $generalCallback ) { + $callbackReturn = call_user_func( $generalCallback, $dtd ); + } + if ( $callbackReturn ) { + // Filter hit! + $this->filterMatch = true; + $this->filterMatchType = $callbackReturn; + $callbackReturn = false; + } + + $parsedDTD = $this->parseDTD( $dtd ); + if ( $externalCallback && isset( $parsedDTD['type'] ) ) { + $callbackReturn = call_user_func( + $externalCallback, + $parsedDTD['type'], + isset( $parsedDTD['publicid'] ) ? $parsedDTD['publicid'] : null, + isset( $parsedDTD['systemid'] ) ? $parsedDTD['systemid'] : null + ); + } + if ( $callbackReturn ) { + // Filter hit! + $this->filterMatch = true; + $this->filterMatchType = $callbackReturn; + $callbackReturn = false; + } + + if ( $checkIfSafe && isset( $parsedDTD['internal'] ) ) { + if ( !$this->checkDTDIsSafe( $parsedDTD['internal'] ) ) { + $this->wellFormed = false; + } + } + } + + /** + * Check if the internal subset of the DTD is safe. + * + * We whitelist an extremely restricted subset of DTD features. + * + * Safe is defined as: + * * Only contains entity defintions (e.g. No 255 bytes). + * * + * allowed if matched exactly for compatibility with graphviz + * * Comments. + * + * @param string $internalSubset The internal subset of the DTD + * @return bool true if safe. + */ + private function checkDTDIsSafe( $internalSubset ) { + $offset = 0; + $res = preg_match( + '/^(?:\s*' . + '|\s*' . + '|\s*)*\s*$/', + $internalSubset + ); + + return (bool)$res; + } + + /** + * Parse DTD into parts. + * + * If there is an error parsing the dtd, sets wellFormed to false. + * + * @param $dtd string + * @return array Possibly containing keys publicid, systemid, type and internal. + */ + private function parseDTD( $dtd ) { + $m = []; + $res = preg_match( + '/^PUBLIC)\s*' . + '(?:"(?P[^"]*)"|\'(?P[^\']*)\')' . // public identifer + '\s*"(?P[^"]*)"|\'(?P[^\']*)\'' . // system identifier + '|(?PSYSTEM)\s*' . + '(?:"(?P[^"]*)"|\'(?P[^\']*)\')' . + ')?\s*' . + '(?:\[\s*(?P.*)\])?\s*>$/s', + $dtd, + $m + ); + if ( !$res ) { + $this->wellFormed = false; + return []; + } + $parsed = []; + foreach ( $m as $field => $value ) { + if ( $value === '' || is_numeric( $field ) ) { + continue; + } + switch ( $field ) { + case 'typepublic': + case 'typesystem': + $parsed['type'] = $value; + break; + case 'pubquote': + case 'pubapos': + $parsed['publicid'] = $value; + break; + case 'pubsysquote': + case 'pubsysapos': + case 'sysquote': + case 'sysapos': + $parsed['systemid'] = $value; + break; + case 'internal': + $parsed['internal'] = $value; + break; + } + } + return $parsed; + } } diff --git a/includes/parser/Parser.php b/includes/parser/Parser.php index be4557d6f9..953f021c2b 100644 --- a/includes/parser/Parser.php +++ b/includes/parser/Parser.php @@ -1610,9 +1610,7 @@ class Parser { true, 'free', $this->getExternalLinkAttribs( $url ), $this->mTitle ); # Register it in the output object... - # Replace unnecessary URL escape codes with their equivalent characters - $pasteurized = self::normalizeLinkUrl( $url ); - $this->mOutput->addExternalLink( $pasteurized ); + $this->mOutput->addExternalLink( $url ); } return $text . $trail; } @@ -1908,10 +1906,7 @@ class Parser { $this->getExternalLinkAttribs( $url ), $this->mTitle ) . $dtrail . $trail; # Register link in the output object. - # Replace unnecessary URL escape codes with the referenced character - # This prevents spammers from hiding links from the filters - $pasteurized = self::normalizeLinkUrl( $url ); - $this->mOutput->addExternalLink( $pasteurized ); + $this->mOutput->addExternalLink( $url ); } return $s; @@ -5086,9 +5081,11 @@ class Parser { } if ( preg_match( "/^($prots)$addr$chars*$/u", $linkValue ) ) { $link = $linkValue; + $this->mOutput->addExternalLink( $link ); } else { $localLinkTitle = Title::newFromText( $linkValue ); if ( $localLinkTitle !== null ) { + $this->mOutput->addLink( $localLinkTitle ); $link = $localLinkTitle->getLinkURL(); } } diff --git a/includes/parser/ParserOutput.php b/includes/parser/ParserOutput.php index b2f99b3d3f..7de3b304f1 100644 --- a/includes/parser/ParserOutput.php +++ b/includes/parser/ParserOutput.php @@ -535,6 +535,10 @@ class ParserOutput extends CacheTime { # We don't register links pointing to our own server, unless... :-) global $wgServer, $wgRegisterInternalExternals; + # Replace unnecessary URL escape codes with the referenced character + # This prevents spammers from hiding links from the filters + $url = parser::normalizeLinkUrl( $url ); + $registerExternalLink = true; if ( !$wgRegisterInternalExternals ) { $registerExternalLink = !self::isLinkInternal( $wgServer, $url ); diff --git a/includes/search/SearchHighlighter.php b/includes/search/SearchHighlighter.php index d0e3a240d6..cebdb40dbb 100644 --- a/includes/search/SearchHighlighter.php +++ b/includes/search/SearchHighlighter.php @@ -29,6 +29,10 @@ class SearchHighlighter { protected $mCleanWikitext = true; + /** + * @warning If you pass false to this constructor, then + * the caller is responsible for HTML escaping. + */ function __construct( $cleanupWikitext = true ) { $this->mCleanWikitext = $cleanupWikitext; } @@ -456,6 +460,10 @@ class SearchHighlighter { $text = preg_replace( "/('''|<\/?[iIuUbB]>)/", "", $text ); $text = preg_replace( "/''/", "", $text ); + // Note, the previous /<\/?[^>]+>/ is insufficient + // for XSS safety as the HTML tag can span multiple + // search results (T144845). + $text = Sanitizer::escapeHtmlAllowEntities( $text ); return $text; } diff --git a/includes/specialpage/RedirectSpecialPage.php b/includes/specialpage/RedirectSpecialPage.php index b1ddacfb2f..9b5d5f463d 100644 --- a/includes/specialpage/RedirectSpecialPage.php +++ b/includes/specialpage/RedirectSpecialPage.php @@ -41,7 +41,7 @@ abstract class RedirectSpecialPage extends UnlistedSpecialPage { $query = $this->getRedirectQuery(); // Redirect to a page title with possible query parameters if ( $redirect instanceof Title ) { - $url = $redirect->getFullURL( $query ); + $url = $redirect->getFullUrlForRedirect( $query ); $this->getOutput()->redirect( $url ); return $redirect; diff --git a/includes/specialpage/SpecialPageFactory.php b/includes/specialpage/SpecialPageFactory.php index 33e1cc30c8..ae010f6fc8 100644 --- a/includes/specialpage/SpecialPageFactory.php +++ b/includes/specialpage/SpecialPageFactory.php @@ -144,6 +144,7 @@ class SpecialPageFactory { 'RandomInCategory' => 'SpecialRandomInCategory', 'Randomredirect' => 'SpecialRandomredirect', 'Randomrootpage' => 'SpecialRandomrootpage', + 'GoToInterwiki' => 'SpecialGoToInterwiki', // High use pages 'Mostlinkedcategories' => 'MostlinkedCategoriesPage', diff --git a/includes/specials/SpecialChangeCredentials.php b/includes/specials/SpecialChangeCredentials.php index 47f8d2f436..970a2e29f2 100644 --- a/includes/specials/SpecialChangeCredentials.php +++ b/includes/specials/SpecialChangeCredentials.php @@ -258,7 +258,7 @@ class SpecialChangeCredentials extends AuthManagerSpecialPage { } $title = Title::newFromText( $returnTo ); - return $title->getFullURL( $returnToQuery ); + return $title->getFullUrlForRedirect( $returnToQuery ); } protected function getRequestBlacklist() { diff --git a/includes/specials/SpecialChangeEmail.php b/includes/specials/SpecialChangeEmail.php index 785447f7f9..eb98fe76a7 100644 --- a/includes/specials/SpecialChangeEmail.php +++ b/includes/specials/SpecialChangeEmail.php @@ -136,7 +136,7 @@ class SpecialChangeEmail extends FormSpecialPage { $query = $request->getVal( 'returntoquery' ); if ( $this->status->value === true ) { - $this->getOutput()->redirect( $titleObj->getFullURL( $query ) ); + $this->getOutput()->redirect( $titleObj->getFullUrlForRedirect( $query ) ); } elseif ( $this->status->value === 'eauth' ) { # Notify user that a confirmation email has been sent... $this->getOutput()->wrapWikiMsg( "
\n$1\n
", diff --git a/includes/specials/SpecialGoToInterwiki.php b/includes/specials/SpecialGoToInterwiki.php new file mode 100644 index 0000000000..809a14aac3 --- /dev/null +++ b/includes/specials/SpecialGoToInterwiki.php @@ -0,0 +1,79 @@ +setHeaders(); + $target = Title::newFromText( $par ); + // Disallow special pages as a precaution against + // possible redirect loops. + if ( !$target || $target->isSpecialPage() ) { + $this->getOutput()->setStatusCode( 404 ); + $this->getOutput()->addWikiMsg( 'gotointerwiki-invalid' ); + return; + } + + $url = $target->getFullURL(); + if ( !$target->isExternal() || $target->isLocal() ) { + // Either a normal page, or a local interwiki. + // just redirect. + $this->getOutput()->redirect( $url, '301' ); + } else { + $this->getOutput()->addWikiMsg( + 'gotointerwiki-external', + $url, + $target->getFullText() + ); + } + } + + /** + * @return bool + */ + public function requiresWrite() { + return false; + } + + /** + * @return String + */ + protected function getGroupName() { + return 'redirects'; + } +} diff --git a/includes/specials/SpecialPageLanguage.php b/includes/specials/SpecialPageLanguage.php index db05ebe587..2943fd4e3d 100644 --- a/includes/specials/SpecialPageLanguage.php +++ b/includes/specials/SpecialPageLanguage.php @@ -136,7 +136,7 @@ class SpecialPageLanguage extends FormSpecialPage { } // Url to redirect to after the operation - $this->goToUrl = $title->getFullURL( + $this->goToUrl = $title->getFullUrlForRedirect( $title->isRedirect() ? [ 'redirect' => 'no' ] : [] ); diff --git a/includes/specials/SpecialPreferences.php b/includes/specials/SpecialPreferences.php index eee5b641a3..40b50ea5bf 100644 --- a/includes/specials/SpecialPreferences.php +++ b/includes/specials/SpecialPreferences.php @@ -148,7 +148,7 @@ class SpecialPreferences extends SpecialPage { // Set session data for the success message $this->getRequest()->getSession()->set( 'specialPreferencesSaveSuccess', 1 ); - $url = $this->getPageTitle()->getFullURL(); + $url = $this->getPageTitle()->getFullUrlForRedirect(); $this->getOutput()->redirect( $url ); return true; diff --git a/includes/specials/SpecialRecentchanges.php b/includes/specials/SpecialRecentchanges.php index ae0887d28f..f88f09c60e 100644 --- a/includes/specials/SpecialRecentchanges.php +++ b/includes/specials/SpecialRecentchanges.php @@ -746,6 +746,9 @@ class SpecialRecentChanges extends ChangesListSpecialPage { $user = $this->getUser(); $config = $this->getConfig(); if ( $options['from'] ) { + $resetLink = $this->makeOptionsLink( $this->msg( 'rclistfromreset' ), + [ 'from' => '' ], $nondefaults ); + $note .= $this->msg( 'rcnotefrom' ) ->numParams( $options['limit'] ) ->params( @@ -754,7 +757,13 @@ class SpecialRecentChanges extends ChangesListSpecialPage { $lang->userTime( $options['from'], $user ) ) ->numParams( $numRows ) - ->parse() . '
'; + ->parse() . ' ' . + Html::rawElement( + 'span', + [ 'class' => 'rcoptions-listfromreset' ], + $this->msg( 'parentheses' )->rawParams( $resetLink )->parse() + ) . + '
'; } # Sort data for display and make sure it's unique after we've added user data. diff --git a/includes/specials/SpecialSearch.php b/includes/specials/SpecialSearch.php index 9cc0048aab..139e4f70c3 100644 --- a/includes/specials/SpecialSearch.php +++ b/includes/specials/SpecialSearch.php @@ -259,7 +259,7 @@ class SpecialSearch extends SpecialPage { return null; } - return $url === null ? $title->getFullURL() : $url; + return $url === null ? $title->getFullUrlForRedirect() : $url; } /** diff --git a/includes/specials/SpecialWatchlist.php b/includes/specials/SpecialWatchlist.php index 365736f52d..c1c9ab0f27 100644 --- a/includes/specials/SpecialWatchlist.php +++ b/includes/specials/SpecialWatchlist.php @@ -81,6 +81,7 @@ class SpecialWatchlist extends ChangesListSpecialPage { if ( ( $config->get( 'EnotifWatchlist' ) || $config->get( 'ShowUpdatedMarker' ) ) && $request->getVal( 'reset' ) && $request->wasPosted() + && $user->matchEditToken( $request->getVal( 'token' ) ) ) { $user->clearAllNotifications(); $output->redirect( $this->getPageTitle()->getFullURL( $opts->getChangedValues() ) ); @@ -660,6 +661,7 @@ class SpecialWatchlist extends ChangesListSpecialPage { 'id' => 'mw-watchlist-resetbutton' ] ) . "\n" . Xml::submitButton( $this->msg( 'enotif_reset' )->text(), [ 'name' => 'mw-watchlist-reset-submit' ] ) . "\n" . + Html::hidden( 'token', $user->getEditToken() ) . "\n" . Html::hidden( 'reset', 'all' ) . "\n"; foreach ( $nondefaults as $key => $value ) { $form .= Html::hidden( $key, $value ) . "\n"; diff --git a/includes/specials/helpers/LoginHelper.php b/includes/specials/helpers/LoginHelper.php index f853f4173b..cfcbf652c0 100644 --- a/includes/specials/helpers/LoginHelper.php +++ b/includes/specials/helpers/LoginHelper.php @@ -89,7 +89,7 @@ class LoginHelper extends ContextSource { } if ( $type === 'successredirect' ) { - $redirectUrl = $returnToTitle->getFullURL( $returnToQuery, false, $proto ); + $redirectUrl = $returnToTitle->getFullUrlForRedirect( $returnToQuery, $proto ); $this->getOutput()->redirect( $redirectUrl ); } else { $this->getOutput()->addReturnTo( $returnToTitle, $returnToQuery, null, $options ); diff --git a/includes/specials/pagers/DeletedContribsPager.php b/includes/specials/pagers/DeletedContribsPager.php index a1f6b84506..78e1092dc5 100644 --- a/includes/specials/pagers/DeletedContribsPager.php +++ b/includes/specials/pagers/DeletedContribsPager.php @@ -129,7 +129,7 @@ class DeletedContribsPager extends IndexPager { $condition = []; $condition['ar_user_text'] = $this->target; - $index = 'usertext_timestamp'; + $index = 'ar_usertext_timestamp'; return [ $index, $condition ]; } diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php index 733c4fff05..2c0afdf00f 100644 --- a/includes/upload/UploadBase.php +++ b/includes/upload/UploadBase.php @@ -1359,7 +1359,10 @@ abstract class UploadBase { $filename, [ $this, 'checkSvgScriptCallback' ], true, - [ 'processing_instruction_handler' => 'UploadBase::checkSvgPICallback' ] + [ + 'processing_instruction_handler' => 'UploadBase::checkSvgPICallback', + 'external_dtd_handler' => 'UploadBase::checkSvgExternalDTD', + ] ); if ( $check->wellFormed !== true ) { // Invalid xml (T60553) @@ -1391,6 +1394,34 @@ abstract class UploadBase { return false; } + /** + * Verify that DTD urls referenced are only the standard dtds + * + * Browsers seem to ignore external dtds. However just to be on the + * safe side, only allow dtds from the svg standard. + * + * @param string $type PUBLIC or SYSTEM + * @param string $publicId The well-known public identifier for the dtd + * @param string $systemId The url for the external dtd + */ + public static function checkSvgExternalDTD( $type, $publicId, $systemId ) { + // This doesn't include the XHTML+MathML+SVG doctype since we don't + // allow XHTML anyways. + $allowedDTDs = [ + 'http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd', + 'http://www.w3.org/TR/2001/REC-SVG-20010904/DTD/svg10.dtd', + 'http://www.w3.org/Graphics/SVG/1.1/DTD/svg11-basic.dtd', + 'http://www.w3.org/Graphics/SVG/1.1/DTD/svg11-tiny.dtd' + ]; + if ( $type !== 'PUBLIC' + || !in_array( $systemId, $allowedDTDs ) + || strpos( $publicId, "-//W3C//" ) !== 0 + ) { + return [ 'upload-scripted-dtd' ]; + } + return false; + } + /** * @todo Replace this with a whitelist filter! * @param string $element diff --git a/languages/i18n/en.json b/languages/i18n/en.json index 37f9f7fbf1..d4196b0162 100644 --- a/languages/i18n/en.json +++ b/languages/i18n/en.json @@ -1429,6 +1429,7 @@ "rcfilters-hideminor-conflicts-typeofchange": "Certain types of change cannot be designated as \"minor,\" so this filter conflicts with the following Type of Change filters: $1", "rcfilters-typeofchange-conflicts-hideminor": "This Type of Change filter conflicts with the \"Minor Edits\" filter. Certain types of change cannot be designated as \"minor.\"", "rcnotefrom": "Below {{PLURAL:$5|is the change|are the changes}} since $3, $4 (up to $1 shown).", + "rclistfromreset": "Reset date selection", "rclistfrom": "Show new changes starting from $2, $3", "rcshowhideminor": "$1 minor edits", "rcshowhideminor-show": "Show", @@ -1557,6 +1558,7 @@ "php-uploaddisabledtext": "File uploads are disabled in PHP.\nPlease check the file_uploads setting.", "uploadscripted": "This file contains HTML or script code that may be erroneously interpreted by a web browser.", "upload-scripted-pi-callback": "Cannot upload a file that contains XML-stylesheet processing instruction.", + "upload-scripted-dtd": "Cannot upload SVG files that contain a non-standard DTD declaration.", "uploaded-script-svg": "Found scriptable element \"$1\" in the uploaded SVG file.", "uploaded-hostile-svg": "Found unsafe CSS in the style element of uploaded SVG file.", "uploaded-event-handler-on-svg": "Setting event-handler attributes $1=\"$2\" is not allowed in SVG files.", @@ -4286,5 +4288,10 @@ "restrictionsfield-help": "One IP address or CIDR range per line. To enable everything, use:
0.0.0.0/0\n::/0
", "revid": "revision $1", "pageid": "page ID $1", - "rawhtml-notallowed": "<html> tags cannot be used outside of normal pages." + "rawhtml-notallowed": "<html> tags cannot be used outside of normal pages.", + "gotointerwiki": "Leaving {{SITENAME}}", + "gotointerwiki-invalid": "The specified title was invalid.", + "gotointerwiki-external": "You are about to leave {{SITENAME}} to visit [[$2]] which is a separate website.\n\n[$1 Click here to continue on to $1].", + "undelete-cantedit": "You cannot undelete this page as you are not allowed to edit this page.", + "undelete-cantcreate": "You cannot undelete this page as there is no existing page with this name and you are not allowed to create this page." } diff --git a/languages/i18n/qqq.json b/languages/i18n/qqq.json index 8dd6d0f3f1..fc1994b0dd 100644 --- a/languages/i18n/qqq.json +++ b/languages/i18n/qqq.json @@ -1617,6 +1617,7 @@ "rcfilters-typeofchange-conflicts-hideminor": "Tooltip shown when hovering over a Type of change filter tag, when the Minor edits filter is also selected.\n\n\"Minor edits\" is {{msg-mw|rcfilters-filter-minor-label}}.\n\n\"Type of change\" is {{msg-mw|rcfilters-filtergroup-changetype}}.\n\nThis indicates that no results will be shown.", "rcnotefrom": "This message is displayed at [[Special:RecentChanges]] when viewing recentchanges from some specific time.\n\nThe corresponding message is {{msg-mw|Rclistfrom}}.\n\nParameters:\n* $1 - the maximum number of changes that are displayed\n* $2 - (Optional) a date and time\n* $3 - a date\n* $4 - a time\n* $5 - Number of changes are displayed, for use with PLURAL", "rclistfrom": "Used on [[Special:RecentChanges]]. Parameters:\n* $1 - (Currently not use) date and time. The date and the time adds to the rclistfrom description.\n* $2 - time. The time adds to the rclistfrom link description (with split of date and time).\n* $3 - date. The date adds to the rclistfrom link description (with split of date and time).\n\nThe corresponding message is {{msg-mw|Rcnotefrom}}.", + "rclistfromreset": "Used on [[Special:RecentChanges]] to reset a selection of a certain date range.", "rcshowhideminor": "Option text in [[Special:RecentChanges]]. Parameters:\n* $1 - the \"show/hide\" command, with the text taken from either {{msg-mw|rcshowhideminor-show}} or {{msg-mw|rcshowhideminor-hide}}\n{{Identical|Minor edit}}", "rcshowhideminor-show": "{{doc-actionlink}}\nOption text in [[Special:RecentChanges]] in conjunction with {{msg-mw|rcshowhideminor}}.\n\nSee also:\n* {{msg-mw|rcshowhideminor-hide}}\n{{Identical|Show}}", "rcshowhideminor-hide": "{{doc-actionlink}}\nOption text in [[Special:RecentChanges]] in conjunction with {{msg-mw|rcshowhideminor}}.\n\nSee also:\n* {{msg-mw|rcshowhideminor-show}}\n{{Identical|Hide}}", @@ -1744,6 +1745,7 @@ "php-uploaddisabledtext": "This means that file uploading is disabled in PHP, not upload of PHP-files.", "uploadscripted": "Used as error message when uploading a file.\n\nSee also:\n* {{msg-mw|zip-wrong-format}}\n* {{msg-mw|uploadjava}}\n* {{msg-mw|uploadvirus}}", "upload-scripted-pi-callback": "Used as error message when uploading an SVG file that contains xml-stylesheet processing instruction.", + "upload-scripted-dtd": "Used as an error message when uploading an svg file that contains a DTD declaration where the system identifier of the DTD is for something other than the standard SVG DTDS, or it is a SYSTEM DTD, or the public identifier does not start with -//W3C//. Note that errors related to the internal dtd subset do not use this error message.", "uploaded-script-svg": "Used as error message when uploading an SVG file that contains scriptable tags (script, handler, stylesheet, iframe).\n\nParameters:\n* $1 - The scriptable tag that blocked the SVG file from uploading.", "uploaded-hostile-svg": "Used as error message when uploading an SVG file that contains unsafe CSS.", "uploaded-event-handler-on-svg": "Used as error message when uploading an SVG file that contains event-handler attributes.\n\nParameters:\n* $1 - The event-handler attribute that is being modified in the SVG file.\n* $2 - The value that is given to the event-handler attribute.", @@ -4473,5 +4475,10 @@ "restrictionsfield-help": "Placeholder text displayed in restriction fields (e.g. on Special:BotPassword).", "revid": "Used to format a revision ID number in text. Parameters:\n* $1 - Revision ID number.\n{{Identical|Revision}}", "pageid": "Used to format a page ID number in text. Parameters:\n* $1 - Page ID number.", - "rawhtml-notallowed": "Error message given when $wgRawHtml = true; is set and a user uses an <html> tag in a system message or somewhere other than a normal page." + "rawhtml-notallowed": "Error message given when $wgRawHtml = true; is set and a user uses an <html> tag in a system message or somewhere other than a normal page.", + "gotointerwiki": "{{doc-special|GoToInterwiki}}\n\nSpecial:GoToInterwiki is a warning page displayed before redirecting users to external interwiki links. Its triggered by people going to something like [[Special:Search/google:foo]].", + "gotointerwiki-invalid": "Message shown on Special:GoToInterwiki if given an invalid title.", + "gotointerwiki-external": "Message shown on Special:GoToInterwiki if given a external interwiki link (e.g. [[Special:GoToInterwiki/Google:Foo]]). $1 is the full url the user is trying to get to. $2 is the text of the interwiki link (e.g. \"Google:foo\").", + "undelete-cantedit": "Shown if the user tries to undelete a page that they cannot edit", + "undelete-cantcreate": "Shown if the user tries to undelete a page which currently does not exist, and they are not allowed to create it. This could for example happen on a wiki with custom protection levels where the page name has been create-protected and the user has the right to undelete but not the right to edit protected pages." } diff --git a/languages/messages/MessagesEn.php b/languages/messages/MessagesEn.php index b9280ea096..c52ec0e081 100644 --- a/languages/messages/MessagesEn.php +++ b/languages/messages/MessagesEn.php @@ -427,6 +427,7 @@ $specialPageAliases = [ 'Fewestrevisions' => [ 'FewestRevisions' ], 'FileDuplicateSearch' => [ 'FileDuplicateSearch' ], 'Filepath' => [ 'FilePath' ], + 'GoToInterwiki' => [ 'GoToInterwiki' ], 'Import' => [ 'Import' ], 'Invalidateemail' => [ 'InvalidateEmail' ], 'JavaScriptTest' => [ 'JavaScriptTest' ], diff --git a/resources/src/mediawiki.rcfilters/styles/mw.rcfilters.variables.less b/resources/src/mediawiki.rcfilters/styles/mw.rcfilters.variables.less index 1ef49e2102..3060f255cd 100644 --- a/resources/src/mediawiki.rcfilters/styles/mw.rcfilters.variables.less +++ b/resources/src/mediawiki.rcfilters/styles/mw.rcfilters.variables.less @@ -11,9 +11,9 @@ // Result list circle indicators // Defined and used in mw.rcfilters.ui.ChangesListWrapperWidget.less -@result-circle-margin: 0.1em; +@result-circle-margin: 3px; @result-circle-general-margin: 0.5em; // In these small sizes, 'em' appears // squished and inconsistent. // Pixels are better for this use case: -@result-circle-diameter: 5px; +@result-circle-diameter: 6px; diff --git a/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.FilterWrapperWidget.js b/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.FilterWrapperWidget.js index 761fc6500b..c81b685abb 100644 --- a/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.FilterWrapperWidget.js +++ b/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.FilterWrapperWidget.js @@ -71,7 +71,10 @@ enter: 'onTextInputEnter' } ); this.capsule.connect( this, { capsuleItemClick: 'onCapsuleItemClick' } ); - this.capsule.popup.connect( this, { toggle: 'onCapsulePopupToggle' } ); + this.capsule.popup.connect( this, { + toggle: 'onCapsulePopupToggle', + ready: 'onCapsulePopupReady' + } ); // Initialize this.$element @@ -102,26 +105,29 @@ this.scrollToTop( filterWidget.$element ); }; + /** + * Respond to capsule popup ready event, fired after the popup is visible, positioned and clipped + */ + mw.rcfilters.ui.FilterWrapperWidget.prototype.onCapsulePopupReady = function () { + mw.hook( 'RcFilters.popup.open' ).fire( this.filterPopup.getSelectedFilter() ); + + this.scrollToTop( this.capsule.$element, 10 ); + if ( !this.filterPopup.getSelectedFilter() ) { + // No selection, scroll the popup list to top + setTimeout( function () { this.capsule.popup.$body.scrollTop( 0 ); }.bind( this ), 0 ); + } + }; + /** * Respond to popup toggle event. Reset selection in the list when the popup is closed. * * @param {boolean} isVisible Popup is visible */ mw.rcfilters.ui.FilterWrapperWidget.prototype.onCapsulePopupToggle = function ( isVisible ) { - if ( !isVisible ) { - if ( !this.textInput.getValue() ) { - // Only reset selection if we are not filtering - this.filterPopup.resetSelection(); - this.capsule.resetSelection(); - } - } else { - mw.hook( 'RcFilters.popup.open' ).fire( this.filterPopup.getSelectedFilter() ); - - this.scrollToTop( this.capsule.$element, 10 ); - if ( !this.filterPopup.getSelectedFilter() ) { - // No selection, scroll the popup list to top - setTimeout( function () { this.capsule.popup.$body.scrollTop( 0 ); }.bind( this ), 0 ); - } + if ( !isVisible && !this.textInput.getValue() ) { + // Only reset selection if we are not filtering + this.filterPopup.resetSelection(); + this.capsule.resetSelection(); } }; diff --git a/tests/phpunit/includes/resourceloader/ResourceLoaderStartUpModuleTest.php b/tests/phpunit/includes/resourceloader/ResourceLoaderStartUpModuleTest.php index 1b756be629..1e09e92a10 100644 --- a/tests/phpunit/includes/resourceloader/ResourceLoaderStartUpModuleTest.php +++ b/tests/phpunit/includes/resourceloader/ResourceLoaderStartUpModuleTest.php @@ -8,7 +8,7 @@ class ResourceLoaderStartUpModuleTest extends ResourceLoaderTestCase { ] ); } - public static function provideGetModuleRegistrations() { + public function provideGetModuleRegistrations() { return [ [ [ 'msg' => 'Empty registry', @@ -33,6 +33,88 @@ mw.loader.register( [ "test.blank", "{blankVer}" ] +] );', + ] ], + [ [ + 'msg' => 'Omit raw modules from registry', + 'modules' => [ + 'test.raw' => new ResourceLoaderTestModule( [ 'isRaw' => true ] ), + 'test.blank' => new ResourceLoaderTestModule(), + ], + 'out' => ' +mw.loader.addSource( { + "local": "/w/load.php" +} ); +mw.loader.register( [ + [ + "test.blank", + "{blankVer}" + ] +] );', + ] ], + [ [ + 'msg' => 'Version falls back gracefully if getVersionHash throws', + 'modules' => [ + 'test.fail' => ( + ( $mock = $this->getMockBuilder( 'ResourceLoaderTestModule' ) + ->setMethods( [ 'getVersionHash' ] )->getMock() ) + && $mock->method( 'getVersionHash' )->will( + $this->throwException( new Exception ) + ) + ) ? $mock : $mock + ], + 'out' => ' +mw.loader.addSource( { + "local": "/w/load.php" +} ); +mw.loader.register( [ + [ + "test.fail", + "" + ] +] ); +mw.loader.state( { + "test.fail": "error" +} );', + ] ], + [ [ + 'msg' => 'Use version from getVersionHash', + 'modules' => [ + 'test.version' => ( + ( $mock = $this->getMockBuilder( 'ResourceLoaderTestModule' ) + ->setMethods( [ 'getVersionHash' ] )->getMock() ) + && $mock->method( 'getVersionHash' )->willReturn( '1234567' ) + ) ? $mock : $mock + ], + 'out' => ' +mw.loader.addSource( { + "local": "/w/load.php" +} ); +mw.loader.register( [ + [ + "test.version", + "1234567" + ] +] );', + ] ], + [ [ + 'msg' => 'Re-hash version from getVersionHash if too long', + 'modules' => [ + 'test.version' => ( + ( $mock = $this->getMockBuilder( 'ResourceLoaderTestModule' ) + ->setMethods( [ 'getVersionHash' ] )->getMock() ) + && $mock->method( 'getVersionHash' )->willReturn( '12345678' ) + ) ? $mock : $mock + ], + 'out' => ' +mw.loader.addSource( { + "local": "/w/load.php" +} ); +mw.loader.register( [ + [ + "test.version", + "016es8l" + ] ] );', ] ], [ [ @@ -303,8 +385,8 @@ mw.loader.register( [ /** * @dataProvider provideGetModuleRegistrations - * @covers ResourceLoaderStartUpModule::compileUnresolvedDependencies * @covers ResourceLoaderStartUpModule::getModuleRegistrations + * @covers ResourceLoaderStartUpModule::compileUnresolvedDependencies * @covers ResourceLoader::makeLoaderRegisterScript */ public function testGetModuleRegistrations( $case ) { @@ -344,6 +426,7 @@ mw.loader.register( [ ]; } /** + * @covers ResourceLoaderStartUpModule::getModuleRegistrations * @dataProvider provideRegistrations */ public function testRegistrationsMinified( $modules ) { @@ -368,6 +451,7 @@ mw.loader.register( [ } /** + * @covers ResourceLoaderStartUpModule::getModuleRegistrations * @dataProvider provideRegistrations */ public function testRegistrationsUnminified( $modules ) { diff --git a/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php b/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php index e0a82d06af..2618e781a1 100644 --- a/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php +++ b/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php @@ -49,13 +49,26 @@ class ResourceLoaderTest extends ResourceLoaderTestCase { * @covers ResourceLoader::register * @covers ResourceLoader::getModule */ - public function testRegisterValid() { + public function testRegisterValidObject() { $module = new ResourceLoaderTestModule(); $resourceLoader = new EmptyResourceLoader(); $resourceLoader->register( 'test', $module ); $this->assertEquals( $module, $resourceLoader->getModule( 'test' ) ); } + /** + * @covers ResourceLoader::register + * @covers ResourceLoader::getModule + */ + public function testRegisterValidArray() { + $module = new ResourceLoaderTestModule(); + $resourceLoader = new EmptyResourceLoader(); + // Covers case of register() setting $rl->moduleInfos, + // but $rl->modules lazy-populated by getModule() + $resourceLoader->register( 'test', [ 'object' => $module ] ); + $this->assertEquals( $module, $resourceLoader->getModule( 'test' ) ); + } + /** * @covers ResourceLoader::register */ @@ -384,6 +397,33 @@ mw.example(); ); } + /** + * @covers ResourceLoader::makeLoaderRegisterScript + */ + public function testMakeLoaderRegisterScript() { + $this->assertEquals( + 'mw.loader.register( [ + [ + "test.name", + "1234567" + ] +] );', + ResourceLoader::makeLoaderRegisterScript( [ + [ 'test.name', '1234567' ], + ] ), + 'Nested array parameter' + ); + + $this->assertEquals( + 'mw.loader.register( "test.name", "1234567" );', + ResourceLoader::makeLoaderRegisterScript( + 'test.name', + '1234567' + ), + 'Variadic parameters' + ); + } + /** * @covers ResourceLoader::makeLoaderSourcesScript */ diff --git a/tests/phpunit/includes/upload/UploadBaseTest.php b/tests/phpunit/includes/upload/UploadBaseTest.php index a42c86c3a6..dd68cdcab7 100644 --- a/tests/phpunit/includes/upload/UploadBaseTest.php +++ b/tests/phpunit/includes/upload/UploadBaseTest.php @@ -130,8 +130,8 @@ class UploadBaseTest extends MediaWikiTestCase { */ public function testCheckSvgScriptCallback( $svg, $wellFormed, $filterMatch, $message ) { list( $formed, $match ) = $this->upload->checkSvgString( $svg ); - $this->assertSame( $wellFormed, $formed, $message ); - $this->assertSame( $filterMatch, $match, $message ); + $this->assertSame( $wellFormed, $formed, $message . " (well-formed)" ); + $this->assertSame( $filterMatch, $match, $message . " (filter match)" ); } public static function provideCheckSvgScriptCallback() { @@ -254,10 +254,16 @@ class UploadBaseTest extends MediaWikiTestCase { ], [ ' ]> ', - true, + false, true, 'SVG with embedded stylesheet (http://html5sec.org/#125)' ], + [ + ' ', + true, + true, + 'SVG with embedded stylesheet no doctype' + ], [ ' alert(1) ', true, @@ -364,7 +370,7 @@ class UploadBaseTest extends MediaWikiTestCase { ], [ ' ]> &lol2; ', - true, + false, true, 'SVG with encoded script tag in internal entity (reported by Beyond Security)' ], @@ -374,6 +380,16 @@ class UploadBaseTest extends MediaWikiTestCase { false, 'SVG with external entity' ], + [ + // The base64 = . If for some reason + // entities actually do get loaded, this should trigger + // filterMatch to be true. So this test verifies that we + // are not loading external entities. + ' ]> &foo; ', + false, + false, /* False verifies entities aren't getting loaded */ + 'SVG with data: uri external entity' + ], [ " ", true, @@ -393,6 +409,104 @@ class UploadBaseTest extends MediaWikiTestCase { false, 'SVG with local urls, including filter: in style' ], + [ + ' ]> ', + false, + false, + 'SVG with evil default attribute values' + ], + [ + ' ', + true, + true, + 'SVG with an evil external dtd' + ], + [ + '', + true, + true, + 'SVG with random public doctype' + ], + [ + '', + true, + true, + 'SVG with random SYSTEM doctype' + ], + [ + '] >', + false, + false, + 'SVG with parameter entity' + ], + [ + '', + false, + false, + 'SVG with entity referencing parameter entity' + ], + [ + ' ] >', + false, + false, + 'SVG with long entity' + ], + [ + ' ] >&foo;', + true, + false, + 'SVG with apostrophe quote entity' + ], + [ + ' ] >&foo;', + false, + false, + 'SVG with recursive entity', + ], + [ + ' ]> ', + true, /* well-formed */ + false, /* filter-hit */ + 'GraphViz-esque svg with #FIXED xlink ns (Should be allowed)' + ], + [ + ' ]> ', + false, + false, + 'GraphViz ATLIST exception should match exactly' + ], + [ + ' ]>', + true, + false, + 'DTD with comments (Should be allowed)' + ], + [ + ' ]>', + false, + false, + 'DTD with invalid comment' + ], + [ + ' ]>', + false, + false, + 'DTD with invalid comment 2' + ], + [ + ' ]>', + true, + false, + 'DTD with aliased entities (Should be allowed)' + ], + [ + ' ]>', + true, + false, + 'DTD with aliased entities apos (Should be allowed)' + ] ]; // @codingStandardsIgnoreEnd } @@ -478,7 +592,10 @@ class UploadTestHandler extends UploadBase { $svg, [ $this, 'checkSvgScriptCallback' ], false, - [ 'processing_instruction_handler' => 'UploadBase::checkSvgPICallback' ] + [ + 'processing_instruction_handler' => 'UploadBase::checkSvgPICallback', + 'external_dtd_handler' => 'UploadBase::checkSvgExternalDTD' + ] ); return [ $check->wellFormed, $check->filterMatch ]; }