Make WordPress Core

Opened 12 years ago

Closed 11 years ago

#19282 closed defect (bug) (fixed)

wp_get_attachment_link() does not allow HTML in link text

Reported by: sergeybiryukov's profile SergeyBiryukov Owned by: ryan's profile ryan
Milestone: 3.4 Priority: normal
Severity: normal Version: 2.8
Component: General Keywords: has-patch commit
Focuses: Cc:


Background: #18156

Escaping $link_text was introduced in [10495] along with the $text parameter itself and changed to esc_attr() in [11204]. Looks like it shouldn't really be there:

  1. $link_text is not an attribute, it's literally a link text.
  2. adjacent_post_link() doesn't escape link text.

Attachments (2)

19282.patch (462 bytes) - added by SergeyBiryukov 12 years ago.
19282.diff (473 bytes) - added by griffinjt 11 years ago.

Download all attachments as: .zip

Change History (7)

#1 @SergeyBiryukov
12 years ago

  • Milestone changed from Awaiting Review to 3.4

#2 @griffinjt
11 years ago

Letting straight HTML come through doesn't seem like the safest way to go. Why not just filter using wp_kses_post()? Not escaping allows for <script> tags to pass through, so if we want to add HTML, let's at least filter what type of HTML tags can come through. I've attached an updated diff for it.

11 years ago

#3 @SergeyBiryukov
11 years ago

I don't see a reason to escape the text here. If someone calls wp_get_attachment_link() with <script> tags, they could as well insert them into the template file directly.

That would be inconsistent with other *_link() functions which don't escape anchor text:

  • the_feed_link()
  • post_comments_feed_link()
  • edit_term_link()
  • edit_post_link()
  • edit_comment_link()
  • edit_bookmark_link()
  • adjacent_post_link()
  • get_next_posts_link()
  • get_previous_posts_link()
  • get_next_comments_link()
  • get_previous_comments_link()

#4 @nacin
11 years ago

  • Keywords commit added

We try to avoid kses on the frontend for performance reasons.

Removing the escaping seems appropriate here.

#5 @ryan
11 years ago

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

In [20654]:

Don't escape anchor text as an attributein wp_get_attachment_link(). Props SergeyBiryukov. fixes #19282

Note: See TracTickets for help on using tickets.