r83296 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r83295‎ | r83296 | r83297 >
Date:16:15, 5 March 2011
Author:hashar
Status:resolved (Comments)
Tags:
Comment:
Tests for bug 27854 - Http::isValidURI is to lax

Those tests are made to help someone fix bug 27854. You might want to add
some more. Please note we want this function to only validate curl supported
URI, hence the hardcoded protocols http,https and ftp.

Some of those tests currently fail because of bug 27854.
Modified paths:
  • /trunk/phase3/tests/phpunit/includes/HttpTest.php (modified) (history)

Diff [purge]

Index: trunk/phase3/tests/phpunit/includes/HttpTest.php
@@ -522,4 +522,85 @@
523523 Http::$httpEngine = 'curl';
524524 $this->runCookieRequests();
525525 }
 526+
 527+ /**
 528+ * Test Http::isValidURI()
 529+ * @bug 27854 : Http::isValidURI is to lax
 530+ *@dataProvider provideURI */
 531+ function testIsValidUri( $expect, $URI, $message = '' ) {
 532+ $this->assertEquals(
 533+ $expect,
 534+ (bool) Http::isValidURI( $URI ),
 535+ $message
 536+ );
 537+ }
 538+
 539+ /**
 540+ * Feeds URI to test a long regular expression in Http::isValidURI
 541+ */
 542+ function provideURI() {
 543+ /** Format: 'boolean expectation', 'URI to test', 'Optional message' */
 544+ return array(
 545+ array( false, '¿non sens before!! http://a', 'Allow anything before URI' ),
 546+
 547+ # (ftp|http|https) - only three schemes allowed
 548+ array( true, 'http://www.example.org/' ),
 549+ array( true, 'https://www.example.org/' ),
 550+ array( true, 'ftp://www.example.org/' ),
 551+ array( true, 'http://www.example.org', 'URI without directory' ),
 552+ array( true, 'http://a', 'Short name' ),
 553+ array( true, 'http://étoile', 'Allow UTF-8 in hostname' ), # 'étoile' is french for 'star'
 554+ array( false, '\\host\directory', 'CIFS share' ),
 555+ array( false, 'gopher://host/dir', 'Reject gopher scheme' ),
 556+ array( false, 'telnet://host', 'Reject telnet scheme' ),
 557+
 558+ # :\/\/ - double slashes
 559+ array( false, 'http//example.org', 'Reject missing column in protocol' ),
 560+ array( false, 'http:/example.org', 'Reject missing slash in protocol' ),
 561+ array( false, 'http:example.org', 'Must have two slashes' ),
 562+ # Following fail since hostname can be made of anything
 563+ array( false, 'http:///example.org', 'Must have exactly two slashes, not three' ),
 564+
 565+ # (\w+:{0,1}\w*@)? - optional user:pass
 566+ array( true, 'http://user@host', 'Username provided' ),
 567+ array( true, 'http://user:@host', 'Username provided, no password' ),
 568+ array( true, 'http://user:pass@host', 'Username and password provided' ),
 569+
 570+ # (\S+) - host part is made of anything not whitespaces
 571+ array( false, 'http://!"èèè¿¿¿~~\'', 'hostname is made of any non whitespace' ),
 572+ array( false, 'http://exam:ple.org/', 'hostname can not use columns!' ),
 573+
 574+ # (:[0-9]+)? - port number
 575+ array( true, 'http://example.org:80/' ),
 576+ array( true, 'https://example.org:80/' ),
 577+ array( true, 'http://example.org:443/' ),
 578+ array( true, 'https://example.org:443/' ),
 579+ array( true, 'ftp://example.org:1/', 'Minimum' ),
 580+ array( true, 'ftp://example.org:65535/', 'Maximum port number' ),
 581+
 582+ # Part after the hostname is / or / with something else
 583+ array( true, 'http://example/#' ),
 584+ array( true, 'http://example/!' ),
 585+ array( true, 'http://example/:' ),
 586+ array( true, 'http://example/.' ),
 587+ array( true, 'http://example/?' ),
 588+ array( true, 'http://example/+' ),
 589+ array( true, 'http://example/=' ),
 590+ array( true, 'http://example/&' ),
 591+ array( true, 'http://example/%' ),
 592+ array( true, 'http://example/@' ),
 593+ array( true, 'http://example/-' ),
 594+ array( true, 'http://example//' ),
 595+ array( true, 'http://example/&' ),
 596+
 597+ # Fragment
 598+ array( true, 'http://exam#ple.org', ), # This one is valid, really!
 599+ array( true, 'http://example.org:80#anchor' ),
 600+ array( true, 'http://example.org/?id#anchor' ),
 601+ array( true, 'http://example.org/?#anchor' ),
 602+
 603+ array( false, 'http://a ¿non !!sens after', 'Allow anything after URI' ),
 604+ );
 605+ }
 606+
526607 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r83360(bug 27854) Http::isValidURI is way to lax. This is a much simplified regex t...bawolff02:43, 6 March 2011
r83515switch 'column' for 'colon'...hashar07:27, 8 March 2011

Comments

#Comment by Nikerabbit (talk | contribs)   16:43, 5 March 2011

+array( false, 'http//example.org', 'Reject missing column in protocol' ),

+array( false, 'http://exam:ple.org/', 'hostname can not use columns!' ),

Colon?

#Comment by Hashar (talk | contribs)   07:28, 8 March 2011

Yes: colon. It is in follow up r83515. Thanks!

Status & tagging log