Skip to content

Commit fe60682

Browse files
ioigoumeioigoume
authored
BUGFIX: http-getselfurl-robust-path-and-query (#2589)
* Improve HTTP::getSelfURL() path detection and URL reconstruction * Revert reconstruction of URL from parts when REQUEST_URI can be used.Add specific test for the modrewrite scenario.
1 parent bc52390 commit fe60682

File tree

2 files changed

+83
-12
lines changed

2 files changed

+83
-12
lines changed

‎src/SimpleSAML/Utils/HTTP.php‎

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -785,12 +785,19 @@ public function getSelfURL(): string
785785
$cur_path = realpath($_SERVER['SCRIPT_FILENAME']);
786786
// make sure we got a string from realpath()
787787
$cur_path = is_string($cur_path) ? $cur_path : '';
788+
788789
// find the path to the current script relative to the public/ directory of SimpleSAMLphp
789790
$rel_path = str_replace($baseDir . 'public' . DIRECTORY_SEPARATOR, '', $cur_path);
790-
// convert that relative path to an HTTP query
791+
791792
$url_path = str_replace(DIRECTORY_SEPARATOR, '/', $rel_path);
792-
// find where the relative path starts in the current request URI
793-
$uri_pos = (!empty($url_path)) ? strpos($_SERVER['REQUEST_URI'] ?? '', $url_path) : false;
793+
794+
$requestUri = (string)($_SERVER['REQUEST_URI'] ?? '');
795+
$requestPath = (string)parse_url($requestUri, PHP_URL_PATH);
796+
$requestQuery = (string)parse_url($requestUri, PHP_URL_QUERY);
797+
$requestFragment = (string)parse_url($requestUri, PHP_URL_FRAGMENT);
798+
799+
// Match script-relative path only against the path part of the request
800+
$uri_pos = (!empty($url_path)) ? strpos($requestPath, $url_path) : false;
794801

795802
if ($cur_path == $rel_path || $uri_pos === false) {
796803
/*
@@ -799,12 +806,13 @@ public function getSelfURL(): string
799806
* - $_SERVER['SCRIPT_FILENAME'] points to a script that doesn't exist. E.g. functional testing. In this
800807
* case, realpath() returns false and str_replace an empty string, so we compare them loosely.
801808
*
802-
* - The URI requested does not belong to a script in the public/ directory of SimpleSAMLphp. In that case,
803-
* removing SimpleSAMLphp's base dir from the current path yields the same path, so $cur_path and
809+
* - The script is not located under the public/ directory of SimpleSAMLphp. In that case, removing
810+
* SimpleSAMLphp's base dir and public/ from the current path yields the same path, so $cur_path and
804811
* $rel_path are equal.
805812
*
806-
* - The request URI does not match the current script. Even if the current script is located in the
807-
* public/ directory of SimpleSAMLphp, the URI does not contain its relative path, and $uri_pos is false.
813+
* - The request path does not match the current script. Even if the current script is located in the
814+
* public/ directory of SimpleSAMLphp, the request path (without query string) does not contain its
815+
* relative path, and $uri_pos is false.
808816
*
809817
* It doesn't matter which one of those cases we have. We just know we can't apply our base URL to the
810818
* current URI, so we need to build it back from the PHP environment, unless we have a base URL specified
@@ -814,20 +822,33 @@ public function getSelfURL(): string
814822
$appurl = ($appcfg !== null) ? $appcfg->getOptionalString('baseURL', null) : null;
815823

816824
if (!empty($appurl)) {
817-
$protocol = parse_url($appurl, PHP_URL_SCHEME);
818-
$hostname = parse_url($appurl, PHP_URL_HOST);
819-
$port = parse_url($appurl, PHP_URL_PORT);
820-
$port = !empty($port) ? ':' . $port : '';
825+
$protocol = (string)parse_url($appurl, PHP_URL_SCHEME);
826+
$hostname = (string)parse_url($appurl, PHP_URL_HOST);
827+
$portNum = parse_url($appurl, PHP_URL_PORT);
828+
$port = !empty($portNum) ? ':' . $portNum : '';
821829
} else {
822830
// no base URL specified for app, just use the current URL
823831
$protocol = $this->getServerHTTPS() ? 'https' : 'http';
824832
$hostname = $this->getServerHost();
825833
$port = $this->getServerPort();
826834
}
835+
827836
return $protocol . '://' . $hostname . $port . $_SERVER['REQUEST_URI'];
828837
}
829838

830-
return $this->getBaseURL() . $url_path . substr($_SERVER['REQUEST_URI'], $uri_pos + strlen($url_path));
839+
// Normal case: baseURL + script-relative path + remaining path, plus query if present
840+
$suffix = substr($requestPath, $uri_pos + strlen($url_path));
841+
$url = $this->getBaseURL() . $url_path . $suffix;
842+
843+
if ($requestQuery !== '') {
844+
$url .= '?' . $requestQuery;
845+
}
846+
847+
if ($requestFragment !== '') {
848+
$url .= '#' . $requestFragment;
849+
}
850+
851+
return $url;
831852
}
832853

833854

‎tests/src/SimpleSAML/Utils/HTTPTest.php‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -757,4 +757,54 @@ public static function slowPostDelayProvider(): array
757757
'positive config 10000 => 10000' => [10000, 10000],
758758
];
759759
}
760+
761+
/**
762+
* Ensure getSelfURL() returns the externally visible URL when SimpleSAMLphp
763+
* is reached via a rewritten path (e.g. /cas/login -> /simplesaml/module.php/...),
764+
* and the internal script name (module.php) appears only in the query string.
765+
*
766+
* This simulates an Apache mod_rewrite rule like:
767+
* RewriteRule ^/cas/login(.*) /${SSP_APACHE_ALIAS}module.php/casserver/login.php$1 [PT]
768+
*
769+
* In this scenario the public URL is /cas/login?... while the actual script is
770+
* public/module.php.
771+
*/
772+
public function testGetSelfURLWithRewrittenCasLogin(): void
773+
{
774+
$originalServer = $_SERVER;
775+
776+
$httpUtils = new Utils\HTTP();
777+
778+
$cfg = Configuration::loadFromArray([
779+
'baseurlpath' => 'https://tr-monitor-okta2.qa.athena-institute.net/simplesaml/',
780+
], '[ARRAY]', 'simplesaml');
781+
$baseDir = $cfg->getBaseDir();
782+
783+
$_SERVER = [
784+
'HTTPS' => 'on',
785+
'HTTP_HOST' => 'tr-monitor-okta2.qa.athena-institute.net',
786+
'SERVER_NAME' => 'tr-monitor-okta2.qa.athena-institute.net',
787+
'SERVER_PORT' => 443,
788+
'SCRIPT_URI' => 'https://tr-monitor-okta2.qa.athena-institute.net/cas/login',
789+
'SCRIPT_NAME' => '/module.php',
790+
'SCRIPT_FILENAME' => $baseDir . 'public' . DIRECTORY_SEPARATOR . 'module.php',
791+
'PATH_TRANSLATED' => $baseDir . 'public' . DIRECTORY_SEPARATOR . 'casserver/login.php',
792+
'PHP_SELF' => '/module.php/casserver/login.php',
793+
'QUERY_STRING' => 'service='
794+
. 'https%3A%2F%2Fcas-test-bridge.bridge.qa.cirrusidentity.com%2Fmodule.php%2Fcas%2Flinkback.php'
795+
. '%3FstateId%3D_somestate',
796+
'REQUEST_URI' => '/cas/login?service='
797+
. 'https%3A%2F%2Fcas-test-bridge.bridge.qa.cirrusidentity.com%2Fmodule.php%2Fcas%2Flinkback.php'
798+
. '%3FstateId%3D_somestate',
799+
];
800+
801+
$expected = 'https://tr-monitor-okta2.qa.athena-institute.net'
802+
. '/cas/login'
803+
. '?service=https%3A%2F%2Fcas-test-bridge.bridge.qa.cirrusidentity.com%2Fmodule.php%2Fcas%2Flinkback.php'
804+
. '%3FstateId%3D_somestate';
805+
806+
$this->assertSame($expected, $httpUtils->getSelfURL());
807+
808+
$_SERVER = $originalServer;
809+
}
760810
}

0 commit comments

Comments
 (0)