Skip to content

Commit

Permalink
Fix issue working with non-standard ports (#16455)
Browse files Browse the repository at this point in the history 
### 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](https://stackoverflow.com/questions/4504831/serverhttp-host-contains-port-number-too
) and
[this](https://stackoverflow.com/questions/2297403/what-is-the-difference-between-http-host-and-server-name-in-php
) 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](https://github.com/modxcms/revolution/assets/5102558/14a7cc38-ea43-4475-8b5e-6baf6f58c3e7)

### How to test
I have prepared a test script:
<details><summary>See code:
test_modx_working_with_http_ports.php</summary>

```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)

*/

```
</details>

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](Pixmill-Gmbh/modx-docker@ace541e)
in [MODX Docker](https://github.com/Pixmill-Gmbh/modx-docker) project
  • Loading branch information
@dimasites
dimasites authored Mar 22, 2024
1 parent a2bc18f commit 1371d06
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 19 deletions.
12 changes: 5 additions & 7 deletions core/docs/config.inc.tpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,22 +49,20 @@ if (!defined('MODX_BASE_PATH')) {
if(defined('PHP_SAPI') && (PHP_SAPI == "cli" || PHP_SAPI == "embed")) {
$isSecureRequest = false;
} else {
$isSecureRequest = ((isset($_SERVER['HTTPS']) && !empty($_SERVER['HTTPS']) && strtolower($_SERVER['HTTPS']) !== 'off') || $_SERVER['SERVER_PORT'] == $https_port);
$isSecureRequest = ((isset($_SERVER['HTTPS']) && !empty($_SERVER['HTTPS']) && strtolower($_SERVER['HTTPS']) !== 'off') || parse_url('http://' . $_SERVER['HTTP_HOST'], PHP_URL_PORT) == $https_port);
}
if (!defined('MODX_URL_SCHEME')) {
$url_scheme= $isSecureRequest ? 'https://' : 'http://';
$url_scheme = $isSecureRequest ? 'https://' : 'http://';
define('MODX_URL_SCHEME', $url_scheme);
}
if (!defined('MODX_HTTP_HOST')) {
if(defined('PHP_SAPI') && (PHP_SAPI == "cli" || PHP_SAPI == "embed")) {
$http_host = '{http_host}';
define('MODX_HTTP_HOST', $http_host);
} else {
$http_host= array_key_exists('HTTP_HOST', $_SERVER) ? htmlspecialchars($_SERVER['HTTP_HOST'], ENT_QUOTES) : '{http_host}';
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'];
$http_host = array_key_exists('HTTP_HOST', $_SERVER) ? parse_url($url_scheme . $_SERVER['HTTP_HOST'], PHP_URL_HOST) : '{http_host}';
$http_port = parse_url($url_scheme . $_SERVER['HTTP_HOST'], PHP_URL_PORT);
$http_host .= in_array($http_port, [null, 80, 443]) ? '' : ':' . $http_port;
define('MODX_HTTP_HOST', $http_host);
}
}
Expand Down
9 changes: 3 additions & 6 deletions setup/includes/config/modconfigreader.class.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,9 @@ public function loadDefaults(array $config = array()) {
public function getHttpHost() {
if (php_sapi_name() != 'cli') {
$this->config['https_port'] = isset($_POST['httpsport']) ? $_POST['httpsport'] : 443;
$isSecureRequest = ((isset($_SERVER['HTTPS']) && !empty($_SERVER['HTTPS']) && strtolower($_SERVER['HTTPS']) !== 'off') || $_SERVER['SERVER_PORT'] == $this->config['https_port']);
$this->config['http_host'] = $_SERVER['HTTP_HOST'];
if ($_SERVER['SERVER_PORT'] != 80) {
$this->config['http_host'] = str_replace(':' . $_SERVER['SERVER_PORT'], '', $this->config['http_host']);
}
$this->config['http_host'] .= in_array($_SERVER['SERVER_PORT'], [80, 443]) ? '' : ':' . $_SERVER['SERVER_PORT'];
$this->config['http_host'] = parse_url('http://' . $_SERVER['HTTP_HOST'], PHP_URL_HOST);
$this->config['http_port'] = parse_url('http://' . $_SERVER['HTTP_HOST'], PHP_URL_PORT);
$this->config['http_host'] .= in_array($this->config['http_port'], [null , 80, 443]) ? '' : ':' . $this->config['http_port'];
} else {
$this->config['http_host'] = 'localhost';
$this->config['https_port'] = 443;
Expand Down
11 changes: 5 additions & 6 deletions setup/index.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,11 @@
}
if (!$isCommandLine) {
$https = isset($_SERVER['HTTPS']) ? $_SERVER['HTTPS'] : false;
$installBaseUrl = (!$https || strtolower($https) != 'on') ? 'http://' : 'https://';
$installBaseUrl .= $_SERVER['HTTP_HOST'];
if (isset($_SERVER['SERVER_PORT']) && (string)$_SERVER['SERVER_PORT'] !== '' && $_SERVER['SERVER_PORT'] !== 80) {
$installBaseUrl = str_replace(':' . $_SERVER['SERVER_PORT'], '', $installBaseUrl);
}
$installBaseUrl .= in_array($_SERVER['SERVER_PORT'], [80, 443]) ? '' : ':' . $_SERVER['SERVER_PORT'];
$url_scheme = (!$https || strtolower($https) != 'on') ? 'http://' : 'https://';
$installBaseUrl = $url_scheme;
$installBaseUrl .= parse_url($url_scheme . $_SERVER['HTTP_HOST'], PHP_URL_HOST);
$url_port = parse_url($url_scheme . $_SERVER['HTTP_HOST'], PHP_URL_PORT);
$installBaseUrl .= in_array($url_port, [null , 80, 443]) ? '' : ':' . $url_port;
$installBaseUrl .= $_SERVER['SCRIPT_NAME'];
$installBaseUrl = htmlspecialchars($installBaseUrl, ENT_QUOTES, 'utf-8');
define('MODX_SETUP_URL', $installBaseUrl);
Expand Down

0 comments on commit 1371d06

Please sign in to comment.