[wp-trac] [WordPress Trac] #38231: Allow download_url to respect content-disposition header

[WordPress Trac]

#38231: Allow download_url to respect content-disposition header
--------------------------------------+------------------------------
 Reporter:  cklosows                  |       Owner:  johnjamesjacoby
     Type:  enhancement               |      Status:  assigned
 Priority:  normal                    |   Milestone:  5.9
Component:  HTTP API                  |     Version:  4.7
 Severity:  normal                    |  Resolution:
 Keywords:  has-patch has-unit-tests  |     Focuses:
--------------------------------------+------------------------------

Comment (by johnjamesjacoby):

 [https://core.trac.wordpress.org/attachment/ticket/38231/38231.7.diff
 38231.7.diff] includes the following iterations:

 * Abandons
 `WP_REST_Attachments_Controller::get_filename_from_disposition()`
 approach. @costdev and I collaborated privately for a few full days, and
 concluded that it does not fully sanitize a filename from an untrusted
 source in the many ways that `download_url()` can be used.
 * Looks for `'attachment; filename='` instead of `filename=`. Per the
 [https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-
 Disposition specification], the `Content-Disposition` header only triggers
 save-as dialogs when the `attachment;` "type" is sent with it. (The
 `inline;` type would be displayed as a web page instead of being
 downloaded anyways. The type is a required parameter.)
 * Uses `sanitize_file_name()` to ensure that the suggested `filename`
 value is as close to acceptable for the web server and WordPress rules as
 possible. The nice thing about this is that values with a period in them
 will automatically be checked against `wp_get_mime_types()` using
 `wp_check_filetype()`. The value in doing this here (early) is that the
 temporary file name can still be used if the suggestion happens to be
 illegal.
 * Uses `validate_file()` to avoid directory traversal. This adequately
 prevents the `filename` value from including relative file paths and
 writing to unintended locations in the file system.
 * Includes 6 separate tests with 9 total assertions. These tests confirm
 that valid file name values are correctly saved, and invalid file name
 values are correctly rejected.

 Massive props again to @costdev for going the extra mile with me behind
 the scenes on this. 🙏

 Barring anything we've missed, this patch is ready to be committed.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/38231#comment:17>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform