diff --git a/inc/classes/class-srm-redirect.php b/inc/classes/class-srm-redirect.php index ceb69745..b22abbbf 100644 --- a/inc/classes/class-srm-redirect.php +++ b/inc/classes/class-srm-redirect.php @@ -176,8 +176,10 @@ public function match_redirect( $requested_path ) { // check if requested path is the same as the redirect from path if ( $enable_regex ) { + // for regexes, check whether the requested path matches the $redirect_from regular expression $match_query_params = false; $matched_path = preg_match( '@' . $redirect_from . '@' . $regex_flag, $requested_path ); + // and then return the matching path } else { if ( $case_insensitive ) { $redirect_from = strtolower( $redirect_from ); @@ -207,6 +209,8 @@ public function match_redirect( $requested_path ) { } } + // If the requested path matches a redirect rule... + // this variable is not used after this boolean. if ( $matched_path ) { /** * Whitelist redirect host @@ -223,9 +227,14 @@ public function match_redirect( $requested_path ) { } // Allow for regex replacement in $redirect_to - if ( $enable_regex && ! filter_var( $redirect_to, FILTER_VALIDATE_URL ) ) { + if ( $enable_regex ) { $redirect_to = preg_replace( '@' . $redirect_from . '@' . $regex_flag, $redirect_to, $requested_path ); - $redirect_to = '/' . ltrim( $redirect_to, '/' ); + + // If $redirect_to does not look like a valid URL, + // assume that it needs to be turned into a path relative to root with a leading slash. + if ( ! filter_var( $redirect_to, FILTER_VALIDATE_URL ) ) { + $redirect_to = '/' . ltrim( $redirect_to, '/' ); + } } // re-add the query params if they've not already been added by the wildcard diff --git a/tests/php/test-core.php b/tests/php/test-core.php index c44a5050..4553a6c8 100644 --- a/tests/php/test-core.php +++ b/tests/php/test-core.php @@ -682,4 +682,120 @@ function( $requested_path, $redirected_to, $status_code ) use ( &$redirect_to, & remove_filter('srm_match_query_params', '__return_true'); } + /** + * Test that redirection to an external domain works with a regular expression without substitution + * + * @link https://github.com/10up/safe-redirect-manager/issues/269 + * @since 2.1.2 + */ + public function testRedirectToExternalDomainWithNonSubstitutingRegex269() { + $_SERVER['REQUEST_URI'] = '/be/hhhh'; + $redirect_from = '(go|be)\/h{0,}$'; + $redirect_to = 'http://xu-osp-plugins.local/404-regex'; + $status = 301; + $use_regex = true; + + $expected_redirect = 'http://xu-osp-plugins.local/404-regex'; + $expected_status = 301; + + $actual_request = ''; + $actual_redirect = ''; + $actual_status = 0; + + srm_create_redirect( $redirect_from, $redirect_to, $status, $use_regex ); + + add_action( + 'srm_do_redirect', + function( $requested_path, $redirected_to, $status_code ) use ( &$actual_request, &$actual_redirect, &$actual_status ) { + $actual_request = $requested_path; + $actual_redirect = $redirected_to; + $actual_status = $status_code; + }, + 10, + 3 + ); + + SRM_Redirect::factory()->maybe_redirect(); + $this->assertSame( $_SERVER['REQUEST_URI'], $actual_request, 'The requested path does not meet the expectation' ); + $this->assertSame( $expected_redirect, $actual_redirect, 'The redirect destination does not meet the expectation' ); + $this->assertSame( $expected_status, $actual_status, 'The redirect status does npt meet the expectation.' ); + } + + /** + * Test that redirection to an external domain works with a regular expression with substitution + * + * @link https://github.com/10up/safe-redirect-manager/issues/380 + * @since 2.2.0 + */ + public function testRedirectToExternalDomainWithSubstitutingRegex380() { + $_SERVER['REQUEST_URI'] = '/test/1234'; + $redirect_from = '/test/(.*)'; + $redirect_to = 'http://example.org/$1'; + $status = 301; + $use_regex = true; + + $expected_redirect = 'http://example.org/1234'; + $expected_status = 301; + + $actual_request = ''; + $actual_redirect = ''; + $actual_status = 0; + + srm_create_redirect( $redirect_from, $redirect_to, $status, $use_regex ); + + add_action( + 'srm_do_redirect', + function( $requested_path, $redirected_to, $status_code ) use ( &$actual_request, &$actual_redirect, &$actual_status ) { + $actual_request = $requested_path; + $actual_redirect = $redirected_to; + $actual_status = $status_code; + }, + 10, + 3 + ); + + SRM_Redirect::factory()->maybe_redirect(); + $this->assertSame( $_SERVER['REQUEST_URI'], $actual_request, 'The requested path does not meet the expectation' ); + $this->assertSame( $expected_redirect, $actual_redirect, 'The redirect destination does not meet the expectation' ); + $this->assertSame( $expected_status, $actual_status, 'The redirect status does npt meet the expectation.' ); + } + + /** + * Test that redirection to a local URL works with a regular expression with substitution + * + * @link https://github.com/10up/safe-redirect-manager/issues/380 + * @since 2.2.0 + */ + public function testRedirectToPathWithSubstitutingRegex380() { + $_SERVER['REQUEST_URI'] = '/test/1234'; + $redirect_from = '/test/(.*)'; + $redirect_to = '/result/$1'; + $status = 301; + $use_regex = true; + + $expected_redirect = '/result/1234'; + $expected_status = 301; + + $actual_request = ''; + $actual_redirect = ''; + $actual_status = 0; + + srm_create_redirect( $redirect_from, $redirect_to, $status, $use_regex ); + + add_action( + 'srm_do_redirect', + function( $requested_path, $redirected_to, $status_code ) use ( &$actual_request, &$actual_redirect, &$actual_status ) { + $actual_request = $requested_path; + $actual_redirect = $redirected_to; + $actual_status = $status_code; + }, + 10, + 3 + ); + + SRM_Redirect::factory()->maybe_redirect(); + $this->assertSame( $_SERVER['REQUEST_URI'], $actual_request, 'The requested path does not meet the expectation' ); + $this->assertSame( $expected_redirect, $actual_redirect, 'The redirect destination does not meet the expectation' ); + $this->assertSame( $expected_status, $actual_status, 'The redirect status does npt meet the expectation.' ); + } }