r101808 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r101807‎ | r101808 | r101809 >
Date:14:04, 3 November 2011
Author:mah
Status:ok (Comments)
Tags:
Comment:
use isValidURI for redirect check
Modified paths:
  • /trunk/phase3/includes/HttpFunctions.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/HttpFunctions.php
——@@ -855,7 +855,7 @@
856856 # Check security of URL
857857 $url = $this->getResponseHeader( "Location" );
858858
859 - if ( substr( $url, 0, 7 ) !== 'http://' ) {
 859+ if ( !HTTP::isValidURI( $url ) ) {
860860 wfDebug( __METHOD__ . ": insecure redirection\n" );
861861 break;
862862 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r101817comment that isValidURI must reject file:// URIhashar15:06, 3 November 2011
r101905Follow-up r101808. Use canonical class name.platonides22:58, 3 November 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r67684Fixes for r61911:...tstarling06:39, 9 June 2010

Comments

#Comment by Hashar (talk | contribs)   14:37, 3 November 2011

Since http://Iam.notsecure.com/ is a valid URI, that would redirect people to an insecure site thus defeating the original purpose.

#Comment by MarkAHershberger (talk | contribs)   14:54, 3 November 2011

That url would have passed before this change.

#Comment by MarkAHershberger (talk | contribs)   14:57, 3 November 2011

To be clear, before this change, https://Iam.very.secure.com/ would not have worked.

#Comment by Hashar (talk | contribs)   15:07, 3 November 2011

Marking OK per IRC.

#Comment by Nikerabbit (talk | contribs)   15:20, 3 November 2011

The canonical name of this class is Http if I am not mistaken.

Status & tagging log