WPCOM-Legacy-Redirector icon indicating copy to clipboard operation
WPCOM-Legacy-Redirector copied to clipboard

Store redirects independint of trailing slash

Open jeremeylduvall opened this issue 6 years ago • 3 comments

Currently, if I import a redirect for /test and visit example.com/test/ in my browser, I won't get redirected. The workaround is to upload redirects for /test/ and /test to the same destination URL.

Instead, does it make sense to add a check here that basically does this?

if ( $request_path ) {
	$redirect_uri = self::get_redirect_uri( $request_path );

	if ( ! $redirect_uri ) {
		if ( substr( $request_path , -1 )=='/' ) {
			$redirect_uri = self::get_redirect_uri( rtrim( $request_path, '/' );	
		} else {
			$redirect_uri = self::get_redirect_uri( $request_path . '/' );	
		}		
	}

	if ( $redirect_uri ) {
		header( 'X-legacy-redirect: HIT' );
		$redirect_status = apply_filters( 'wpcom_legacy_redirector_redirect_status', 301, $url );
		wp_safe_redirect( $redirect_uri, $redirect_status );
		exit;
	}
}

Here: https://github.com/Automattic/WPCOM-Legacy-Redirector/blob/master/wpcom-legacy-redirector.php#L108

I'm sure there's a more eloquent way to do this, but it seems silly to have to worry about both variations of a URL.

jeremeylduvall avatar Apr 29 '19 20:04 jeremeylduvall

@jeremeylduvall Those pesky trailing slashes never go away :)

Thanks for scoping this out and for sharing the provided sample code. I was able to test, validate, and implement automatically supporting both scenarios (with or without the trailing slash). I also included query string compatibility as well and I'll be forking + committing code soon.

bdtech avatar May 23 '19 02:05 bdtech

@GaryJones Do we want to allow the creation of redirects ($from_url) for both trailing slash URL variants through the UI? It is currently allowed. This decision impacts how the most optimal solution is implemented for v1.4.0 and supported moving forward.

i.e... https://example.com/redir to https://example.com/page1 https://example.com/redir/ to https://example.com/page2

Possible Solutions: A) Consistent trailing slash agnostic support across the CLI, UI, and internally within the plugin. We'd uniformly consider the example above as one redirect route. Only the CLI can create a trailing slash variant with --skip-validation. We can apply the trailing slash policy based on the site's current permalink structure. B) Allow these to operate as separate redirect routes as is allowed today. We'd add logic to maybe_do_redirect() to support a fallback check on the second possible URL - the trailing slash variant as outlined in this issue. We'd further optimize for performance.

Thoughts? Thank you, sir!

bdtech avatar May 28 '19 16:05 bdtech

+1 to this. We've had to hack the core plugin code for this use case.

bcampeau avatar Oct 08 '20 15:10 bcampeau