Fix issue working with non-standard ports

[dimasites]

What does it do?

I changed logic of modx working with non-standard ports because old logic in some cases works not correct: port :44333 MODX cut ":443" and "33" part breaks the links adresses. See #16428 for details.

Relying on here this and this I suggest using the answers in stackoverflow to check the port not $_SERVER['SERVER_PORT'] but parse_url($_SERVER['HTTP_HOST'], PHP_URL_PORT) because it works more reliably and correctly, as can be seen from my tests.

Why is it needed?

Currently, when using a non-standard port for https (any one other than 443) MODX modifies request URL with mistakes, and redirect users's browser to wrong url.

See my explaination on awesome handmade infographic :) image:

How to test

I have prepared a test script:

See code: test_modx_working_with_http_ports.php

var_dump(parse_url($_SERVER['HTTP_HOST'], PHP_URL_PORT)); echo ' ← parsed port from HTTP_HOST <br>';
var_dump($_SERVER['SERVER_PORT']); echo ' ← SERVER_PORT <br>';

$http_port = parse_url($_SERVER['HTTP_HOST'], PHP_URL_PORT) ?: $_SERVER['SERVER_PORT'];
var_dump($_SERVER['SERVER_PORT']); echo ' ← http_port for usage in fixed config <br>';

$http_host = array_key_exists('HTTP_HOST', $_SERVER) ? htmlspecialchars($_SERVER['HTTP_HOST'], ENT_QUOTES) : 'test-port.ltd';

//this is for test logic with non-standart port without server changes, COMMENT IR FOR REAL SERVER TEST
$http_host = 'test-port.ltd:44333';

echo '<br>';
var_dump($http_host); echo ' ← http_host (old logic)<br>';

$http_host_new_logic = parse_url($http_host, PHP_URL_HOST); 
var_dump($http_host_new_logic); echo ' ← http_host (new logic)<br>';


/*old logic */
if ($_SERVER['SERVER_PORT'] !== 80) {
	
	$orig_http_host = str_replace(':' . $_SERVER['SERVER_PORT'], '', $http_host);
	$fixed_http_host = str_replace(':' . $http_port, '', $http_host);
	$fixed_http_host_new_logic = str_replace(':' . $http_port, '', $http_host_new_logic);
	
	$http_host = str_replace(':' . $_SERVER['SERVER_PORT'], '', $http_host);
}
echo '<br>';
var_dump($orig_http_host); echo ' ← orig_http_host after check port and clean host (old logic)<br>';
var_dump($fixed_http_host); echo ' ← fixed_http_host after check port and clean host (old logic)<br>';
var_dump($fixed_http_host_new_logic); echo ' ← fixed_http_host after check port and clean host (new logic)<br>';

$http_host .= in_array($_SERVER['SERVER_PORT'], [80, 443]) ? '' : ':' . $_SERVER['SERVER_PORT'];

$orig_http_host = in_array($_SERVER['SERVER_PORT'], [80, 443]) ? $http_host.'' : $http_host.':' . $_SERVER['SERVER_PORT'];

$fixed_http_host = !in_array($http_port, [80, 443]) ? $fixed_http_host.'' : $fixed_http_host.':' . $http_port;

$fixed_http_host_new_logic = !in_array($http_port, [80, 443]) ? $fixed_http_host_new_logic.'' : $fixed_http_host_new_logic.':' . $http_port;
echo '<br>';
var_dump($orig_http_host); echo ' ← orig_http_host after replace (old logic)<br>';
var_dump($fixed_http_host); echo ' ← fixed_http_host after replace (old logic)<br>';
var_dump($fixed_http_host_new_logic); echo ' ← fixed_http_host after replace (new logic)<br>';


////////// SPEED TESTS //////////
echo '<br>';

$time_start = microtime(true);
for($i=0;$i<=1000000;$i++){
    // Method 1 Original modx 2.8.5
		$http_host = array_key_exists('HTTP_HOST', $_SERVER) ? htmlspecialchars($_SERVER['HTTP_HOST'], ENT_QUOTES) : 'test-port.ltd:44333';
		
        if ($_SERVER['SERVER_PORT'] !== 80) {
            $http_host = str_replace(':' . $_SERVER['SERVER_PORT'], '', $http_host);
        }
        $http_host .= in_array($_SERVER['SERVER_PORT'], [80, 443]) ? '' : ':' . $_SERVER['SERVER_PORT'];
        define('MODX_HTTP_HOST', $http_host);
}
$time_end = microtime(true);
$time = $time_end - $time_start;
echo "Original modx 2.8.5 config test: ($time seconds)\n<br />";



$time_start = microtime(true);
for($i=0;$i<=1000000;$i++){
    // Method2 Fixed working with ports by dimasites
		$http_host = array_key_exists('HTTP_HOST', $_SERVER) ? parse_url($_SERVER['HTTP_HOST'], PHP_URL_HOST) : 'test-port.ltd:44333';
		
		$http_port = parse_url($_SERVER['HTTP_HOST'], PHP_URL_PORT)?:$_SERVER['SERVER_PORT'];
		
        $http_host .= in_array($http_port, [80, 443]) ? '' : ':' . $http_port;
        
		define('MODX_HTTP_HOST', $http_host);

}
$time_end = microtime(true);
$time = $time_end - $time_start;
echo "Fixed working with ports config: ($time seconds)\n";

/*
My results on PHP 7.4 about:
Hosting server 1 (client:mlk):
Original modx 2.8.5 config test: (3.4942800998688 seconds)
Fixed working with ports config: (3.3323838710785 seconds)

Hosting server 2 (main):
Original modx 2.8.5 config test: (1.6090040206909 seconds)
Fixed working with ports config: (1.4196999073029 seconds)


For URL: https://test-port.ltd/test_modx_working_with_http_ports.php
NULL ← parsed port from HTTP_HOST
string(3) "443" ← SERVER_PORT
string(3) "443" ← http_port for usage in fixed config

string(19) "test-port.ltd:44333" ← http_host (old logic)
string(13) "test-port.ltd" ← http_host (new logic)

string(15) "test-port.ltd33" ← orig_http_host after check port and clean host (old logic)
string(15) "test-port.ltd33" ← fixed_http_host after check port and clean host (old logic)
string(13) "test-port.ltd" ← fixed_http_host after check port and clean host (new logic)

string(15) "test-port.ltd33" ← orig_http_host after replace (old logic)
string(19) "test-port.ltd33:443" ← fixed_http_host after replace (old logic)
string(17) "test-port.ltd:443" ← fixed_http_host after replace (new logic)

Original modx 2.8.5 config test: (1.7936890125275 seconds)
Fixed working with ports config: (1.5702078342438 seconds)


For URL: https://test-port.ltd:44333/test_modx_working_with_http_ports.php
int(44333) ← parsed port from HTTP_HOST
string(3) "443" ← SERVER_PORT
string(3) "443" ← http_port for usage in fixed config

string(19) "test-port.ltd:44333" ← http_host (old logic)
string(13) "test-port.ltd" ← http_host (new logic)

string(15) "test-port.ltd33" ← orig_http_host after check port and clean host (old logic)
string(13) "test-port.ltd" ← fixed_http_host after check port and clean host (old logic)
string(13) "test-port.ltd" ← fixed_http_host after check port and clean host (new logic)

string(15) "test-port.ltd33" ← orig_http_host after replace (old logic)
string(13) "test-port.ltd" ← fixed_http_host after replace (old logic)
string(13) "test-port.ltd" ← fixed_http_host after replace (new logic)

Original modx 2.8.5 config test: (3.5288610458374 seconds)
Fixed working with ports config: (3.4201400279999 seconds)

*/

with a test and comparison of values, as well as speed tests in relation to the replacement of string processing functions (and new logic even a little faster)

Related issue(s)/PR(s)

Resolves #16428 and this hand crutch in MODX Docker project

[opengeek]

@dimasites — can you review and sign the CLA at https://modx.com/community/cla-introduction/ so we can consider getting this contribution into the next release?

[dimasites]

Hi @opengeek i signed it via gitgub's cla-bot when pushed commit earlier, and now additionally signed CLA on modx.com:

And if you finished review with no suggestions, i can check MODX 3 behavior with non-standart ports. Now waiting you response, thx

[opengeek]

Hi @opengeek i signed it via gitgub's cla-bot when pushed commit earlier, and now additionally signed CLA on modx.com:

The cla-bot stuff isn't working any longer for us, and it should have just directed you to sign on modx.com anyway. Very odd. But I have confirmed now via our internal systems.

And if you finished review with no suggestions, i can check MODX 3 behavior with non-standart ports. Now waiting you response, thx

See my review comment…