WordPress.org

Make WordPress Core

Opened 9 years ago

Closed 9 years ago

Last modified 3 years ago

#10077 closed defect (bug) (invalid)

clean_url improperly scrapes %20

Reported by: noel Owned by: ryan
Milestone: 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 9 years ago.
Fix for spaces in clean_url

Download all attachments as: .zip

Change History (10)

@noel
9 years ago

Fix for spaces in clean_url

#1 @Denis-de-Bernardy
9 years ago

  • Version set to 2.8

#2 @xibe
9 years ago

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

#3 follow-up: @Denis-de-Bernardy
9 years ago

  • Severity changed from major to blocker

this one is a blocker imo.

#4 in reply to: ↑ 3 @westi
9 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

#5 @westi
9 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')

#6 @westi
9 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

#7 @Denis-de-Bernardy
9 years ago

  • Keywords needs-unit-tests added

#8 @miqrogroove
9 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."

#9 @DrewAPicture
3 years ago

  • Milestone 2.9 deleted
Note: See TracTickets for help on using tickets.