WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#32549 closed defect (bug) (fixed)

wp_remote_get does not stream to temp file

Reported by: andddd Owned by: dd32
Milestone: 4.3 Priority: normal
Severity: normal Version: 4.1
Component: HTTP API Keywords:
Focuses: Cc:

Description

Source:

$filename = wp_unique_filename( get_temp_dir(), basename( $zip_url ) );

var_dump(get_temp_dir());
var_dump($zip_url);
var_dump($filename);

$response = wp_remote_get( $zip_url, array(
  'timeout' => 15, 
  'stream' => true,
  'filename' => $filename
) );

Output:

string(49) "/var/folders/4p/s0vgsy5s19g63q9t0zd56hkw0000gn/T/" 
string(77) "https://bucket.s3-eu-west-1.amazonaws.com/folder/file.zip" 
string(8) "file.zip"

Seems like wp_unique_filename cannot combine filename with temp path. No error returned after download. wp_remote_get crafts download path the same way so it's broken too if I omit filename option.

Change History (5)

#1 @jacobsantos
4 years ago

$filename = get_temp_dir() . '/' . wp_unique_filename( get_temp_dir(), basename( $zip_url ) );
$response = wp_remote_get( $zip_url, array(
    'timeout' => 15, 
    'stream' => true,
    'filename' => $filename
) );

The code in WP_Http::request() does need to be fixed.

#2 @dd32
4 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from new to closed

The stream parameter expects a filename path to be specified, ie. /var/folders/4p/s0vgsy5s19g63q9t0zd56hkw0000gn/T/file.zip, not file.zip. If you pass a relative path into that, it's saved within the current working directory (in my testing, it went into wp-admin/ as I triggered it through wp-admin/admin-post.php).

wp_unique_filename() only returns a file name, it does not return a path, it's designed to find a unique filename within a folder.

It looks like you want to be using $filename = wp_tempnam( $zip_url ); instead of wp_unique_filename().

#3 @dd32
4 years ago

  • Milestone set to 4.3
  • Resolution invalid deleted
  • Status changed from closed to reopened

Failed to realise exactly what the ticket was referring to - WP_HTTP::request() does indeed incorrectly use wp_unique_filename(), and wp_tempnam() is only defined in an admin context.

#4 @dd32
4 years ago

  • Owner set to dd32
  • Resolution set to fixed
  • Status changed from reopened to closed

In 32712:

WP_HTTP: ensure that the temporary file is created within the temporary directly when stream is specified without a filename parameter.
Fixes #32549

#5 @dd32
4 years ago

  • Version changed from 4.2.2 to 4.1

Broken in r29968 via #26726

Note: See TracTickets for help on using tickets.