Merge pull request #768 from tvdijen/fix-serverPort

rewrite getServerPort

lib/SimpleSAML/Utils/HTTP.php

@@ -108,15 +108,11 @@ class HTTP
      */
     public static function getServerPort()
     {
-        $port = (isset($_SERVER['SERVER_PORT'])) ? $_SERVER['SERVER_PORT'] : '80';
-        if (self::getServerHTTPS()) {
-            if ($port !== '443') {
-                return ':'.$port;
-            }
-        } else {
-            if ($port !== '80') {
-                return ':'.$port;
-            }
+        $default_port = self::getServerHTTPS() ? '443' : '80';
+        $port = isset($_SERVER['SERVER_PORT']) ? $_SERVER['SERVER_PORT'] : $default_port;
+        
+        if ($port !== $default_port) {
+            return ':'.$port;
         }
         return '';
     }

tests/lib/SimpleSAML/Utils/HTTPTest.php

@@ -6,8 +6,6 @@ use SimpleSAML\Utils\HTTP;
 
 class HTTPTest extends TestCase
 {
-
-
     /**
      * Set up the environment ($_SERVER) populating the typical variables from a given URL.
      *
@@ -120,7 +118,6 @@ class HTTPTest extends TestCase
         $_SERVER = $original;
     }
 
-
     /**
      * Test SimpleSAML\Utils\HTTP::getSelfHost() with and without custom port.
      */
@@ -166,7 +163,6 @@ class HTTPTest extends TestCase
         $_SERVER = $original;
     }
 
-
     /**
      * Test SimpleSAML\Utils\HTTP::getSelfURL().
      */
@@ -292,7 +288,6 @@ class HTTPTest extends TestCase
         $_SERVER = $original;
     }
 
-
     /**
      * Test SimpleSAML\Utils\HTTP::checkURLAllowed(), without regex.
      */
@@ -355,6 +350,42 @@ class HTTPTest extends TestCase
         $_SERVER = $original;
     }
 
+    /**
+     * Test SimpleSAML\Utils\HTTP::getServerPort().
+     */
+    public function testGetServerPort()
+    {
+        $original = $_SERVER;
+
+        // Test HTTP + non-standard port
+        $_SERVER['HTTPS'] = 'off';
+        $_SERVER['SERVER_PORT'] = '3030';
+        $this->assertEquals(HTTP::getServerPort(), ':3030');
+
+        // Test HTTP + standard port
+        $_SERVER['SERVER_PORT'] = '80';
+        $this->assertEquals(HTTP::getServerPort(), '');
+
+        // Test HTTP + without port
+        unset($_SERVER['SERVER_PORT']);
+        $this->assertEquals(HTTP::getServerPort(), '');
+
+        // Test HTTPS + non-standard port
+        $_SERVER['HTTPS'] = 'on';
+        $_SERVER['SERVER_PORT'] = '3030';
+        $this->assertEquals(HTTP::getServerPort(), ':3030');
+
+        // Test HTTPS + standard port
+        $_SERVER['SERVER_PORT'] = '443';
+        $this->assertEquals(HTTP::getServerPort(), '');
+
+        // Test HTTPS + without port
+        unset($_SERVER['SERVER_PORT']);
+        $this->assertEquals(HTTP::getServerPort(), '');
+
+        $_SERVER = $original;
+    }
+
     /**
      * Test SimpleSAML\Utils\HTTP::checkURLAllowed(), with the regex as a
      * subdomain of an evil domain.