Make WordPress Core

Opened 2 years ago

Closed 2 years ago

#19282 closed defect (bug) (fixed)

wp_get_attachment_link() does not allow HTML in link text

Reported by: SergeyBiryukov Owned by: 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 2 years ago.
19282.diff (473 bytes) - added by griffinjt 2 years ago.

Download all attachments as: .zip

Change History (7)

SergeyBiryukov2 years ago

comment:1 SergeyBiryukov2 years ago

  • Milestone changed from Awaiting Review to 3.4

comment:2 griffinjt2 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.

griffinjt2 years ago

comment:3 SergeyBiryukov2 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()

comment:4 nacin2 years ago

  • Keywords commit added

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

Removing the escaping seems appropriate here.

comment:5 ryan2 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.