Make WordPress Core

Opened 9 years ago

Closed 9 years ago

#30308 closed defect (bug) (fixed)

Bracket characters ( and ) are incorrectly removed from wp_sanitize_redirect

Reported by: jkohlbach's profile jkohlbach Owned by: johnbillion's profile johnbillion
Milestone: 4.1 Priority: normal
Severity: normal Version: 4.0
Component: Formatting Keywords: has-patch
Focuses: Cc:

Description

According to the URI spec under section 2.3 Unreserved Characters (http://www.ietf.org/rfc/rfc2396.txt) the bracket characters ( and ) are allowed in URI's but wp_sanitize_redirect strips them out.

This means the user is sent to the wrong URL when using wp_redirect or wp_safe_redirect.

To reproduce, open wp-includes/pluggable.php and drop in some debug in the wp_redirect function:

echo '<pre>DEBUG: ' . print_r($location, true) . '</pre>';
$location = wp_sanitize_redirect($location);
echo '<pre>DEBUG: ' . print_r($location, true) . '</pre>';
die();

Then just use wp_redirect('http://google.com/test=(12345)abcdef', 301); and you'll see the brackets are being stripped incorrectly.

Attachments (1)

30308.diff (1.3 KB) - added by voldemortensen 9 years ago.

Download all attachments as: .zip

Change History (8)

#1 follow-up: @SergeyBiryukov
9 years ago

Related/duplicate: #17052, #26037.

#2 in reply to: ↑ 1 @jkohlbach
9 years ago

Replying to SergeyBiryukov:

Related/duplicate: #17052, #26037.

Hi Sergey,

That other ticket is for square brackets, not rounded brackets, ideally they should be both fixed :)

Cheers,
Josh

@voldemortensen
9 years ago

#3 @voldemortensen
9 years ago

  • Keywords has-patch added

This excludes parenthesis from the preg_replace and includes unit tests.

#4 @jkohlbach
9 years ago

Looks good to me, thanks voldemortensen

#5 @johnbillion
9 years ago

  • Milestone changed from Awaiting Review to 4.1

This ticket was mentioned in Slack in #core by voldemortensen. View the logs.


9 years ago

#7 @johnbillion
9 years ago

  • Owner set to johnbillion
  • Resolution set to fixed
  • Status changed from new to closed

In 30684:

Allow brackets in a URL when it's sanitised for a redirect. Brackets are valid in query parameters.

Fixes #30308
Props voldemortensen

Note: See TracTickets for help on using tickets.