redirect_canonical unneeded redirect

[aidvu]

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
);

[aidvu]

Anyone from core interested in discussing this one?

## What?\nAvoid canonical redirects when the requested and redirected query strings resolve to the same arguments but use different encoding, such as + versus %20.\n\n## Why?\nThis prevents unnecessary redirects on static front page requests and avoids redirect loops for setups that cache or normalize those requests differently.\n\n## How?\n- treat query-only encoding changes as equivalent after parsing\n- add a regression test for a static front page request using /?test=a+b+c\n\nTrac ticket: https://core.trac.wordpress.org/ticket/51733

[arkaprabhachowdhury]

Opened [PR #12282](​https://github.com/WordPress/wordpress-develop/pull/12282) to address this ticket. Summary Avoid canonical redirects when the requested and redirected query strings resolve to the same arguments but use different encoding, such as + versus %20. Testing - php -l src/wp-includes/canonical.php - php -l tests/phpunit/tests/canonical.php - php vendor/bin/phpcbf --standard=phpcs.xml.dist src/wp-includes/canonical.php tests/phpunit/tests/canonical.php - php vendor/bin/phpcs --standard=phpcs.xml.dist src/wp-includes/canonical.php tests/phpunit/tests/canonical.php The PR includes focused regression coverage, and its validation details are recorded in the PR description.

I picked this up because the ticket describes a concrete core behavior or edge case that remains relevant in current WordPress. The proposed change is intentionally scoped to the ticket and is submitted here for code review, with the technical discussion remaining on this ticket.