include $parsed_url[path] in the assessment #6995
Conversation
There are instances where a wordpress install is implemented as a backend but served as though in a subdirectory path. i.e. backend is https://some_wordpress_install/ but served to the user as https://example.com/blog/. This fix makes sure to add the path of the home_url and removes that part from the $_SERVER['REQUEST_URI']. This way, the right uri is always preserved, otherwise it breaks.
|
Cameron Barr seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
Could you add some tests to demonstrate the issue and ensure it works as expected? |
|
|
||
| if ( isset( $_SERVER['REQUEST_URI'] ) ) { | ||
| // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized | ||
| $current_url .= ltrim( wp_unslash( $_SERVER['REQUEST_URI'] ), '/' ); | ||
| $current_url .= ltrim( wp_unslash( str_replace($parsed_url['path'], '', $_SERVER['REQUEST_URI']) ), '/' ); |
There was a problem hiding this comment.
The unslashing should be limited to the input var:
| $current_url .= ltrim( wp_unslash( str_replace($parsed_url['path'], '', $_SERVER['REQUEST_URI']) ), '/' ); | |
| $current_url .= ltrim( str_replace($parsed_url['path'], '', wp_unslash( $_SERVER['REQUEST_URI']) ), '/' ); |
| @@ -653,12 +653,14 @@ function amp_get_current_url() { | |||
| if ( isset( $parsed_url['port'] ) ) { | |||
| $current_url .= ':' . $parsed_url['port']; | |||
| } | |||
| $current_url .= '/'; | |||
|
|
|||
| $current_url .= '/' . trim($parsed_url['path'], '/') . '/' ; | |||
There was a problem hiding this comment.
For a site that is located at the root, wouldn't this result in // being appended?
|
|
||
| if ( isset( $_SERVER['REQUEST_URI'] ) ) { | ||
| // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized | ||
| $current_url .= ltrim( wp_unslash( $_SERVER['REQUEST_URI'] ), '/' ); | ||
| $current_url .= ltrim( wp_unslash( str_replace($parsed_url['path'], '', $_SERVER['REQUEST_URI']) ), '/' ); |
There was a problem hiding this comment.
Also, wouldn't it be a good hardening to ensure that it only removes $parsed_url['path'] from the string only when it appears at the very beginning? Let's say my install is located at /wordpress/ but the REQUEST_URI is /wordpress/category/wordpress/ then this would result in /category/ being incorrectly appended to $current_url as opposed to /category/wordpress/.
|
Thank you for the follow up. I will amend the PR shortly and run those tests. Thank you |
There are instances where a wordpress install is implemented as a backend but served as though in a subdirectory path.
i.e. backend is https://some.wordpress.install/ but served to the user as https://example.com/blog/. This fix makes sure to add the path of the home_url and removes that part from the $_SERVER['REQUEST_URI']. This way, the right uri is always preserved, otherwise it breaks.