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: |
|
Owned by: |
|
|---|---|---|---|
| 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)
Change History (14)
markjaquith
— 20 months ago
comment:1
markjaquith
— 20 months ago
- Keywords has-patch 2nd-opinion needs-unit-tests added
- Owner set to markjaquith
- Status changed from new to accepted
comment:2
nacin
— 20 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:3
SergeyBiryukov
— 20 months ago
Related: #12756
comment:6
wonderboymusic
— 4 months ago
Related: #22666
comment:7
wonderboymusic
— 4 months ago
- Milestone changed from Awaiting Review to 3.6
comment:8
kurtpayne
— 4 months ago
Updated unit test for the new test runner. Verified test still fails without patch and still passes with patch.
comment:10
nacin
— 4 months ago
In 1225/tests:
comment:11
nacin
— 4 months ago
- Resolution set to fixed
- Status changed from accepted to closed
In 23537:
Patch removes that part of the logic from 10 functions. Someone else test and give me a sanity check.