From 529e92b2830f02906a7df7854f2aecf46326742d Mon Sep 17 00:00:00 2001 From: Tim Starling Date: Wed, 1 Jun 2011 00:51:09 +0000 Subject: [PATCH] Fixes for r88883, r89197: * Modified WebRequest::findIE6Extension() to fix the performance issue and the hash parsing issue I noted on CR * In FindIE6ExtensionTest, fixed all the assertEquals() calls, I had the expected and actual around the wrong way * Added a couple of extra tests for cases that seemed important during the rewrite. --- includes/WebRequest.php | 55 +++++++++++-------- .../phpunit/includes/FindIE6ExtensionTest.php | 40 ++++++++++---- 2 files changed, 60 insertions(+), 35 deletions(-) diff --git a/includes/WebRequest.php b/includes/WebRequest.php index 209b1b461c..1e0b98d832 100644 --- a/includes/WebRequest.php +++ b/includes/WebRequest.php @@ -814,44 +814,52 @@ class WebRequest { * insecure. * * The criteria for finding an extension are as follows: - * * a possible extension is a dot followed by one or more characters not in <>\"/:|?.# - * * if we find a possible extension followed by the end of the string or a #, that's our extension - * * if we find a possible extension followed by a ?, that's our extension - * ** UNLESS it's exe, dll or cgi, in which case we ignore it and continue searching for another possible extension - * * if we find a possible extension followed by a dot or another illegal character, we ignore it and continue searching + * - a possible extension is a dot followed by one or more characters not + * in <>\"/:|?.# + * - if we find a possible extension followed by the end of the string or + * a #, that's our extension + * - if we find a possible extension followed by a ?, that's our extension + * - UNLESS it's exe, dll or cgi, in which case we ignore it and continue + * searching for another possible extension + * - if we find a possible extension followed by a dot or another illegal + * character, we ignore it and continue searching * * @param $url string URL * @return mixed Detected extension (string), or false if none found */ public static function findIE6Extension( $url ) { - $remaining = $url; - while ( $remaining !== '' ) { + $pos = 0; + $hashPos = strpos( $url, '#' ); + if ( $hashPos !== false ) { + $urlLength = $hashPos; + } else { + $urlLength = strlen( $url ); + } + $remainingLength = $urlLength; + while ( $remainingLength > 0 ) { // Skip ahead to the next dot - $pos = strcspn( $remaining, '.' ); - if ( $pos === strlen( $remaining ) || $remaining[$pos] === '#' ) { + $pos += strcspn( $url, '.', $pos, $remainingLength ); + if ( $pos >= $urlLength ) { // End of string, we're done return false; } // We found a dot. Skip past it - $remaining = substr( $remaining, $pos + 1 ); + $pos++; + $remainingLength = $urlLength - $pos; + // Check for illegal characters in our prospective extension, - // or for another dot, or for a hash sign - $pos = strcspn( $remaining, "<>\\\"/:|?*.#" ); - if ( $pos === strlen( $remaining ) ) { - // No illegal character or next dot or hash + // or for another dot + $nextPos = $pos + strcspn( $url, "<>\\\"/:|?*.", $pos, $remainingLength ); + if ( $nextPos >= $urlLength ) { + // No illegal character or next dot // We have our extension - return $remaining; - } - if ( $remaining[$pos] === '#' ) { - // We've found a legal extension followed by a hash - // Trim the hash and everything after it, then return that - return substr( $remaining, 0, $pos ); + return substr( $url, $pos, $urlLength - $pos ); } - if ( $remaining[$pos] === '?' ) { + if ( $url[$nextPos] === '?' ) { // We've found a legal extension followed by a question mark // If the extension is NOT exe, dll or cgi, return it - $extension = substr( $remaining, 0, $pos ); + $extension = substr( $url, $pos, $nextPos - $pos ); if ( strcasecmp( $extension, 'exe' ) && strcasecmp( $extension, 'dll' ) && strcasecmp( $extension, 'cgi' ) ) { @@ -861,7 +869,8 @@ class WebRequest { } // We found an illegal character or another dot // Skip to that character and continue the loop - $remaining = substr( $remaining, $pos ); + $pos = $nextPos + 1; + $remainingLength = $urlLength - $pos; } return false; } diff --git a/tests/phpunit/includes/FindIE6ExtensionTest.php b/tests/phpunit/includes/FindIE6ExtensionTest.php index d1d534144a..534f0980dd 100644 --- a/tests/phpunit/includes/FindIE6ExtensionTest.php +++ b/tests/phpunit/includes/FindIE6ExtensionTest.php @@ -6,97 +6,113 @@ class FindIE6ExtensionTest extends MediaWikiTestCase { function testSimple() { $this->assertEquals( - WebRequest::findIE6Extension( 'x.y' ), 'y', + WebRequest::findIE6Extension( 'x.y' ), 'Simple extension' ); } function testSimpleNoExt() { $this->assertEquals( - WebRequest::findIE6Extension( 'x' ), '', + WebRequest::findIE6Extension( 'x' ), 'No extension' ); } function testEmpty() { $this->assertEquals( - WebRequest::findIE6Extension( '' ), '', + WebRequest::findIE6Extension( '' ), 'Empty string' ); } function testQuestionMark() { $this->assertEquals( - WebRequest::findIE6Extension( '?' ), '', + WebRequest::findIE6Extension( '?' ), 'Question mark only' ); } function testExtQuestionMark() { $this->assertEquals( - WebRequest::findIE6Extension( '.x?' ), 'x', + WebRequest::findIE6Extension( '.x?' ), 'Extension then question mark' ); } function testQuestionMarkExt() { $this->assertEquals( - WebRequest::findIE6Extension( '?.x' ), 'x', + WebRequest::findIE6Extension( '?.x' ), 'Question mark then extension' ); } function testInvalidChar() { $this->assertEquals( - WebRequest::findIE6Extension( '.x*' ), '', + WebRequest::findIE6Extension( '.x*' ), 'Extension with invalid character' ); } + function testInvalidCharThenExtension() { + $this->assertEquals( + 'x', + WebRequest::findIE6Extension( '*.x' ), + 'Invalid character followed by an extension' + ); + } + function testMultipleQuestionMarks() { $this->assertEquals( - WebRequest::findIE6Extension( 'a?b?.c?.d?e?f' ), 'c', + WebRequest::findIE6Extension( 'a?b?.c?.d?e?f' ), 'Multiple question marks' ); } function testExeException() { $this->assertEquals( - WebRequest::findIE6Extension( 'a?b?.exe?.d?.e' ), 'd', + WebRequest::findIE6Extension( 'a?b?.exe?.d?.e' ), '.exe exception' ); } function testExeException2() { $this->assertEquals( - WebRequest::findIE6Extension( 'a?b?.exe' ), 'exe', + WebRequest::findIE6Extension( 'a?b?.exe' ), '.exe exception 2' ); } function testHash() { $this->assertEquals( - WebRequest::findIE6Extension( 'a#b.c' ), '', + WebRequest::findIE6Extension( 'a#b.c' ), 'Hash character preceding extension' ); } function testHash2() { $this->assertEquals( - WebRequest::findIE6Extension( 'a?#b.c' ), '', + WebRequest::findIE6Extension( 'a?#b.c' ), 'Hash character preceding extension 2' ); } + + function testDotAtEnd() { + $this->assertEquals( + '', + WebRequest::findIE6Extension( '.' ), + 'Dot at end of string' + ); + } } -- 2.20.1