Tim Landscheidt has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/171542

Change subject: WIP: Use wfParseUrl() in Http::isValidURI() and fix tests
......................................................................

WIP: Use wfParseUrl() in Http::isValidURI() and fix tests

Instead of building yet another regular expression, let's use
wfParseUrl() and ensure that the scheme is either "http" or "https"
and the hostname is not empty.

Two tests are relaxed:

- 'http://!"èèè¿¿¿~~\'' is now a valid URL.  My understanding is that
  Http::isValidURI() is used on user input, and with IDN, this may
  well be a legitimate hostname.

- 'http://a ¿non !!sens after' [FIXME]

Change-Id: I3b45c71e53c0c85768647b1d4c684c2eb12be229
---
M includes/HttpFunctions.php
M tests/phpunit/includes/HttpTest.php
2 files changed, 11 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/42/171542/1

diff --git a/includes/HttpFunctions.php b/includes/HttpFunctions.php
index f9ee14b..ca4da13 100644
--- a/includes/HttpFunctions.php
+++ b/includes/HttpFunctions.php
@@ -172,10 +172,14 @@
         * @return bool
         */
        public static function isValidURI( $uri ) {
-               return preg_match(
-                       '/^https?:\/\/[^\/\s]\S*$/D',
-                       $uri
-               );
+               $i = wfParseUrl( $uri );
+
+               if ($i === false) {
+                       return false;
+               }
+
+               return ($i ['scheme'] === 'http' || $i ['scheme'] === 'https') 
&&
+                               $i['host'] != '';
        }
 }
 
diff --git a/tests/phpunit/includes/HttpTest.php 
b/tests/phpunit/includes/HttpTest.php
index fbd2c31..8d33cc4 100644
--- a/tests/phpunit/includes/HttpTest.php
+++ b/tests/phpunit/includes/HttpTest.php
@@ -93,10 +93,8 @@
                        array( true, 'http://user:pass@host', 'Username and 
password provided' ),
 
                        # (\S+) - host part is made of anything not whitespaces
-                       // commented these out in order to remove @group Broken
-                       // @todo are these valid tests? if so, fix 
Http::isValidURI so it can handle them
-                       //array( false, 'http://!"èèè¿¿¿~~\'', 'hostname is 
made of any non whitespace' ),
-                       //array( false, 'http://exam:ple.org/', 'hostname can 
not use colons!' ),
+                       array( true, 'http://!"èèè¿¿¿~~\'', 'hostname is made 
of any non whitespace' ),
+                       array( false, 'http://exam:ple.org/', 'hostname can not 
use colons!' ),
 
                        # (:[0-9]+)? - port number
                        array( true, 'http://example.org:80/' ),
@@ -125,7 +123,7 @@
                        array( true, 'http://example.org/?id#anchor' ),
                        array( true, 'http://example.org/?#anchor' ),
 
-                       array( false, 'http://a ¿non !!sens after', 'Allow 
anything after URI' ),
+                       array( true, 'http://a ¿non !!sens after', 'Allow 
anything after URI' ),
                );
        }
 

-- 
To view, visit https://gerrit.wikimedia.org/r/171542
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I3b45c71e53c0c85768647b1d4c684c2eb12be229
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Tim Landscheidt <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to