Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Change trivial use of getVal('action') to getRawVal

Browse source 
Per docs added in I18767cd809f67b, these don't need normalization
as they are only compared against predefined strings, and besides
are generally entered manually in a form, and even then would not
require the kinds of Unicode chars that have multiple/non-normalized
forms.

In nearby areas to also fix some trivial cases:

* getVal('title') obviously needs normalization.
  Use getText() to make this more obvious.

* getVal() compared against simple string literals within the code
  obviously don't need normalization (e.g. printable === 'no').

* Change hot code in MediaWiki checking for whether 'diff' or 'oldid'
  are set to getCheck (which uses getRawVal) instead of getVal.
  As a bonus this means it now handles values like "0" correctly,
  which could theoretically have caused bad behaviour before.

Change-Id: Ied721cfdf59c7ba11d1afa6f4cc59ede1381238e
This commit is contained in:
Timo Tijhof 2021-08-10 22:59:39 +01:00
parent 1106a05430
commit e387cd9c35
15 changed files with 34 additions and 35 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
api.php
View file

@ -105,12 +105,12 @@ function wfApiMain() {
if ( $processor ) {
try {
$manager = $processor->getModuleManager();
$module = $manager->getModule( $wgRequest->getVal( 'action' ), 'action' );
$module = $manager->getModule( $wgRequest->getRawVal( 'action' ), 'action' );
} catch ( Throwable $ex ) {
$module = null;
}
if ( !$module || $module->mustBePosted() ) {
$items[] = "action=" . $wgRequest->getVal( 'action' );
$items[] = "action=" . $wgRequest->getRawVal( 'action' );
} else {
$items[] = wfArrayToCgi( $wgRequest->getValues() );
}

22
includes/MediaWiki.php
View file

@ -78,8 +78,8 @@ class MediaWiki {
private function parseTitle() {
$request = $this->context->getRequest();
$curid = $request->getInt( 'curid' );
$title = $request->getVal( 'title' );
$action = $request->getVal( 'action' );
$title = $request->getText( 'title' );
$action = $request->getRawVal( 'action' );
if ( $curid ) {
// URLs like this are generated by RC, because rc_title isn't always accurate
@ -199,7 +199,7 @@ class MediaWiki {
$output = $this->context->getOutput();
$user = $this->context->getUser();
if ( $request->getVal( 'printable' ) === 'yes' ) {
if ( $request->getRawVal( 'printable' ) === 'yes' ) {
$output->setPrintable();
}
@ -355,10 +355,10 @@ class MediaWiki {
$request = $this->context->getRequest();
$output = $this->context->getOutput();
if ( $request->getVal( 'action', 'view' ) != 'view'
if ( $request->getRawVal( 'action', 'view' ) != 'view'
|| $request->wasPosted()
|| ( $request->getCheck( 'title' )
&& $title->getPrefixedDBkey() == $request->getVal( 'title' ) )
&& $title->getPrefixedDBkey() == $request->getText( 'title' ) )
|| count( $request->getValueNames( [ 'action', 'title' ] ) )
|| !$this->getHookRunner()->onTestCanonicalRedirect( $request, $title, $output )
) {
@ -442,12 +442,12 @@ class MediaWiki {
// Namespace might change when using redirects
// Check for redirects ...
$action = $request->getVal( 'action', 'view' );
$action = $request->getRawVal( 'action', 'view' );
$file = ( $page instanceof WikiFilePage ) ? $page->getFile() : null;
if ( ( $action == 'view' || $action == 'render' ) // ... for actions that show content
&& !$request->getVal( 'oldid' ) // ... and are not old revisions
&& !$request->getVal( 'diff' ) // ... and not when showing diff
&& $request->getVal( 'redirect' ) != 'no' // ... unless explicitly told not to
&& !$request->getCheck( 'oldid' ) // ... and are not old revisions
&& !$request->getCheck( 'diff' ) // ... and not when showing diff
&& $request->getRawVal( 'redirect' ) !== 'no' // ... unless explicitly told not to
// ... and the article is not a non-redirect image page with associated file
&& !( is_object( $file ) && $file->exists() && !$file->getRedirected() )
) {
@ -559,7 +559,7 @@ class MediaWiki {
$this->main();
} catch ( Exception $e ) {
$context = $this->context;
$action = $context->getRequest()->getVal( 'action', 'view' );
$action = $context->getRequest()->getRawVal( 'action', 'view' );
if (
$e instanceof DBConnectionError &&
$context->hasTitle() &&
@ -867,7 +867,7 @@ class MediaWiki {
$request = $this->context->getRequest();
// Send Ajax requests to the Ajax dispatcher.
if ( $request->getVal( 'action' ) === 'ajax' ) {
if ( $request->getRawVal( 'action' ) === 'ajax' ) {
// Set a dummy title, because $wgTitle == null might break things
$title = Title::makeTitle( NS_SPECIAL, 'Badtitle/performing an AJAX call in '
. __METHOD__

4
includes/OutputPage.php
View file

@ -2754,7 +2754,7 @@ class OutputPage extends ContextSource {
# not especially useful as a returnto parameter. Use the title
# from the request instead, if there was one.
$request = $this->getRequest();
$returnto = Title::newFromText( $request->getVal( 'title', '' ) );
$returnto = Title::newFromText( $request->getText( 'title' ) );
if ( $action == 'edit' ) {
$msg = 'whitelistedittext';
$displayReturnto = $returnto;
@ -3469,7 +3469,7 @@ class OutputPage extends ContextSource {
public function userCanPreview() {
$request = $this->getRequest();
if (
$request->getVal( 'action' ) !== 'submit' ||
$request->getRawVal( 'action' ) !== 'submit' ||
!$request->wasPosted()
) {
return false;

6
includes/Title.php
View file

@ -498,10 +498,10 @@ class Title implements LinkTarget, PageIdentity, IDBAccessObject {
* THIS IS NOT THE FUNCTION YOU WANT. Use Title::newFromText().
*
* Example of wrong and broken code:
* $title = Title::newFromURL( $wgRequest->getVal( 'title' ) );
* $title = Title::newFromURL( $request->getText( 'title' ) );
*
* Example of right code:
* $title = Title::newFromText( $wgRequest->getVal( 'title' ) );
* $title = Title::newFromText( $request->getText( 'title' ) );
*
* Create a new Title from URL-encoded text. Ensures that
* the given title's length does not exceed the maximum.
@ -2366,7 +2366,7 @@ class Title implements LinkTarget, PageIdentity, IDBAccessObject {
// @todo FIXME: This causes breakage in various places when we
// actually expected a local URL and end up with dupe prefixes.
if ( $wgRequest->getVal( 'action' ) == 'render' ) {
if ( $wgRequest->getRawVal( 'action' ) == 'render' ) {
LoggerFactory::getInstance( 'T263581' )
->debug(
"Title::getLocalURL called from render action",

2
includes/actions/ActionFactory.php
View file

@ -315,7 +315,7 @@ class ActionFactory {
}
$request = $context->getRequest();
$actionName = $request->getVal( 'action', 'view' );
$actionName = $request->getRawVal( 'action', 'view' );
// Normalize to lowercase
$actionName = strtolower( $actionName );

6
includes/page/Article.php
View file

@ -295,7 +295,7 @@ class Article implements Page {
}
$oldRev = $this->mRevisionRecord;
if ( $request->getVal( 'direction' ) == 'next' ) {
if ( $request->getRawVal( 'direction' ) === 'next' ) {
$nextid = 0;
if ( $oldRev ) {
$nextRev = $this->revisionStore->getNextRevision( $oldRev );
@ -309,7 +309,7 @@ class Article implements Page {
} else {
$this->mRedirectUrl = $this->getTitle()->getFullURL( 'redirect=no' );
}
} elseif ( $request->getVal( 'direction' ) == 'prev' ) {
} elseif ( $request->getRawVal( 'direction' ) === 'prev' ) {
$previd = 0;
if ( $oldRev ) {
$prevRev = $this->revisionStore->getPreviousRevision( $oldRev );
@ -827,7 +827,7 @@ class Article implements Page {
$diff = $request->getVal( 'diff' );
$rcid = $request->getVal( 'rcid' );
$diffOnly = $request->getBool( 'diffonly', $user->getOption( 'diffonly' ) );
$purge = $request->getVal( 'action' ) == 'purge';
$purge = $request->getRawVal( 'action' ) === 'purge';
$unhide = $request->getInt( 'unhide' ) == 1;
$oldid = $this->getOldID();

2
includes/parser/ParserCache.php
View file

@ -308,7 +308,7 @@ class ParserCache {
// idhash seem to mean 'page id' + 'rendering hash' (r3710)
$pageid = $page->getId( PageRecord::LOCAL );
// TODO: remove the split T263581
$renderkey = (int)( $wgRequest->getVal( 'action' ) == 'render' );
$renderkey = (int)( $wgRequest->getRawVal( 'action' ) == 'render' );
$title = $this->titleFactory->castFromPageIdentity( $page );
$hash = $options->optionsHash( $usedOptions, $title );

2
includes/skins/Skin.php
View file

@ -750,7 +750,7 @@ abstract class Skin extends ContextSource {
* @return string HTML
*/
public function getUndeleteLink() {
$action = $this->getRequest()->getVal( 'action', 'view' );
$action = $this->getRequest()->getRawVal( 'action', 'view' );
$title = $this->getTitle();
$linkRenderer = MediaWikiServices::getInstance()->getLinkRenderer();

2
includes/specialpage/ChangesListSpecialPage.php
View file

@ -628,7 +628,7 @@ abstract class ChangesListSpecialPage extends SpecialPage {
}
// Used by Structured UI app to get results without MW chrome
if ( $this->getRequest()->getVal( 'action' ) === 'render' ) {
if ( $this->getRequest()->getRawVal( 'action' ) === 'render' ) {
$this->getOutput()->setArticleBodyOnly( true );
}

2
includes/specialpage/LoginSignupSpecialPage.php
View file

@ -106,7 +106,7 @@ abstract class LoginSignupSpecialPage extends AuthManagerSpecialPage {
$request = $this->getRequest();
$this->mPosted = $request->wasPosted();
$this->mAction = $request->getVal( 'action' );
$this->mAction = $request->getRawVal( 'action' );
$this->mFromHTTP = $request->getBool( 'fromhttp', false )
|| $request->getBool( 'wpFromhttp', false );
$this->mStickHTTPS = $this->getConfig()->get( 'ForceHTTPS' )

2
includes/specials/SpecialImport.php
View file

@ -107,7 +107,7 @@ class SpecialImport extends SpecialPage {
$this->checkReadOnly();
$request = $this->getRequest();
if ( $request->wasPosted() && $request->getVal( 'action' ) == 'submit' ) {
if ( $request->wasPosted() && $request->getRawVal( 'action' ) == 'submit' ) {
$this->doImport();
}
$this->showForm();

2
includes/specials/SpecialMergeHistory.php
View file

@ -110,7 +110,7 @@ class SpecialMergeHistory extends SpecialPage {
*/
private function loadRequestParams() {
$request = $this->getRequest();
$this->mAction = $request->getVal( 'action' );
$this->mAction = $request->getRawVal( 'action' );
$this->mTarget = $request->getVal( 'target' );
$this->mDest = $request->getVal( 'dest' );
$this->mSubmitted = $request->getBool( 'submitted' );

9
includes/specials/SpecialMovepage.php
View file

@ -156,11 +156,10 @@ class MovePageForm extends UnlistedSpecialPage {
$this->outputHeader();
$request = $this->getRequest();
$target = $par ?? $request->getVal( 'target' );
// Yes, the use of getVal() and getText() is wanted, see T22365
$oldTitleText = $request->getVal( 'wpOldTitle', $target );
// Beware: The use of WebRequest::getText() is wanted! See T22365
$target = $par ?? $request->getText( 'target' );
$oldTitleText = $request->getText( 'wpOldTitle', $target );
$this->oldTitle = Title::newFromText( $oldTitleText );
if ( !$this->oldTitle ) {
@ -205,7 +204,7 @@ class MovePageForm extends UnlistedSpecialPage {
$this->moveOverShared = $request->getBool( 'wpMoveOverSharedFile' );
$this->watch = $request->getCheck( 'wpWatch' ) && $user->isRegistered();
if ( $request->getVal( 'action' ) == 'submit' && $request->wasPosted()
if ( $request->getRawVal( 'action' ) == 'submit' && $request->wasPosted()
&& $user->matchEditToken( $request->getVal( 'wpEditToken' ) )
) {
$this->doSubmit();

2
includes/specials/SpecialUndelete.php
View file

@ -153,7 +153,7 @@ class SpecialUndelete extends SpecialPage {
$request = $this->getRequest();
$user = $this->getUser();
$this->mAction = $request->getVal( 'action' );
$this->mAction = $request->getRawVal( 'action' );
if ( $par !== null && $par !== '' ) {
$this->mTarget = $par;
} else {

2
includes/specials/SpecialWatchlist.php
View file

@ -345,7 +345,7 @@ class SpecialWatchlist extends ChangesListSpecialPage {
}
}
if ( $this->getRequest()->getVal( 'action' ) == 'submit' ) {
if ( $this->getRequest()->getRawVal( 'action' ) == 'submit' ) {
$allBooleansFalse = [];
// If the user submitted the form, start with a baseline of "all