Merge pull request #395 from 10up/fix/380-regex-with-url

Allow regular expression parsing if the redirect target is a valid URL
10up · Sep 17, 2024 · 6cf8d09 · 6cf8d09
2 parents cb1c045 + aba231b
commit 6cf8d09
Show file tree

Hide file tree

Showing 2 changed files with 127 additions and 2 deletions.
diff --git 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
@@ -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.' );
+	}
 }