-
Notifications
You must be signed in to change notification settings - Fork 698
BUGFIX: http-getselfurl-robust-path-and-query #2589
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: simplesamlphp-2.5
Are you sure you want to change the base?
BUGFIX: http-getselfurl-robust-path-and-query #2589
Conversation
aa29612 to
5575aae
Compare
5575aae to
b043a77
Compare
b043a77 to
9b816fd
Compare
|
Relates to simplesamlphp-module-casserver Pull Request 65 |
| $hostname = $this->getServerHost(); | ||
| $port = $this->getServerPort(); | ||
| } | ||
| return $protocol . '://' . $hostname . $port . $_SERVER['REQUEST_URI']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this line need to change? The REQUEST_URI is not being parsed or interpreted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pradtke I changed it for consistency. I wanted all $_SERVER accesses to be grouped at the top of the method and then reconstruct the needed parts from those parsed values. I will revert this part.
|
I ran a test in our environment where we rewrite the path @ioigoume I think we would need the test case expanded. Below are some of the SERVER variables from when I tested. |
…Add specific test for the modrewrite scenario.
Improve HTTP::getSelfURL() path detection and URL reconstruction
HTTP::getSelfURL()mis-reconstructs the current URL when SimpleSAMLphp is accessed through a rewritten path such asRewriteRule ^/cas/login(.*) /${SSP_APACHE_ALIAS}module.php/casserver/login.php$1 [PT]. In this scenario the public URL is/cas/login?service=..., while Apache internally rewrites it to/simplesaml/module.php/casserver/login.php?service=.... The old implementation assumes that the script-relative filesystem path (for examplemodule.php/casserver/login.php) appears verbatim in$_SERVER['REQUEST_URI'], searches for that substring in the entireREQUEST_URI, and rebuilds the URL by stitching togethergetBaseURL(), that matched path, and the remainder ofREQUEST_URI. With mod_rewrite,REQUEST_URIcontains only the external path (/cas/login?...), so the expected script path is not present, or may only appear by accident inside query parameters. This causesgetSelfURL()to either fall back to an incorrect URL or to construct one that points tomodule.php/...instead of/cas/login, or that has a malformed path/query combination, breaking redirects and return URLs that rely ongetSelfURL()to reflect the actual public URL.