WordPress.org

Make WordPress Core

Opened 2 months ago

Last modified 2 months ago

#43745 reviewing defect (bug)

(Yet another ;-) Redirect loop with encoded query keys

Reported by: wrwrwr0 Owned by: SergeyBiryukov
Milestone: 5.0 Priority: normal
Severity: minor Version: trunk
Component: Canonical Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

Some live examples:

https://news.microsoft.com/?%C2%A2
https://newsroom.fb.com/?%C2%A2
https://www.thewaltdisneycompany.com/?%C2%A2

(At the time of writing these show "page isn't redirecting properly" / "redirected you too many times" notice.)

To reproduce on a fresh install set some permalink structure and put a static page on front, for convenience:

wp rewrite structure '/%year%/%monthnum%/%postname%/'
wp option update show_on_front page
wp option update page_on_front 2

Observe a 301 with the location exactly matching the request:

curl -I http://test.local/?%C2%A2

(Query with any character in a key matching the first regex in wp_sanitize_redirect().)

redirect_canonical() is hooked by default to template_redirect. Under some circumstances $redirect_url can be set to more or less the same as $requested_url in the somewhat lengthy "is 404" conditional. The code that readds additional query args, around line 360 of the function, uses rawurlencode_deep(), which leaves keys unencoded. In consequence, $requested_url and $redirect_url around line 490 may differ only in query encoding. Further, wp_sanitize_request(), applied after the chained redirects check, reencodes some multibyte characters.

Attachments (1)

43745.diff (1.4 KB) - added by soulseekah 2 months ago.
fix + tests

Download all attachments as: .zip

Change History (3)

@soulseekah
2 months ago

fix + tests

#1 @soulseekah
2 months ago

  • Keywords has-patch has-unit-tests added

Welcome to Trac!

Thanks for your detailed explanation and the steps to reproduce.

43745.diff contains a fix and a test.

#2 @SergeyBiryukov
2 months ago

  • Milestone changed from Awaiting Review to 5.0
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing
Note: See TracTickets for help on using tickets.