WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 4 years ago

#10077 closed defect (bug) (invalid)

clean_url improperly scrapes %20

Reported by: noel Owned by: ryan
Milestone: 2.9 Priority: high
Severity: major Version: 2.8
Component: Formatting Keywords: has-patch needs-unit-tests
Focuses: Cc:

Description

clean_url will currently turn a url like http://jcksn.com/Noel%20Jackson into http://jcksn.com/NoelJackson

This patch fixes that problem, adding
x20 to the regex.

Attachments (1)

add-spaces.patch (562 bytes) - added by noel 5 years ago.
Fix for spaces in clean_url

Download all attachments as: .zip

Change History (9)

noel5 years ago

Fix for spaces in clean_url

comment:1 Denis-de-Bernardy5 years ago

  • Version set to 2.8

comment:2 xibe5 years ago

  • Owner changed from rboren to ryan
  • Status changed from new to reviewing

comment:3 follow-up: Denis-de-Bernardy5 years ago

  • Severity changed from major to blocker

this one is a blocker imo.

comment:4 in reply to: ↑ 3 westi5 years ago

  • Milestone changed from 2.8 to 2.8.1
  • Severity changed from blocker to major

Replying to Denis-de-Bernardy:

this one is a blocker imo.

this can't be a blocker at this stage as it isn't a regression.

This will have to wait for 2.8.1

comment:5 westi5 years ago

Just tested this.

And clean_url('http://jcksn.com/Noel%20Jackson') works fine already
What doesn't work is clean_url('http://jcksn.com/Noel Jackson')

comment:6 westi5 years ago

Would be good to see a nice set of test cases to add for this.

Including all the things we are trying to remove.

I started with the bug here: http://svn.automattic.com/wordpress-tests/wp-testcase/test_includes_formatting.php

Patches welcome

comment:7 Denis-de-Bernardy5 years ago

  • Keywords needs-unit-tests added

comment:8 miqrogroove4 years ago

  • Cc miqrogroove@… added
  • Resolution set to invalid
  • Status changed from reviewing to closed

This is not a bug. The behavior described by westi is correct. I have reviewed the trunk code, compared it to RFC 1738, and it appears to be perfect. Spaces fall under the definition of Unsafe URL Characters and are never allowed to appear unless encoded.

If there is a cosmetic problem involving URLs that are passed to clean_url(), then we should have a ticket describing why and how clean_url() is being used at a higher level.

I don't object to westi's idea for test cases, but this is currently a matter of "garbage in, garbage out."

Note: See TracTickets for help on using tickets.