WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 2 months ago

#17052 reopened defect (bug)

wp_sanitize_redirect() removes square brackets from URL

Reported by: bluntelk Owned by:
Milestone: Future Release Priority: normal
Severity: minor Version: 3.1
Component: Formatting Keywords: has-patch commit
Focuses: Cc:

Description

The function wp_sanitize_redirect() removes square brackets from URLs.

PHP's functionality with arrays in the URL require square braces, stripping them from the URL means that pages (and plugins) that rely on them fail.

To Reproduce:

<?php
$url = 'http://example.com/my_url_array[1]=hello+world';
print wp_sanitize_redirect($url);
?>

Current Output:
http://example.com/my_url_array1=hello+world

Expected Output:
http://example.com/my_url_array[1]=hello+world

Whilst developers should be able to work around this as the function is pluggable I believe this should just work out of the box.

Attachments (3)

pluggable.17052.patch (499 bytes) - added by bluntelk 3 years ago.
adds square braces to the regular expression for allowed characters in a safe URL
basic_auth_for_wp_redirect.diff (504 bytes) - added by david.binda 3 months ago.
17052-02.patch (1.6 KB) - added by gcorne 2 months ago.

Download all attachments as: .zip

Change History (14)

comment:1 dd323 years ago

  • Keywords has-patch removed
  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

whilst referring to blogroll, the root cause is the same

In short, the brackets shouldn't be in the url, they *should* be encoded, but often arn't.

#16859

bluntelk3 years ago

adds square braces to the regular expression for allowed characters in a safe URL

comment:2 dd323 years ago

  • Milestone set to Awaiting Review
  • Resolution duplicate deleted
  • Status changed from closed to reopened

Probably shouldnt've closed this so fast as a duplicate.

It's the same cause, but probably a lot better explained and in a different usecase.

comment:3 bluntelk3 years ago

  • Keywords has-patch needs-testing added

Hi dd32,

Thanks for re-opening this ticket.

I was working under the assumption that the characters we simply missing from the regular expression due to an oversight.

It is a powerful feature to support.

If it is determined under review that they should be stripped from the URL, we can always automatically encode the square brackets into their entities inside the function.

Thanks for your time.

Jason

comment:4 bluntelk2 years ago

  • Version changed from 3.1 to 3.4

comment:5 SergeyBiryukov2 years ago

  • Version changed from 3.4 to 3.1

Version number indicates when the bug was initially introduced/reported.

comment:6 c3mdigital8 months ago

  • Keywords needs-refresh added; needs-testing removed

comment:7 SergeyBiryukov5 months ago

#26037 was marked as a duplicate.

comment:8 nacin3 months ago

  • Component changed from General to Formatting
  • Milestone changed from Awaiting Review to Future Release

comment:9 david.binda3 months ago

"@" is being removed from URL as well. As "@" might be a part of basic-auth URL, it should be preserved in the URL.

As I noticed that https://core.trac.wordpress.org/ticket/26037 was closed as a dupe (despite it contains "@" as well), I'm posting a patch containing both ("@" and "[]") to his ticket.

gcorne2 months ago

comment:10 gcorne2 months ago

Allowing square brackets is also important when dealing with IPv6 literals (see RFC2732). The patch 17052-02.patch adds a couple unit tests in addition to adding [ and ] to the whitelist of allowed characters.

I am not sure that we want to "@" as part of this same patch, but "@" is a reserved character so I think it is appropriate for it to be allowed.

comment:11 gcorne2 months ago

  • Keywords commit added; needs-refresh removed
Note: See TracTickets for help on using tickets.