Opened 4 years ago
Last modified 3 years ago
#51733 new defect (bug)
redirect_canonical unneeded redirect
Reported by: | aidvu | Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | minor | Version: | 3.3 |
Component: | Canonical | Keywords: | needs-patch |
Focuses: | Cc: |
Description
Recently caught an unneeded redirect in redirect_canonical
. Started caching responses, and ended in a redirect loop.
tl;dr
Per RFC3986, +
is a reserved character, and we should use %20
to encode a space in URLs (percent encoding).
Per RFC1866, encode query args as application/x-www-form-urlencoded
in which case a space is encoded as a +
.
Posting here to discuss whether we want to fix this in core, or simply leave it as is.
To reproduce:
- Use any
twenty{year}
theme - Customize homepage
- Select
A static page
- Visit homepage with query args, e.g. https://vubuesinessplantest.help/?test=a+b+c
- A redirect occurs to https://vubuesinessplantest.help/?test=a%20b%20c
Considering we're using parse_str()
to parse query variables, and it correctly parses both RFCs I'd argue we shouldn't redirect in the case when query args are the same.
Here's a filter I've written to skip redirects in this case:
<?php // Don't redirect if we're only re-encoding query params add_filter( 'redirect_canonical', function ( $redirect_url, $requested_url ) { $parsed_redirect = parse_url( $redirect_url ); $parsed_requested = parse_url( $requested_url ); // Scheme changed, do redirect if ( $parsed_requested['scheme'] !== $parsed_redirect['scheme'] ) { return $redirect_url; } // Host changed, do redirect if ( $parsed_requested['host'] !== $parsed_redirect['host'] ) { return $redirect_url; } // Path changed, do redirect if ( $parsed_requested['path'] !== $parsed_redirect['path'] ) { return $redirect_url; } // Parse query args parse_str( $parsed_redirect['query'], $query_redirect ); parse_str( $parsed_requested['query'], $query_request ); // Sort by keys, if the order changed ksort( $query_redirect ); ksort( $query_request ); // If parsed query args are the same, skip redirect if ( $query_redirect === $query_request ) { return false; } // Otherwise, do redirect return $redirect_url; }, 10, 2 );
Note: See
TracTickets for help on using
tickets.
Anyone from core interested in discussing this one?