WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 2 months ago

#16859 accepted defect (bug)

esc_url eats square brackets. — at Version 13

Reported by: f00f Owned by: westi
Milestone: Future Release Priority: normal
Severity: major Version: 3.1
Component: Formatting Keywords: has-patch dev-feedback
Focuses: Cc:

Description (last modified by westi)

When adding a link to the blogroll (using wp-admin/link-add.php), square brackets in the link are removed, breaking the link.

Example:

http://lokale-wochenzeitungen.de/index.php?id=485&tx_ttnews[pointer]=6&tx_ttnews[tt_news]=132583&tx_ttnews[backPid]=741&cHash=ee9c87874b

becomes

http://lokale-wochenzeitungen.de/index.php?id=485&tx_ttnewspointer=6&tx_ttnewstt_news=132583&tx_ttnewsbackPid=741&cHash=ee9c87874b

Workaround: Use URL-encoded links (%5B and %5D instead of [ and ]).

This also affects urls which are made clickable by {{{make_clickable}}

Change History (15)

comment:1 follow-up: @ldebrouwer4 years ago

  • Cc ldebrouwer added
  • Keywords 2nd-opinion added; needs-patch removed

To achieve this we would need to alter esc_url which I'm pretty sure is not going to happen because URLs containing brackets shouldn't be floating around and should always be encoded!

If you take a look at the URL spec ( http://www.w3.org/Addressing/URL/url-spec.txt ) you can read:

"The 'national' and 'punctuation' characters do not appear in any productions and therefore may not appear in URLs.

national { | } | vline | [ | ] | \ | | ~

punctuation < | >"

I don't know if you've copied the URL from somewhere but the brackets definitely shouldn't be there.

comment:2 in reply to: ↑ 1 @f00f4 years ago

Replying to ldebrouwer:

To achieve this we would need to alter esc_url which I'm pretty sure is not going to happen because URLs containing brackets shouldn't be floating around and should always be encoded!

Is it possible to wrap a urlencode() around the URL before escaping it? Didn't have a look at the code yet, so don't know if that makes sense.

If you take a look at the URL spec ( http://www.w3.org/Addressing/URL/url-spec.txt ) you can read:

[...]

True (wasn't aware of that, thanks).
However, the PHP FAQ suggests using square brackets for form fields to create arrays ( http://www.php.net/manual/en/faq.html.php#faq.html.arrays ). Also, a similar problem came up in ticket:12690, so I thought it might be interesting.

I don't know if you've copied the URL from somewhere but the brackets definitely shouldn't be there.

The URL is generated by the (IMHO popular) tt_news extension for Typo3.

comment:3 follow-up: @dd324 years ago

However, the PHP FAQ suggests using square brackets for form fields to create arrays

That's correct, however, it doesnt mean that the characters should be unescaped in a GET request.

comment:4 in reply to: ↑ 3 ; follow-up: @f00f4 years ago

Replying to dd32:

However, the PHP FAQ suggests using square brackets for form fields to create arrays

That's correct, however, it doesnt mean that the characters should be unescaped in a GET request.

Yah, but apparently it happens when copy-pasting a URL from the address bar or using 'copy link location'.

comment:5 in reply to: ↑ 4 @ldebrouwer4 years ago

Replying to f00f:

Yah, but apparently it happens when copy-pasting a URL from the address bar or using 'copy link location'.

And that, to me, seems the root of the problem. You copy a, according to standards, malformed URL from a third-party. And WordPress, as a service, filters all the bad characters from the URL for security reasons. To me WordPress should not anticipate specific behaviour of third-party software ( browsers aside ), just the behaviour of the users.

comment:6 @f00f4 years ago

I hear you. Well, hopefully this ticket helps someone experiencing the same problem I did.

comment:7 @dd324 years ago

Just looking at this, [] should definitely be allowed in the url's.. at least in the query segment

comment:8 @ldebrouwer4 years ago

No they shouldn't, at least not unencoded. And like I pointed out earlier it's not up to WordPress to fix the problems of third-parties.

comment:9 @dd324 years ago

No they shouldn't, at least not unencoded.

In URL's produced, sure.

However, users should not be expected to encode the values themselves, many links/sites do not encode them themselves, which leads to this case of users encountering issues when pasting the links in.

[] should be allowed to be entered, they shouldn't be striped, perhaps they should be encoded or similar however.

@edwardw4 years ago

[PATCH] Urlencode brackets when cleaning

comment:10 @edwardw4 years ago

  • Keywords has-patch dev-feedback added; 2nd-opinion removed
  • Owner set to edwardw
  • Status changed from new to accepted

Even though brackets should be encoded for use in third-party applications in theory, we should not blame the user who often may not even be aware of this. Often the URL is copied from the location bar or other source where they have not encoded this, and especially due to PHP's use of brackets to pass arrays. I have attached a patch which urlencode()s brackets when cleaning URLs.

comment:11 @westi4 years ago

  • Keywords needs-patch added; has-patch dev-feedback removed

I'm not sure we should be patching esc_url for this issue but rather just fix the link add/editing work flow to correctly encode the square brackets.

@edwardw4 years ago

[PATCH] Urlencode brackets when cleaning, patching wp-admin/bookmark.php instead

comment:12 @edwardw4 years ago

  • Keywords has-patch dev-feedback added; needs-patch removed

Agreed.

comment:13 @westi3 years ago

  • Description modified (diff)
  • Owner changed from edwardw to westi
  • Summary changed from Square brackets are removed from links in blogroll to esc_url eats square brackets.

I've changed my mind about this as it affects a number of places and I think we do better to patch esc_url to fix this for [] by encoding them for you.

Note: See TracTickets for help on using tickets.