Skip to content
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

File uploads #313

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

File uploads #313

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3ebc94a
Initial cURL file upload support
soulseekah Feb 11, 2018
1b60f14
Initial fsockopen transport upload support
soulseekah Feb 11, 2018
a47689d
Introduce Requests_File for simple file uploads
soulseekah Feb 15, 2018
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
9 changes: 5 additions & 4 deletions library/Requests.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,13 +259,14 @@ public static function trace($url, $headers = array(), $options = array()) {
* @param array $headers
* @param array $data
* @param array $options
* @param array $files
* @return Requests_Response
*/
/**
* Send a POST request
*/
public static function post($url, $headers = array(), $data = array(), $options = array()) {
return self::request($url, $headers, $data, self::POST, $options);
public static function post($url, $headers = array(), $data = array(), $options = array(), $files = array()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO, this should either be part of $data or part of $options. For the former, I wouldn't be opposed to having a Requests_File interface (with a default implementation) so that you could do e.g:

$response = Requests::post(
    'http://example.com',
    [ 'Content-Type' => 'text/plain' ],
    new Requests_File( __DIR__ . '/README.txt' )
);

(And potentially allow an array of these.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good idea, would allow us to set filenames and mime-types, just like cURL_File does, I was originally intending to do it as part of data, then remembered how this library is based on the Python Requests library and thought that following their convention would be appreciated.

If not, I have nothing against wrapping into a Requests_File instead.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Python has the benefit of named parameters, so in Python it's:

requests.post( 'http://example.com', files={ 'file': open( 'x', 'rb' ) } )

The equivalent for PHP is our $options array, but we already deviate a bit anyway due to the nature of the language, so I think overloading the $data parameter makes more sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, makes sense. I think Requests_File is the more elegant way of doing it then. I'll work on this a bit and send more stuff into here. Thanks.

Copy link
Collaborator

Choose a reason for hiding this comment

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

btw, for the interface, I'd recommend calling it Requests_File_Interface, and having get_headers() and get_body() methods, which should allow an advanced implementation to offer multipart uploads (although maybe not in Requests itself) or uploading using a data string.

The default implementation should be Requests_File, and we should be able to imply most stuff (upload name, size, etc) from the filename passed in.

return self::request($url, $headers, $data, self::POST, $options, $files);
}
/**
* Send a PUT request
Expand Down Expand Up @@ -354,7 +355,7 @@ public static function patch($url, $headers, $data = array(), $options = array()
* @param array $options Options for the request (see description for more information)
* @return Requests_Response
*/
public static function request($url, $headers = array(), $data = array(), $type = self::GET, $options = array()) {
public static function request($url, $headers = array(), $data = array(), $type = self::GET, $options = array(), $files = array()) {
if (empty($options['type'])) {
$options['type'] = $type;
}
Expand All @@ -376,7 +377,7 @@ public static function request($url, $headers = array(), $data = array(), $type
$capabilities = array('ssl' => $need_ssl);
$transport = self::get_transport($capabilities);
}
$response = $transport->request($url, $headers, $data, $options);
$response = $transport->request($url, $headers, $data, $options, $files);

$options['hooks']->dispatch('requests.before_parse', array(&$response, $url, $headers, $data, $type, $options));

Expand Down
25 changes: 20 additions & 5 deletions library/Requests/Transport/cURL.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,13 @@ public function __destruct() {
* @param array $headers Associative array of request headers
* @param string|array $data Data to send either as the POST body, or as parameters in the URL for a GET/HEAD
* @param array $options Request options, see {@see Requests::response()} for documentation
* @param array $files File uploads
* @return string Raw HTTP result
*/
public function request($url, $headers = array(), $data = array(), $options = array()) {
public function request($url, $headers = array(), $data = array(), $options = array(), $files = array()) {
$this->hooks = $options['hooks'];

$this->setup_handle($url, $headers, $data, $options);
$this->setup_handle($url, $headers, $data, $options, $files);

$options['hooks']->dispatch('curl.before_send', array(&$this->handle));

Expand Down Expand Up @@ -305,8 +306,9 @@ public function &get_subrequest_handle($url, $headers, $data, $options) {
* @param array $headers Associative array of request headers
* @param string|array $data Data to send either as the POST body, or as parameters in the URL for a GET/HEAD
* @param array $options Request options, see {@see Requests::response()} for documentation
* @param array $files File uploads
*/
protected function setup_handle($url, $headers, $data, $options) {
protected function setup_handle($url, $headers, $data, $options, $files = array()) {
$options['hooks']->dispatch('curl.before_request', array(&$this->handle));

// Force closing the connection for old versions of cURL (<7.22).
Expand All @@ -321,13 +323,26 @@ protected function setup_handle($url, $headers, $data, $options) {

if ($data_format === 'query') {
$url = self::format_get($url, $data);
$data = '';
$data = array();
}
elseif (!is_string($data)) {

if (!is_string($data) && empty($files)) {
$data = http_build_query($data, null, '&');
}
}

if (!empty($files)) {
if (function_exists('curl_file_create')) {
foreach($files as $key => $path) {
$data[$key] = curl_file_create($path);
}
} else {
foreach($files as $key => $path) {
$data[$key] = "@$path";
}
}
}

switch ($options['type']) {
case Requests::POST:
curl_setopt($this->handle, CURLOPT_POST, true);
Expand Down
35 changes: 32 additions & 3 deletions library/Requests/Transport/fsockopen.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,10 @@ class Requests_Transport_fsockopen implements Requests_Transport {
* @param array $headers Associative array of request headers
* @param string|array $data Data to send either as the POST body, or as parameters in the URL for a GET/HEAD
* @param array $options Request options, see {@see Requests::response()} for documentation
* @param array $files File uploads
* @return string Raw HTTP result
*/
public function request($url, $headers = array(), $data = array(), $options = array()) {
public function request($url, $headers = array(), $data = array(), $options = array(), $files = array()) {
$options['hooks']->dispatch('fsockopen.before_request');

$url_parts = parse_url($url);
Expand Down Expand Up @@ -153,14 +154,14 @@ public function request($url, $headers = array(), $data = array(), $options = ar
$out = sprintf("%s %s HTTP/%.1f\r\n", $options['type'], $path, $options['protocol_version']);

if ($options['type'] !== Requests::TRACE) {
if (is_array($data)) {
if (is_array($data) && empty($files)) {
$request_body = http_build_query($data, null, '&');
}
else {
$request_body = $data;
}

if (!empty($data)) {
if (!empty($data) && empty($files)) {
if (!isset($case_insensitive_headers['Content-Length'])) {
$headers['Content-Length'] = strlen($request_body);
}
Expand All @@ -169,6 +170,34 @@ public function request($url, $headers = array(), $data = array(), $options = ar
$headers['Content-Type'] = 'application/x-www-form-urlencoded; charset=UTF-8';
}
}

if (!empty($files)) {
$boundary = sha1(time());
$headers['Content-Type'] = "multipart/form-data; boundary=$boundary";

$request_body = '';

if (!empty($data)) {
foreach ($data as $key => $value) {
$request_body .= "--$boundary\r\n";
$request_body .= "Content-Disposition: form-data; name=\"$key\"";
$request_body .= "\r\n\r\n" . $value . "\r\n";
}
}

foreach ($files as $key => $path) {
$filename = basename($path);
$request_body .= "--$boundary\r\n";
$request_body .= "Content-Disposition: form-data; name=\"$key\"; filename=\"$filename\"";
// @todo Compression, encoding (base64), etc.
// @todo Large files can hit PHP memory limits quite quickly
$request_body .= "\r\n\r\n" . file_get_contents($path) . "\r\n";
}

$request_body .= "--$boundary--\r\n\r\n";

$headers['Content-Length'] = strlen($request_body);
}
}

if (!isset($case_insensitive_headers['Host'])) {
Expand Down
9 changes: 9 additions & 0 deletions tests/Transport/Base.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -845,4 +845,13 @@ public function testBodyDataFormat() {
$this->assertEquals(httpbin('/post'), $result['url']);
$this->assertEquals(array('test' => 'true', 'test2' => 'test'), $result['form']);
}

public function testFileUploads() {
file_put_contents($tmpfile = tempnam(sys_get_temp_dir(), 'requests'), 'some secret bytes, yo');
$request = Requests::post('http://httpbin.org/post', array(), array('foo' => 'bar'), $this->getOptions(), array('file1' => $tmpfile));

$result = json_decode($request->body, true);
$this->assertEquals($result['files']['file1'], 'some secret bytes, yo');
$this->assertEquals($result['form']['foo'], 'bar');
}
}