Skip to content

Set the file extension when is not present in the URL #2690

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Set the file extension when is not present in the URL #2690

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f3f6d1f
If file extension is not present use the mime type
mitogh May 8, 2022
dda2634
Format code in order to follow `phpcs`
mitogh May 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Format code in order to follow phpcs 
Make sure files follow the rules established in this repository
  • Loading branch information
@mitogh
mitogh committed May 8, 2022
commit dda26347c5e86715c7b276652be74224dab37fa1
4 changes: 2 additions & 2 deletions src/wp-admin/includes/file.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1213,8 +1213,8 @@ function download_url( $url, $timeout = 300, $signature_verification = false ) {
$mime_type = wp_remote_retrieve_header( $response, 'content-type' );
if ( $mime_type && 'tmp' === pathinfo( $tmpfname, PATHINFO_EXTENSION ) ) {
$valid_mime_types = array_flip( get_allowed_mime_types() );
if ( ! empty( $valid_mime_types[ $mime_type] ) ) {
$extensions = explode( '|', $valid_mime_types[ $mime_type ] );
if ( ! empty( $valid_mime_types[ $mime_type ] ) ) {
$extensions = explode( '|', $valid_mime_types[ $mime_type ] );
$new_image_name = str_replace( '.tmp', ".{$extensions[0]}", $tmpfname );
Copy link
Member

@dmsnell dmsnell Aug 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we're going to intend on replacing the extension, it would be helpful to ensure that we only replace a terminating .tmp instead of any .tmp because a file could be named something like file.tmp.dat.tmp and then we'd transform more than we want.

here we could use a PCRE function to ensure this is the end that we're replacing.

$new_image_name = preg_replace( '/\.tmp$/', ".{$extensions[0]}", $tmpfname );

We could also do this with substr(), if we're confident that pathinfo is returning tmp only when the file ends in .tmp.

Copy link
Contributor

@joedolson joedolson Sep 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the PHP docs, PATHINFO_EXTENSION only returns the last extension if a path has more than one:

https://www.php.net/manual/en/function.pathinfo.php

So it seems to me that we should have that confidence.

if ( 0 === validate_file( $new_image_name ) ) {
if ( rename( $tmpfname, $new_image_name ) ) {
Expand Down
14 changes: 8 additions & 6 deletions tests/phpunit/tests/admin/includesFile.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,12 +403,14 @@ public function test_download_url_should_use_the_content_type_header_to_set_exte
* Data provider for test_download_url_should_use_the_content_type_header_to_set_extension_of_a_file_if_extension_was_not_determined
*
* @see test_download_url_should_use_the_content_type_header_to_set_extension_of_a_file_if_extension_was_not_determined()
* @test
* @ticket 54738
*
* @return Generator
*/
public function data_download_url_should_use_the_content_type_header_to_set_extension_of_a_file_if_extension_was_not_determined() {
yield 'Content-Type header in the response' => array(
function(){
function () {
return array(
'response' => array(
'code' => 200,
Expand All @@ -418,11 +420,11 @@ function(){
),
);
},
'.jpg'
'.jpg',
);

yield 'Invalid Content-Type header' => array(
function(){
function () {
return array(
'response' => array(
'code' => 200,
Expand All @@ -432,11 +434,11 @@ function(){
),
);
},
'.tmp'
'.tmp',
);

yield 'Valid content type but not supported mime type' => array(
function(){
function () {
return array(
'response' => array(
'code' => 200,
Expand All @@ -446,7 +448,7 @@ function(){
),
);
},
'.tmp'
'.tmp',
);
}
}