[project @ Fix trust root test runner and then all failing tests that were exposed]

4 files changed, 75 insertions, 29 deletions

diff --git a/Auth/OpenID/TrustRoot.php b/Auth/OpenID/TrustRoot.php
index 16e4de6..e3727a3 100644
--- a/Auth/OpenID/TrustRoot.php
+++ b/Auth/OpenID/TrustRoot.php
@@ -32,7 +32,11 @@ define('Auth_OpenID___TLDs',
        'nc|ne|nf|ng|ni|nl|no|np|nr|nu|nz|om|pa|pe|pf|pg|ph|pk|pl|pm|pn|pr|' .
        'ps|pt|pw|py|qa|re|ro|ru|rw|sa|sb|sc|sd|se|sg|sh|si|sj|sk|sl|sm|sn|' .
        'so|sr|st|sv|sy|sz|tc|td|tf|tg|th|tj|tk|tm|tn|to|tp|tr|tt|tv|tw|tz|' .
-       'ua|ug|uk|um|us|uy|uz|va|vc|ve|vg|vi|vn|vu|wf|ws|ye|yt|yu|za|zm|zw)$/');
+       'ua|ug|uk|um|us|uy|uz|va|vc|ve|vg|vi|vn|vu|wf|ws|ye|yt|yu|za|zm|zw)' .
+       '\.?$/');
+
+define('Auth_OpenID___HostSegmentRe',
+       "/^(?:[-a-zA-Z0-9!$&'\\(\\)\\*+,;=._~]|%[a-zA-Z0-9]{2})*$/");
 
 /**
  * A wrapper for trust-root related functions
@@ -86,10 +90,16 @@ class Auth_OpenID_TrustRoot {
      */
     function _parse($trust_root)
     {
+        $trust_root = Auth_OpenID_urinorm($trust_root);
+        if ($trust_root === null) {
+            return false;
+        }
+
         $parts = @parse_url($trust_root);
         if ($parts === false) {
             return false;
         }
+
         $required_parts = array('scheme', 'host');
         $forbidden_parts = array('user', 'pass', 'fragment');
         $keys = array_keys($parts);
@@ -101,9 +111,7 @@ class Auth_OpenID_TrustRoot {
             return false;
         }
 
-        // Return false if the original trust root value has more than
-        // one port specification.
-        if (preg_match("/:\/\/[^:]+(:\d+){2,}(\/|$)/", $trust_root)) {
+        if (!preg_match(Auth_OpenID___HostSegmentRe, $parts['host'])) {
             return false;
         }
 
@@ -139,6 +147,9 @@ class Auth_OpenID_TrustRoot {
 
         if (isset($parts['path'])) {
             $path = strtolower($parts['path']);
+            if (substr($path, 0, 1) != '/') {
+                return false;
+            }
         } else {
             $path = '/';
         }
@@ -148,6 +159,7 @@ class Auth_OpenID_TrustRoot {
             $parts['port'] = false;
         }
 
+
         $parts['unparsed'] = $trust_root;
 
         return $parts;
@@ -189,6 +201,25 @@ class Auth_OpenID_TrustRoot {
         if ($parts['host'] == 'localhost') {
             return true;
         }
+        
+        $host_parts = explode('.', $parts['host']);
+        if ($parts['wildcard']) {
+            // Remove the empty string from the beginning of the array
+            array_shift($host_parts);
+        }
+
+        if ($host_parts && !$host_parts[count($host_parts) - 1]) {
+            array_pop($host_parts);
+        }
+
+        if (!$host_parts) {
+            return false;
+        }
+
+        // Don't allow adjacent dots
+        if (in_array('', $host_parts, true)) {
+            return false;
+        }
 
         // Get the top-level domain of the host. If it is not a valid TLD,
         // it's not sane.
@@ -198,19 +229,20 @@ class Auth_OpenID_TrustRoot {
         }
         $tld = $matches[1];
 
-        // Require at least two levels of specificity for non-country
-        // tlds and three levels for country tlds.
-        $elements = explode('.', $parts['host']);
-        $n = count($elements);
-        if ($parts['wildcard']) {
-            $n -= 1;
-        }
-        if (strlen($tld) == 2) {
-            $n -= 1;
-        }
-        if ($n <= 1) {
+        if (count($host_parts) == 1) {
             return false;
         }
+
+        if ($parts['wildcard']) {
+            // It's a 2-letter tld with a short second to last segment
+            // so there needs to be more than two segments specified
+            // (e.g. *.co.uk is insane)
+            $second_level = $host_parts[count($host_parts) - 2];
+            if (strlen($tld) == 2 && strlen($second_level) <= 3) {
+                return count($host_parts) > 2;
+            }
+        }
+
         return true;
     }
 
@@ -258,8 +290,14 @@ class Auth_OpenID_TrustRoot {
         $base_path = $trust_root_parsed['path'];
         $path = $url_parsed['path'];
         if (!isset($trust_root_parsed['query'])) {
-            if (substr($path, 0, strlen($base_path)) != $base_path) {
-                return false;
+            if ($base_path != $path) {
+                if (substr($path, 0, strlen($base_path)) != $base_path) {
+                    return false;
+                }
+                if (substr($base_path, strlen($base_path) - 1, 1) != '/' &&
+                    substr($path, strlen($base_path), 1) != '/') {
+                    return false;
+                }
             }
         } else {
             $base_query = $trust_root_parsed['query'];
diff --git a/Tests/Auth/OpenID/Parse.php b/Tests/Auth/OpenID/Parse.php
index 7acca99..9a3547c 100644
--- a/Tests/Auth/OpenID/Parse.php
+++ b/Tests/Auth/OpenID/Parse.php
@@ -183,4 +183,4 @@ class Tests_Auth_OpenID_Parse extends PHPUnit_TestSuite {
     }
 }
 
-?>
\ No newline at end of file
+?>
diff --git a/Tests/Auth/OpenID/TrustRoot.php b/Tests/Auth/OpenID/TrustRoot.php
index 95d3dec..9c7c40e 100644
--- a/Tests/Auth/OpenID/TrustRoot.php
+++ b/Tests/Auth/OpenID/TrustRoot.php
@@ -22,16 +22,16 @@ class Tests_Auth_OpenID_TRParseCase extends PHPUnit_TestCase {
         $parsed = (bool)Auth_OpenID_TrustRoot::_parse($this->case);
         switch ($this->expected) {
         case 'sane':
-            $this->assertTrue($is_sane);
-            $this->assertTrue($parsed);
+            $this->assertTrue($parsed, "Did not parse");
+            $this->assertTrue($is_sane, "Is not sane");
             break;
         case 'insane':
-            $this->assertTrue($parsed);
-            $this->assertFalse($is_sane);
+            $this->assertTrue($parsed, "Did not parse");
+            $this->assertFalse($is_sane, "Is sane");
             break;
         default:
-            $this->assertFalse($parsed);
-            $this->assertFalse($is_sane);
+            $this->assertFalse($parsed, "Did parse");
+            $this->assertFalse($is_sane, "Is sane");
         }
     }
 }
@@ -161,9 +161,13 @@ class Tests_Auth_OpenID_TrustRoot extends PHPUnit_TestSuite {
         $this->setName($name);
 
         foreach (Tests_Auth_OpenID_trustRootTests() as $test) {
-            $this->addTest($test);
+            $this->_addTestByValue($test);
         }
     }
+
+    function _addTestByValue($test) {
+        $this->addTest($test);
+    }
 }
 
 ?>
diff --git a/Tests/Auth/OpenID/data/trustroot.txt b/Tests/Auth/OpenID/data/trustroot.txt
index f6414bf..8194409 100644
--- a/Tests/Auth/OpenID/data/trustroot.txt
+++ b/Tests/Auth/OpenID/data/trustroot.txt
@@ -3,7 +3,7 @@ Trust root parsing checking
 ========================================
 
 ----------------------------------------
-15: Does not parse
+18: Does not parse
 ----------------------------------------
 baz.org
 *.foo.com
@@ -15,16 +15,18 @@ foo.*.com
 http://foo.*.com
 http://www.*
 http://*foo.com/
+http://foo.com\/
+http://localhost:1900foo/
 http://foo.com/invalid#fragment
 
  
  	
 5
+http:///
 
 ----------------------------------------
-16: Insane
+15: Insane
 ----------------------------------------
-http:///
 http://*/
 https://*/
 http://*.com
@@ -68,7 +70,7 @@ return_to matching
 ========================================
 
 ----------------------------------------
-42: matches
+44: matches
 ----------------------------------------
 http://*/                             http://cnn.com/
 http://*/                             http://livejournal.com/
@@ -89,6 +91,8 @@ http://*.b.foo.com                    http://w.x.b.foo.com
 http://*.bar.co.uk                    http://www.bar.co.uk
 http://*.uoregon.edu                  http://x.cs.uoregon.edu
 http://x.com/abc                      http://x.com/abc
+http://127.1/abc                      http://127.1/abc
+http://10.0.0.1/abc                   http://10.0.0.1/abc
 http://x.com/abc                      http://x.com/abc/def
 http://*.x.com                        http://x.com/gallery
 http://*.x.com                        http://foo.x.com/gallery