bcosca / fatfree

A powerful yet easy-to-use PHP micro-framework designed to help you build dynamic and robust Web applications - fast!
2.66k stars 446 forks source link

Base::reroute fails when SERVER_PORT is empty string #1194

Closed sorbits closed 4 years ago

sorbits commented 4 years ago

I am using F3 v3.7.2, PHP7.4-fpm, Caddy v2.1.1 and Ubuntu 20.04.

Caddy sets SERVER_PORT to an empty string which is handled correctly by F3 when setting up REALM, here the code is as follows:

($port && !in_array($port,[80,443])?(':'.$port):'')

But it fails in Base::reroute where the code is:

$port=$this->hive['PORT'];
$port=in_array($port,[80,443])?'':(':'.$port);

I propose never setting PORT to an empty string with the following patch:

--- base.php (old version)
+++ base.php (new version)
@@ -2433,9 +2433,9 @@
            'samesite'=>'Lax',
        ];
        $port=80;
-       if (isset($headers['X-Forwarded-Port']))
+       if (!empty($headers['X-Forwarded-Port']))
            $port=$headers['X-Forwarded-Port'];
-       elseif (isset($_SERVER['SERVER_PORT']))
+       elseif (!empty($_SERVER['SERVER_PORT']))
            $port=$_SERVER['SERVER_PORT'];
        // Default configuration
        $this->hive+=[
@@ -2496,7 +2496,7 @@
            'QUIET'=>FALSE,
            'RAW'=>FALSE,
            'REALM'=>$scheme.'://'.$_SERVER['SERVER_NAME'].
-               ($port && !in_array($port,[80,443])?(':'.$port):'').
+               (!in_array($port,[80,443])?(':'.$port):'').
                $_SERVER['REQUEST_URI'],
            'RESPONSE'=>'',
            'ROOT'=>$_SERVER['DOCUMENT_ROOT'],

For consistency we also check X-Forwarded-Port with !empty and remove the null check for $port when setting up REALM (to make the code consistent with Base::reroute).

ikkez commented 4 years ago

Hi. Sounds good. Do you want to create a pull request for fatfree-core?