WordPress.org

Make WordPress Core

Opened 20 months ago

Closed 4 months ago

#19032 closed defect (bug) (fixed)

home_url() malfunctions when the passed URL contains two dots in a row ("..")

Reported by: markjaquith Owned by: markjaquith
Priority: high Milestone: 3.6
Component: General Version: 2.6
Severity: normal Keywords: has-patch 2nd-opinion
Cc: kpayne@…

Description

home_url() (and a bunch of other similar functions) will spit back your WordPress Site URL if the URL/path you pass in to it has two dots ("..") in a row. It was an unexplained addition to the original code that has persisted.

Reproduction:

echo home_url( '/foo-bar/elipsis...no-work/' );

Observed:

http://example.com

Expected:

http://example.com/foo-bar/elipsis...no-work/

It is due to this:

if ( !empty( $path ) && is_string( $path ) && strpos( $path, '..' ) === false )
    $url .= '/' . ltrim( $path, '/' );

We should just remove that part of the condition. It doesn't serve any legitimate purpose that Nacin or I can tell, and it makes legitimate URLs with two (or more) dots fail in a very unexpected way.

Attachments (3)

19032.diff (4.0 KB) - added by markjaquith 20 months ago.
19032-unit-test.patch (2.7 KB) - added by kurtpayne 13 months ago.
Unit test
19032-unit-test.2.patch (2.3 KB) - added by kurtpayne 4 months ago.

Download all attachments as: .zip

Change History (14)

markjaquith20 months ago

comment:1 markjaquith20 months ago

  • Keywords has-patch 2nd-opinion needs-unit-tests added
  • Owner set to markjaquith
  • Status changed from new to accepted

Patch removes that part of the logic from 10 functions. Someone else test and give me a sanity check.

comment:2 nacin20 months ago

In IRC we tracked this down to #7001. It was codified into unit tests, specifically TestSSLLinks::test_admin_url_invalid(), but I think the original case was invalid. We're talking about URL paths rather than anything that will touch the filesystem.

comment:4 nacin19 months ago

  • Version set to 2.6

kurtpayne13 months ago

Unit test

comment:5 kurtpayne13 months ago

  • Cc kpayne@… added
  • Keywords needs-unit-tests removed

comment:7 wonderboymusic4 months ago

  • Milestone changed from Awaiting Review to 3.6

comment:8 kurtpayne4 months ago

Updated unit test for the new test runner. Verified test still fails without patch and still passes with patch.

comment:9 ocean904 months ago

#23489 was marked as a duplicate.

comment:10 nacin4 months ago

In 1225/tests:

Test for allowing .. in paths passed to *_url functions. see #19032.

comment:11 nacin4 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 23537:

Allow paths with two consecutive dots to be passed to home_url() and all related *_url() functions.

props markjaquith.
fixes #19032.

Note: See TracTickets for help on using tickets.