WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 5 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:

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 6 years ago.
7145.diff (639 bytes) - added by scohoust 5 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Scott H6 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 H6 years ago

comment:2 ryan6 years ago

  • Milestone changed from 2.5.2 to 2.9

Milestone 2.5.2 deleted

comment:3 jairus5 years ago

  • Keywords dev-feedback added

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

comment:4 mrmist5 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.)

comment:5 mrmist5 years ago

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

comment:6 scohoust5 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.

scohoust5 years ago

comment:7 Denis-de-Bernardy5 years ago

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

confirmed bug. it's in the link manager

comment:8 ryan5 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.

comment:9 ryan5 years ago

  • Milestone changed from 2.9 to 2.8

comment:10 ryan5 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.