WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#22149 closed defect (bug) (fixed)

Customizer Uploader doesn't restrict by file type

Reported by: pento Owned by: nacin
Milestone: 3.5 Priority: normal
Severity: normal Version: 3.5
Component: Upload Keywords: has-patch commit
Focuses: Cc:
PR Number:

Description

When using the Theme Customizer to upload a new background image, it doesn't restrict the file types that can be uploaded. For example, I can upload a video file, the file will be uploaded and inserted into the Media Library, then the Customizer will try to treat it like an image.

I think the best way around this would be to restrict uploads in the Customizer to images only.

Attachments (3)

22149.diff (1.9 KB) - added by kovshenin 7 years ago.
Limits uploads to only images in custom-header.php and custom-background.php
22149.2.diff (1.5 KB) - added by kovshenin 7 years ago.
Uses wp_match_mime_types to compare to valid image mime types.
22149.3.diff (2.6 KB) - added by kovshenin 7 years ago.
Added a similar scenario for async uploads.

Download all attachments as: .zip

Change History (20)

#1 @nacin
7 years ago

  • Milestone changed from Awaiting Review to 3.5

#2 @kovshenin
7 years ago

  • Keywords needs-patch added

Confirmed. This is also true for the old Custom Header and Custom Background upload fields. Here's what I get when I upload a PDF as a Header Image: http://cl.ly/image/3P2c1I0o2e2H And when I do that with Custom Background I get the "Background updated" message, and the background-image in body.custom-background now points to my PDF file.

@kovshenin
7 years ago

Limits uploads to only images in custom-header.php and custom-background.php

@kovshenin
7 years ago

Uses wp_match_mime_types to compare to valid image mime types.

@kovshenin
7 years ago

Added a similar scenario for async uploads.

#3 @kovshenin
7 years ago

The async fix in 22149.3.diff won't work unless the Customizer upload control passes on the context, which, judging by the code in customize-controls.js, it should in post_data[context]. Opened #22215.

#4 @kovshenin
7 years ago

  • Keywords has-patch added; needs-patch removed

#5 follow-up: @MikeHansenMe
7 years ago

  • Cc mdhansen@… added

Tested patch 22149.3.diff and 22149.2.diff and both worked on custom-header.php and custom-background.php. However, I am able to upload mp4 files and pdf on the customizer without warning.

#6 in reply to: ↑ 5 @kovshenin
7 years ago

Replying to MikeHansenMe:

22149.3.diff should block non-image uploads in the Customizer only when the patch is #22215 is applied.

#7 @MikeHansenMe
7 years ago

Confirmed with patch from #22215 22149.3.diff blocks the upload.

#8 @nacin
7 years ago

  • Keywords commit added

Needs testing and commit.

#9 @DrewAPicture
7 years ago

22149.3.diff with 22215.diff works as expected for me.

#10 @koopersmith
7 years ago

For custom headers and backgrounds, this looks solid.

For the customizer (and anything that uses Plupload), we can limit the filetypes that a Plupload will accept via JS (see the filter property), and could block the uploads earlier on as a result.

At this point, it probably makes sense to land the customizer-specific code as well.

#11 @koopersmith
7 years ago

In [22492] — Customizer: Restrict image control uploaders to only allow image file types. see #22149.

#12 @koopersmith
7 years ago

While [22492] works, Plupload's falls back to the browser's default behavior for filetypes it doesn't recognize, which is pretty universally a terrible experience. So we might just want to reject it PHP-side for now.

#13 @nacin
7 years ago

In theory, [22492] is the best solution. In practice, Plupload handles disallowed files terribly, by falling back to the browser default. That means HTML files replace the entire viewport, ZIP files get re-downloaded, etc. I'd rather revert and go with solely 22149.3.diff.

#14 @nacin
7 years ago

koopersmith and I were both testing this with drag events. This is definitely a better experience for the dialog, because non-images are grayed out. We should be able to intercept the drag event for disallowed files in our own Plupload handlers/wrappers. So I'm going to leave it for now.

#15 @nacin
7 years ago

In 22519:

Customizer uploader control: Pass arrays of extensions around instead. see #22149.

#16 @nacin
7 years ago

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

In 22521:

Don't allow non-image uploads for custom headers and backgrounds. props kovshenin. fixes #22149.

#17 @nacin
7 years ago

I opened #22403 as a follow-up.

Note: See TracTickets for help on using tickets.