WordPress.org

Make WordPress Core

Opened 12 years ago

Closed 11 years ago

#7145 closed defect (bug) (fixed)

Link 'Web Address' field strips pipes

Reported by: Jairus Owned by:
Milestone: 2.8 Priority: normal
Severity: minor Version: 2.5.1
Component: General Keywords: has-patch tested commit
Focuses: Cc:
PR Number:

Description

When creating a new (or editing an existing) link, all pipes will be stripped from the Web Address field (but not Name or Description).

Thus, it's impossible to use Links to link to any sites which use pipes in their URL structure, such as:

http://www.hallmark.com/webapp/wcs/stores/servlet/category3|10001|10051|181568|147551;-102001;181568|ecards|Just%20Because

Attachments (2)

linkdiff.diff (608 bytes) - added by Scott H 12 years ago.
7145.diff (639 bytes) - added by scohoust 11 years ago.

Download all attachments as: .zip

Change History (12)

#1 @Scott H
12 years ago

  • Component changed from Administration to General
  • Keywords has-patch added; needs-patch removed

Whether this was left out intentionally or not, I don't know. Attached does the trick though.

@Scott H
12 years ago

#2 @ryan
12 years ago

  • Milestone changed from 2.5.2 to 2.9

Milestone 2.5.2 deleted

#3 @jairus
11 years ago

  • Keywords dev-feedback added

Is there a reason why this is pushed to 2.9 instead of being patched in sooner?

#4 @mrmist
11 years ago

  • Keywords needs-patch added; has-patch removed

Patch is stale. Also if only wp-includes/formatting.php expression is changed, then the resultant pipe symbol gets urlencoded. I'm not sure whether that is the intended result or not. I'd say probably not, so removing has-patch for now.

Can't think of any particular reason why a pipe shouldn't be allowed, though. (It would only seem potentially harmful if prefixed by other harmful stuff.)

#5 @mrmist
11 years ago

Actually the encoding probably is right, so it would just need refreshing in that case.

#6 @scohoust
11 years ago

  • Cc wp@… added

Just tried this patch on r10933 (I submitted the original) and couldn't find any issue. Could you point out where the problem is?

Reuploaded the diff to work with current trunk.

@scohoust
11 years ago

#7 @Denis-de-Bernardy
11 years ago

  • Keywords has-patch tested commit added; needs-patch links dev-feedback removed

confirmed bug. it's in the link manager

#8 @ryan
11 years ago

Other characters are unsafe because gateways and other transport agents are known to sometimes modify such characters. These characters are "{", "}", "|", "\", "^", "~", "[", "]", and "`".

All unsafe characters must always be encoded within a URL.

http://www.ietf.org/rfc/rfc1738.txt

Doesn't seem like pipe is marked unsafe for security reasons, so it's probably safe to let it through.

#9 @ryan
11 years ago

  • Milestone changed from 2.9 to 2.8

#10 @ryan
11 years ago

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

(In [11023]) Allow pipes through clean_url(). Props scohoust. fixes #7145

Note: See TracTickets for help on using tickets.