revolution icon indicating copy to clipboard operation
revolution copied to clipboard

[2.x] fix working with non-standart ports

Open dimasites opened this issue 1 year ago • 0 comments

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: explaination for pull request v2

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

dimasites avatar Jul 01 '23 22:07 dimasites