Make WordPress Core

Opened 4 years ago

Closed 6 months ago

Last modified 6 months ago

#53167 closed defect (bug) (fixed)

upload_filetypes should map to get_allowed_mime_types

Reported by: spacedmonkey's profile spacedmonkey Owned by: adamsilverstein's profile adamsilverstein
Milestone: 6.6 Priority: normal
Severity: normal Version: 3.0
Component: Networks and Sites Keywords: has-patch has-unit-tests commit needs-dev-note
Focuses: multisite, performance Cc:

Description

Currently in multisite upload_filetypes site option is massively out of date with mime / ext allowed in get_allowed_mime_types.

Attachments (1)

53167.diff (1.7 KB) - added by adamsilverstein 6 months ago.

Download all attachments as: .zip

Change History (33)

This ticket was mentioned in PR #1232 on WordPress/wordpress-develop by spacedmonkey.


4 years ago
#1

  • Keywords has-patch added

This ticket was mentioned in Slack in #core-multisite by spacedmonkey. View the logs.


4 years ago

#3 @flixos90
3 years ago

  • Focuses performance added

This also relates to performance since it complicates usage of modern image formats like WebP on multisite environments.

#4 @adamsilverstein
10 months ago

  • Milestone changed from Awaiting Review to 6.6
  • Owner set to adamsilverstein
  • Status changed from new to assigned

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


9 months ago

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


8 months ago

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


8 months ago

#8 @adamsilverstein
8 months ago

  • Keywords needs-testing-info has-unit-tests added

PR looks good, thanks @spacedmonkey! Some steps to test would be helpful here so we can get some manual testing.

This ticket was mentioned in PR #6380 on WordPress/wordpress-develop by @pbearne.


8 months ago
#9

Trac ticket: 53167

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


7 months ago

#11 @oglekler
7 months ago

I didn't find difference between patches 🤔
@pbearne and @spacedmonkey can you provide testing instructions please?

#12 @spacedmonkey
7 months ago

@oglekler this change will only effect new installs of multisite. You will have to reset the local environment and install multisite with this patch installed to see this change.

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


7 months ago

This ticket was mentioned in Slack in #core-performance by joemcgill. View the logs.


6 months ago

#15 @adamsilverstein
6 months ago

I didn't find difference between patches 🤔

I think its just updated against trunk.

@pbearne and @spacedmonkey can you provide testing instructions please?

  1. Test with a new install of a multisite network with the patch in place. Ideally on a server that supports AVIF.
  2. Create a new site in the multisite
  3. try uploading an AVIF image in the media library, it should work.

Check wp site option get upload_filetypes to see if the option includes avif for example.

Last edited 6 months ago by adamsilverstein (previous) (diff)

This ticket was mentioned in Slack in #core-test by oglekler. View the logs.


6 months ago

#17 @adamsilverstein
6 months ago

53167.diff is a refresh against trunk

#18 @adamsilverstein
6 months ago

  • Type changed from enhancement to defect (bug)

marking this as a bug for clarity.

#19 @spacedmonkey
6 months ago

Steps to replicate:

Install patch / checkout PR
Create a fresh install of WordPress.
Convert site to multisite.
Go Network > Settings

In the Upload file types field it should look like this.

jpg jpeg jpe gif png bmp tiff tif webp avif ico heic asf asx wmv wmx wm avi divx flv mov qt mpeg mpg mpe mp4 m4v ogv webm mkv 3gp 3gpp 3g2 3gp2 txt asc c cc h srt csv tsv ics rtx css vtt dfxp mp3 m4a m4b aac ra ram wav ogg oga flac mid midi wma wax mka rtf pdf class tar zip gz gzip rar 7z psd xcf doc pot pps ppt wri xla xls xlt xlw mdb mpp docx docm dotx dotm xlsx xlsm xlsb xltx xltm xlam pptx pptm ppsx ppsm potx potm ppam sldx sldm onetoc onetoc2 onetmp onepkg oxps xps odt odp ods odg odc odb odf wp wpd key numbers pages

Which is different from the current list.

$misc_exts        = array(
                // Images.
                'jpg',
                'jpeg',
                'png',
                'gif',
                'webp',
                // Video.
                'mov',
                'avi',
                'mpg',
                '3gp',
                '3g2',
                // "audio".
                'midi',
                'mid',
                // Miscellaneous.
                'pdf',
                'doc',
                'ppt',
                'odt',
                'pptx',
                'docx',
                'pps',
                'ppsx',
                'xls',
                'xlsx',
                'key',
        );

CC @joemcgill @pbearne @mukesh27

This ticket was mentioned in Slack in #core-performance by spacedmonkey. View the logs.


6 months ago

#21 @costdev
6 months ago

Playground links:

Last edited 6 months ago by costdev (previous) (diff)

This ticket was mentioned in Slack in #core-performance by spacedmonkey. View the logs.


6 months ago

#23 @pavanpatil1
6 months ago

Test Report

Patch tested - https://github.com/WordPress/wordpress-develop/pull/6380

Environment:
OS: Windows 11
Browser: Chrome v125

Actual Result: The newly added file extensions are now properly visible in the "Upload File Types" section within network settings. Also, files with these extensions can be uploaded successfully✅

Screenshots:
Upload file types in network settings: https://prnt.sc/JExTb0WrsBMH
Upload .avif file: https://prnt.sc/-J73tfInlD1a

#24 @spacedmonkey
6 months ago

  • Keywords commit added

As this has now been tested and validated it works, I am marking as commit.

This ticket was mentioned in Slack in #core-performance by spacedmonkey. View the logs.


6 months ago

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


6 months ago

#27 @joemcgill
6 months ago

One thing that I was curious about when looking at this patch is whether there was a reason that the default mime types allowed for multisite was chosen for a good reason. Looking back at #24434, which is an early example of updating this list, the sentiment by Core devs at the time was that the list seemed somewhat arbitrary, so I think that pulling this list from get_allowed_mime_types makes sense.

I'll also note that this will only affect newly created networks, since this is only used to populate the schema for new networks, not change the allowed mime types for existing networks. To do so, a network admin would need to updated the list of allowed upload file types in /wp-admin/network/settings.php, which updates the upload_filetypes option for the site. I confirmed that applying this PR to an existing site does not change the set of allowed file types.

I think this makes sense and makes maintenance of this list much less manual.

#28 @rajinsharwar
6 months ago

  • Keywords needs-testing-info removed

Test Report

Patch tested: PR-6380

Screenshots

Added ZIP in allowed file list: https://prnt.sc/zPjZaT4x__tr
Can upload the ZIP file: https://prnt.sc/1TrSatbnXi-t

✅ The issue is resolved with a patch.

Removing 'needs-testing-info' as Comment:19 has the information.

This ticket was mentioned in Slack in #core-performance by joemcgill. View the logs.


6 months ago

#30 @tb1909
6 months ago

Test Report

Patch tested: PR-6380

✅ The issue is resolved with a patch.

Last edited 6 months ago by tb1909 (previous) (diff)

#31 @adamsilverstein
6 months ago

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

In 58400:

Networks and sites: use get_allowed_mime_types to populate multisite’s upload_filetypes.

Ensure new multisite installs are up to date with the current mime types supported in core.

Note that this will only affect newly created networks, since this is only used to populate the schema for new networks, not change the allowed mime types for existing networks

Props spacedmonkey, costdev, pavanpatil1, joemcgill, rajinsharwar, tb1909.
Fixes #53167.

#32 @adamsilverstein
6 months ago

  • Keywords needs-dev-note added
Note: See TracTickets for help on using tickets.