WordPress.org

Make WordPress Core

Opened 19 months ago

Closed 18 months ago

Last modified 18 months 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:

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 18 months ago.
Limits uploads to only images in custom-header.php and custom-background.php
22149.2.diff (1.5 KB) - added by kovshenin 18 months ago.
Uses wp_match_mime_types to compare to valid image mime types.
22149.3.diff (2.6 KB) - added by kovshenin 18 months ago.
Added a similar scenario for async uploads.

Download all attachments as: .zip

Change History (20)

comment:1 nacin19 months ago

  • Milestone changed from Awaiting Review to 3.5

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

kovshenin18 months ago

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

kovshenin18 months ago

Uses wp_match_mime_types to compare to valid image mime types.

kovshenin18 months ago

Added a similar scenario for async uploads.

comment:3 kovshenin18 months 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.

comment:4 kovshenin18 months ago

  • Keywords has-patch added; needs-patch removed

comment:5 follow-up: MikeHansenMe18 months 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.

comment:6 in reply to: ↑ 5 kovshenin18 months ago

Replying to MikeHansenMe:

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

comment:7 MikeHansenMe18 months ago

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

comment:8 nacin18 months ago

  • Keywords commit added

Needs testing and commit.

comment:9 DrewAPicture18 months ago

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

comment:10 koopersmith18 months 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.

comment:11 koopersmith18 months ago

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

comment:12 koopersmith18 months 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.

comment:13 nacin18 months 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.

comment:14 nacin18 months 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.

comment:15 nacin18 months ago

In 22519:

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

comment:16 nacin18 months 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.

comment:17 nacin18 months ago

I opened #22403 as a follow-up.

Note: See TracTickets for help on using tickets.