Make WordPress Core

Opened 2 years ago

Closed 2 years ago

#54256 closed enhancement (fixed)

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

Reported by: sabbirshouvo's profile sabbirshouvo Owned by: sergeybiryukov's profile 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 2 years ago.
Patch added
54256.2.diff (4.3 KB) - added by sabbirshouvo 2 years ago.
update: Added another missing attribute check in wp-admin/themes.php file
54256.3.diff (4.3 KB) - added by sabbirshouvo 2 years ago.
fixed spacing issue while escaping values.

Download all attachments as: .zip

Change History (11)

@sabbirshouvo
2 years ago

Patch added

#1 @sabbirshouvo
2 years ago

  • Keywords has-patch added

#2 follow-up: @audrasjb
2 years 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
2 years 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
2 years 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
2 years ago

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

@sabbirshouvo
2 years ago

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

#6 @audrasjb
2 years 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
2 years ago

fixed spacing issue while escaping values.

#7 @SergeyBiryukov
2 years ago

  • Milestone changed from Awaiting Review to 5.9

#8 @SergeyBiryukov
2 years 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.