Make WordPress Core

Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#37924 closed defect (bug) (fixed)

Cannot delete or update themes in directories containing an uppercase letter from Appearance > Themes

Reported by: chrisjean's profile chrisjean Owned by: obenland's profile obenland
Milestone: 4.7 Priority: normal
Severity: normal Version: 4.7
Component: Themes Keywords: has-patch
Focuses: administration Cc:

Description

The shiny updates v2 code changes [37714] introduced a bug where themes in directories with one or more uppercase letters in the directory name cannot be deleted or updated from inside the Appearance > Themes page. This is due to the $_POST['slug'] data being sanitized using sanitize_key() which forces uppercase characters to lowercase.

The shiny updates v2 changes did not create the same problem with plugins since plugins keep track of plugin (akismet/akismet.php) and slug (akismet) separately with only the slug being passed through sanitize_key().

Looking at the plugin-handling code, the plugin value is sanitized using sanitize_text_field(). The attached patch updates the theme code to use sanitize_text_field() rather than sanitize_key() when sanitizing the slug. In my testing, this fixes both updating and deleting themes in directories with uppercase characters.

I should note that while there aren't any themes on .org that have an uppercase letter in the directory name, all of the themes released by iThemes.com (my employer) use uppercase letters in the theme directory name, I've seen other theme vendors do the same, and I've seen many customer sites where they have custom theme directory names that include uppercase letters.

Attachments (1)

37924.patch (712 bytes) - added by chrisjean 9 years ago.

Download all attachments as: .zip

Change History (7)

@chrisjean
9 years ago

#1 @obenland
9 years ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 4.7
  • Owner set to obenland
  • Status changed from new to reviewing

#2 @aaroncampbell
9 years ago

I saw this slot into 4.7. I'm on the road at the moment, and I know it's too late for 4.6.1, but I do think it was a regression. If there's another minor release, we should consider it.

#3 @jorbin
9 years ago

@obenland Can you review and test? I don't want this to slip from 4.7

#4 @obenland
9 years ago

Will do

#5 @obenland
9 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 38710:

Themes: Account for uppercase chars when managing themes.

Fixes a bug where the UI wasn't updated after deleting/updating a theme.

Also introduces unit tests for theme management ajax handlers. For now they're
focused on wp_ajax_update_theme() but they can include tests for other
handlers as well.

Props chrisjean for initial patch.
Fixes #37924.

#6 @obenland
9 years ago

In 38712:

Tests: Add newly introduced theme to theme list.

Introduced in [38710].

See #37924.

Note: See TracTickets for help on using tickets.