remove absolute URI from $_SERVER['REQUEST_URI'] (issue #91)#93
remove absolute URI from $_SERVER['REQUEST_URI'] (issue #91)#93dg merged 3 commits intonette:masterfrom
Conversation
src/Http/RequestFactory.php
Outdated
|
|
||
| // path & query | ||
| $requestUrl = isset($_SERVER['REQUEST_URI']) ? $_SERVER['REQUEST_URI'] : '/'; | ||
| $requestUrl = isset($_SERVER['REQUEST_URI']) ? Strings::replace($_SERVER['REQUEST_URI'], '#^\w+://[^/]+#i') : '/'; |
There was a problem hiding this comment.
I think it's better to make it more readable by splitting it into lines.
$requestUrl = isset($_SERVER['REQUEST_URI']) ? $_SERVER['REQUEST_URI'] : '/';
$requestUrl = Strings::replace($requestUrl, '#^\w+://[^/]+#i');The pattern could be optimized by removing pointless case-insensitive flag and avoiding any possible backtracking
$requestUrl = isset($_SERVER['REQUEST_URI']) ? $_SERVER['REQUEST_URI'] : '/';
$requestUrl = Strings::replace($requestUrl, '#^\w++://[^/]++#');With removed backtracking, we may probably replace Strings::replace with faster preg_replace
$requestUrl = isset($_SERVER['REQUEST_URI']) ? $_SERVER['REQUEST_URI'] : '/';
$requestUrl = preg_replace('#^\w++://[^/]++#', '', $requestUrl);There was a problem hiding this comment.
OK, you're right. I have modified it. Thanks.
|
|
||
| // path & query | ||
| $requestUrl = isset($_SERVER['REQUEST_URI']) ? $_SERVER['REQUEST_URI'] : '/'; | ||
| $requestUrl = preg_replace('#^\w++://[^/]++#', '', $requestUrl); |
There was a problem hiding this comment.
Is this actually RFC compliant? What if I send mismatching Host header or use different protocol than used in here? Shouldn't we strip it only if it is a match and throw exception otherwise?
There was a problem hiding this comment.
See how Symfony handles this. They replace it only if it matches current schema and host. But I haven't seen any specification which would define how this should actually be handled.
No description provided.