WordPress.org

Make WordPress Core

Opened 8 weeks ago

Closed 4 weeks ago

#54256 closed enhancement (fixed)

Properly escape url and attributes in wp-admin/themes.php

Reported by: sabbirshouvo Owned by: SergeyBiryukov
Milestone: 5.9 Priority: normal
Severity: normal Version:
Component: Themes Keywords: has-patch
Focuses: coding-standards Cc:

Description

There are multiple unescaped url and attributes in wp-admin/themes.php
It's against WordPress coding standard.

Attachments (3)

54256.diff (3.6 KB) - added by sabbirshouvo 8 weeks ago.
Patch added
54256.2.diff (4.3 KB) - added by sabbirshouvo 8 weeks ago.
update: Added another missing attribute check in wp-admin/themes.php file
54256.3.diff (4.3 KB) - added by sabbirshouvo 7 weeks ago.
fixed spacing issue while escaping values.

Download all attachments as: .zip

Change History (11)

@sabbirshouvo
8 weeks ago

Patch added

#1 @sabbirshouvo
8 weeks ago

  • Keywords has-patch added

#2 follow-up: @audrasjb
8 weeks ago

  • Version trunk deleted

I think most of these variables don't need to be escaped, since they are generated by WordPress itself and can't be edited in any way.

(removing trunk version)

#3 @audrasjb
8 weeks ago

In my opinion, the only one where we may perhaps consider an escaping function is $theme['screenshot'][0].

#4 in reply to: ↑ 2 @sabbirshouvo
8 weeks ago

Replying to audrasjb:

I think most of these variables don't need to be escaped, since they are generated by WordPress itself and can't be edited in any way.

(removing trunk version)

Thanks for you feedback. I want to mention about few cases where same attributes are escaped and some are not in the same file.

/wp-admin/themes.php
Please check line: 535,548,555,567,870,894,907,913

if these needs attribute escaping then why not line 1120,1129,1140 ?

#5 @audrasjb
8 weeks ago

Hmm in any case you're right we need at least more consistency here :)

@sabbirshouvo
8 weeks ago

update: Added another missing attribute check in wp-admin/themes.php file

#6 @audrasjb
8 weeks ago

Small WordPress Coding Standards issue: we need spaces in each function’s parenthesis.
Example: replace esc_attr($aria_name) with esc_attr( $aria_name )

@sabbirshouvo
7 weeks ago

fixed spacing issue while escaping values.

#7 @SergeyBiryukov
4 weeks ago

  • Milestone changed from Awaiting Review to 5.9

#8 @SergeyBiryukov
4 weeks ago

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

In 52020:

Coding Standards: Consistently escape attribute in wp-admin/themes.php.

Follow-up to [27012], [38057], [47816], [51083].

Props sabbirshouvo, audrasjb.
Fixes #54256.

Note: See TracTickets for help on using tickets.