From f054e3d7634c3c0bb2c0e62fc7f98cdcb8621ff5 Mon Sep 17 00:00:00 2001 From: Ryan McCue Date: Mon, 26 Oct 2015 16:32:19 +1000 Subject: [PATCH 1/9] Allow passing a stream as $data --- library/Requests/Transport/cURL.php | 33 +++++--- library/Requests/Transport/fsockopen.php | 98 +++++++++++++++++++----- 2 files changed, 100 insertions(+), 31 deletions(-) diff --git a/library/Requests/Transport/cURL.php b/library/Requests/Transport/cURL.php index c1ad4466a..a06bb628f 100755 --- a/library/Requests/Transport/cURL.php +++ b/library/Requests/Transport/cURL.php @@ -299,27 +299,40 @@ protected function setup_handle($url, $headers, $data, $options) { $url = self::format_get($url, $data); $data = ''; } - elseif (!is_string($data)) { - $data = http_build_query($data, null, '&'); + elseif ($options['type'] !== Requests::TRACE) { + if (is_resource($data)) { + $stat = fstat($data); + curl_setopt($this->handle, CURLOPT_INFILE, $data); + curl_setopt($this->handle, CURLOPT_INFILESIZE, $stat['size']); + + // We need to set CURLOPT_PUT so that cURL uses INFILE, but + // this will set the request type to PUT by default. We need + // to set CURLOPT_CUSTOMREQUEST to set it back. + curl_setopt($this->handle, CURLOPT_PUT, true); + curl_setopt($this->handle, CURLOPT_CUSTOMREQUEST, $options['type']); + } + elseif (!is_string($data)) { + $data = http_build_query($data, null, '&'); + curl_setopt($this->handle, CURLOPT_POSTFIELDS, $data); + } + else { + curl_setopt($this->handle, CURLOPT_POSTFIELDS, $data); + } } } switch ($options['type']) { case Requests::POST: curl_setopt($this->handle, CURLOPT_POST, true); - curl_setopt($this->handle, CURLOPT_POSTFIELDS, $data); break; + + case Requests::HEAD: + curl_setopt($this->handle, CURLOPT_NOBODY, true); + // Fall-through case Requests::PATCH: case Requests::PUT: case Requests::DELETE: case Requests::OPTIONS: - curl_setopt($this->handle, CURLOPT_CUSTOMREQUEST, $options['type']); - curl_setopt($this->handle, CURLOPT_POSTFIELDS, $data); - break; - case Requests::HEAD: - curl_setopt($this->handle, CURLOPT_CUSTOMREQUEST, $options['type']); - curl_setopt($this->handle, CURLOPT_NOBODY, true); - break; case Requests::TRACE: curl_setopt($this->handle, CURLOPT_CUSTOMREQUEST, $options['type']); break; diff --git a/library/Requests/Transport/fsockopen.php b/library/Requests/Transport/fsockopen.php index 197d513b2..c8762e8cb 100755 --- a/library/Requests/Transport/fsockopen.php +++ b/library/Requests/Transport/fsockopen.php @@ -34,6 +34,13 @@ class Requests_Transport_fsockopen implements Requests_Transport { */ public $info; + /** + * Request body to send. + * + * @var stream|string|null + */ + protected $request_body = null; + /** * What's the maximum number of bytes we should keep? * @@ -138,7 +145,7 @@ public function request($url, $headers = array(), $data = array(), $options = ar if ($data_format === 'query') { $path = self::format_get($url_parts, $data); - $data = ''; + $data = null; } else { $path = self::format_get($url_parts, array()); @@ -146,27 +153,11 @@ public function request($url, $headers = array(), $data = array(), $options = ar $options['hooks']->dispatch('fsockopen.remote_host_path', array(&$path, $url)); - $request_body = ''; + $this->request_body = ''; $out = sprintf("%s %s HTTP/%.1f\r\n", $options['type'], $path, $options['protocol_version']); - if ($options['type'] !== Requests::TRACE) { - if (is_array($data)) { - $request_body = http_build_query($data, null, '&'); - } - else { - $request_body = $data; - } - - if (!empty($data)) { - if (!isset($case_insensitive_headers['Content-Length'])) { - $headers['Content-Length'] = strlen($request_body); - } - - if (!isset($case_insensitive_headers['Content-Type'])) { - $headers['Content-Type'] = 'application/x-www-form-urlencoded; charset=UTF-8'; - } - } - } + $body_headers = $this->prepare_body($data, $case_insensitive_headers, $options); + $headers = array_merge($headers, $body_headers); if (!isset($case_insensitive_headers['Host'])) { $out .= sprintf('Host: %s', $url_parts['host']); @@ -202,11 +193,16 @@ public function request($url, $headers = array(), $data = array(), $options = ar $out .= "Connection: Close\r\n"; } - $out .= "\r\n" . $request_body; + $out .= "\r\n"; + if (is_string($this->request_body)) { + $out .= $this->request_body; + } $options['hooks']->dispatch('fsockopen.before_send', array(&$out)); fwrite($socket, $out); + $this->send_body($socket); + $options['hooks']->dispatch('fsockopen.after_send', array($out)); if (!$options['blocking']) { @@ -317,6 +313,66 @@ public function request_multiple($requests, $options) { return $responses; } + /** + * Prepare the body data to send. + * + * @param string|resource|null $data Data as a string, stream resource, or null. + * @param array|Requests_Utility_CaseInsensitiveDictionary $headers Headers set on the request. + * @param array $options Options set on the request. + * @return array Extra headers to add to the request. + */ + protected function prepare_body($data, $headers, $options) { + if (empty($data)) { + return array(); + } + + $body_headers = array(); + if ($options['type'] !== Requests::TRACE) { + if (is_array($data)) { + $this->request_body = http_build_query($data, null, '&'); + $length = strlen($this->request_body); + } + elseif (is_resource($data)) { + $this->request_body = $data; + $stat = fstat($data); + $length = $stat['size']; + } + else { + $this->request_body = $data; + $length = strlen($this->request_body); + } + + if (!empty($data)) { + if (!isset($headers['Content-Length'])) { + $body_headers['Content-Length'] = $length; + } + + if (!isset($headers['Content-Type'])) { + $body_headers['Content-Type'] = 'application/x-www-form-urlencoded; charset=UTF-8'; + } + } + } + + return $body_headers; + } + + /** + * Send body data with the request. + * + * @param resource $stream Remote socket for the server. + */ + protected function send_body($stream) { + if (!is_resource($this->request_body)) { + // Already sent + return; + } + + while (!feof($this->request_body)) { + $bytes = fread($this->request_body, Requests::BUFFER_SIZE); + fwrite($stream, $bytes); + } + } + /** * Retrieve the encodings we can accept * From 9217c5d48bd58169dbd4eb9490dbcb9c197a87ea Mon Sep 17 00:00:00 2001 From: Ryan McCue Date: Mon, 26 Oct 2015 16:33:06 +1000 Subject: [PATCH 2/9] Check the stream is valid before using --- library/Requests.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/library/Requests.php b/library/Requests.php index 8f1f7be33..422f5b6d5 100755 --- a/library/Requests.php +++ b/library/Requests.php @@ -572,6 +572,16 @@ protected static function set_defaults(&$url, &$headers, &$data, &$type, &$optio $options['data_format'] = 'body'; } } + + // Check the data stream is valid + if (is_resource($data)) { + if ($options['data_format'] !== 'body') { + throw new Requests_Exception('Streams can only be sent in the request body.', 'requests.stream_as_query', $data); + } + if (get_resource_type($data) !== 'stream') { + throw new Requests_Exception('Invalid stream resource for request body.', 'requests.invalid_stream', $data); + } + } } /** From 258acf16d93ef23f431907338a7514b540f68d23 Mon Sep 17 00:00:00 2001 From: Ryan McCue Date: Mon, 26 Oct 2015 16:42:05 +1000 Subject: [PATCH 3/9] Bail if the stream doesn't support stat --- library/Requests/Transport/cURL.php | 3 +++ library/Requests/Transport/fsockopen.php | 3 +++ 2 files changed, 6 insertions(+) diff --git a/library/Requests/Transport/cURL.php b/library/Requests/Transport/cURL.php index a06bb628f..b5c278e34 100755 --- a/library/Requests/Transport/cURL.php +++ b/library/Requests/Transport/cURL.php @@ -302,6 +302,9 @@ protected function setup_handle($url, $headers, $data, $options) { elseif ($options['type'] !== Requests::TRACE) { if (is_resource($data)) { $stat = fstat($data); + if (!$stat) { + throw new Requests_Exception('Body stream resource does not support stat.', 'requests.stream_no_stat', $stat); + } curl_setopt($this->handle, CURLOPT_INFILE, $data); curl_setopt($this->handle, CURLOPT_INFILESIZE, $stat['size']); diff --git a/library/Requests/Transport/fsockopen.php b/library/Requests/Transport/fsockopen.php index c8762e8cb..5a4095038 100755 --- a/library/Requests/Transport/fsockopen.php +++ b/library/Requests/Transport/fsockopen.php @@ -335,6 +335,9 @@ protected function prepare_body($data, $headers, $options) { elseif (is_resource($data)) { $this->request_body = $data; $stat = fstat($data); + if (!$stat) { + throw new Requests_Exception('Body stream resource does not support stat.', 'requests.stream_no_stat', $stat); + } $length = $stat['size']; } else { From e569575f31692ebfb429ab75ab60efb9dcaffb6b Mon Sep 17 00:00:00 2001 From: Ryan McCue Date: Mon, 26 Oct 2015 17:00:06 +1000 Subject: [PATCH 4/9] Add tests for stream inputs --- tests/Transport/Base.php | 52 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/tests/Transport/Base.php b/tests/Transport/Base.php index 95d03560e..23b8c713d 100755 --- a/tests/Transport/Base.php +++ b/tests/Transport/Base.php @@ -181,6 +181,42 @@ public function testPOSTWithNestedData() { $this->assertEquals(array('test' => 'true', 'test2[test3]' => 'test', 'test2[test4]' => 'test-too'), $result['form']); } + public function streamProvider() { + $streams = array(); + + // Regular stream + $contents = file_get_contents(__FILE__); + $stream = fopen(__FILE__, 'r'); + $streams[] = array($stream, $contents, strlen($contents)); + + // In-memory stream + $string = "Hello! \xF0\x9F\x92\xA9"; + $stream = fopen('php://memory', 'r+'); + fwrite($stream, $string); + + // Reset for reading + rewind($stream); + $streams[] = array($stream, $string, strlen($string)); + + return $streams; + } + + /** + * @dataProvider streamProvider + */ + public function testPOSTWithStream($stream, $expected_data, $expected_length) { + $request = Requests::post(httpbin('/post'), array(), $stream, $this->getOptions()); + fclose($stream); + $this->assertEquals(200, $request->status_code); + + $result = json_decode($request->body, true); + $this->assertEquals($expected_data, $result['data']); + + // Check the length we sent + $sent_headers = new Requests_Utility_CaseInsensitiveDictionary($result['headers']); + $this->assertEquals($expected_length, $sent_headers['Content-Length']); + } + public function testRawPUT() { $data = 'test'; $request = Requests::put(httpbin('/put'), array(), $data, $this->getOptions()); @@ -211,6 +247,22 @@ public function testPUTWithArray() { $this->assertEquals(array('test' => 'true', 'test2' => 'test'), $result['form']); } + /** + * @dataProvider streamProvider + */ + public function testPUTWithStream($stream, $expected_data, $expected_length) { + $request = Requests::put(httpbin('/put'), array(), $stream, $this->getOptions()); + fclose($stream); + $this->assertEquals(200, $request->status_code); + + $result = json_decode($request->body, true); + $this->assertEquals($expected_data, $result['data']); + + // Check the length we sent + $sent_headers = new Requests_Utility_CaseInsensitiveDictionary($result['headers']); + $this->assertEquals($expected_length, $sent_headers['Content-Length']); + } + public function testRawPATCH() { $data = 'test'; $request = Requests::patch(httpbin('/patch'), array(), $data, $this->getOptions()); From a1ec1b6e91c001dfcb877a04581a06e8a7f9e31a Mon Sep 17 00:00:00 2001 From: Ryan McCue Date: Mon, 26 Oct 2015 17:00:40 +1000 Subject: [PATCH 5/9] Add test for streams without stat --- tests/Transport/Base.php | 18 ++++++++++ tests/bootstrap.php | 77 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+) diff --git a/tests/Transport/Base.php b/tests/Transport/Base.php index 23b8c713d..da60d4eda 100755 --- a/tests/Transport/Base.php +++ b/tests/Transport/Base.php @@ -217,6 +217,24 @@ public function testPOSTWithStream($stream, $expected_data, $expected_length) { $this->assertEquals($expected_length, $sent_headers['Content-Length']); } + public function testPOSTWithInvalidStream() { + // Register our wrapper + stream_wrapper_register('requeststestvar', 'RequestsTest_VariableStream'); + + $data = base64_encode('hello'); + $stream = fopen('requeststestvar://', 'r+'); + stream_wrapper_unregister('requeststestvar'); + + fwrite($stream, 'Hello!'); + rewind($stream); + + // This should fail, as the stream doesn't support stat + $this->setExpectedException('Requests_Exception', 'Body stream resource does not support stat.'); + + $request = Requests::post(httpbin('/post'), array(), $stream, $this->getOptions()); + fclose($stream); + } + public function testRawPUT() { $data = 'test'; $request = Requests::put(httpbin('/put'), array(), $data, $this->getOptions()); diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 8cc8c0e19..14524e535 100755 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -153,3 +153,80 @@ public static function test() { return true; } } + +class RequestsTest_VariableStream { + protected $position; + protected $data; + + public function stream_open($path, $mode, $options, &$opened_path) { + $this->position = 0; + return true; + } + + public function stream_read($count) { + $ret = substr($this->data, $this->position, $count); + $this->position += strlen($ret); + return $ret; + } + + public function stream_write($data) { + $left = substr($this->data, 0, $this->position); + $right = substr($this->data, $this->position + strlen($data)); + $this->data = $left . $data . $right; + $this->position += strlen($data); + return strlen($data); + } + + public function stream_tell() { + return $this->position; + } + + public function stream_eof() { + return $this->position >= strlen($this->data); + } + + public function stream_seek($offset, $whence) { + switch ($whence) { + case SEEK_SET: + if ($offset < strlen($this->data) && $offset >= 0) { + $this->position = $offset; + return true; + } else { + return false; + } + break; + + case SEEK_CUR: + if ($offset >= 0) { + $this->position += $offset; + return true; + } else { + return false; + } + break; + + case SEEK_END: + if (strlen($this->data) + $offset >= 0) { + $this->position = strlen($this->data) + $offset; + return true; + } else { + return false; + } + break; + + default: + return false; + } + } + + /** + * Stat the file. + * + * Specifically *not* implemented, as we're using this for testing. + * + * @return bool + */ + public function stream_stat() { + return false; + } +} From edb7929ae16fd0d56f10095763dce5ce95ffc6db Mon Sep 17 00:00:00 2001 From: Ryan McCue Date: Mon, 26 Oct 2015 17:45:18 +1000 Subject: [PATCH 6/9] Ensure we set POSTFIELDS for POST requests --- library/Requests/Transport/cURL.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/Requests/Transport/cURL.php b/library/Requests/Transport/cURL.php index b5c278e34..0fb3d0006 100755 --- a/library/Requests/Transport/cURL.php +++ b/library/Requests/Transport/cURL.php @@ -297,7 +297,7 @@ protected function setup_handle($url, $headers, $data, $options) { if ($data_format === 'query') { $url = self::format_get($url, $data); - $data = ''; + curl_setopt($this->handle, CURLOPT_POSTFIELDS, ''); } elseif ($options['type'] !== Requests::TRACE) { if (is_resource($data)) { From 6fd3a89fbc4b87e597ba4d901c01e3f409a98a27 Mon Sep 17 00:00:00 2001 From: Ryan McCue Date: Mon, 26 Oct 2015 17:54:18 +1000 Subject: [PATCH 7/9] Don't set POSTFIELDS for non-POST requests, duh --- library/Requests/Transport/cURL.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/library/Requests/Transport/cURL.php b/library/Requests/Transport/cURL.php index 0fb3d0006..434c1a78e 100755 --- a/library/Requests/Transport/cURL.php +++ b/library/Requests/Transport/cURL.php @@ -297,7 +297,6 @@ protected function setup_handle($url, $headers, $data, $options) { if ($data_format === 'query') { $url = self::format_get($url, $data); - curl_setopt($this->handle, CURLOPT_POSTFIELDS, ''); } elseif ($options['type'] !== Requests::TRACE) { if (is_resource($data)) { @@ -327,6 +326,10 @@ protected function setup_handle($url, $headers, $data, $options) { switch ($options['type']) { case Requests::POST: curl_setopt($this->handle, CURLOPT_POST, true); + if ($data_format === 'query') { + // cURL needs a POST body, even if we didn't have one. + curl_setopt($this->handle, CURLOPT_POSTFIELDS, ''); + } break; case Requests::HEAD: From c283e3dbd49b22c7f3f09a4184ef2a2de4240491 Mon Sep 17 00:00:00 2001 From: Ryan McCue Date: Mon, 26 Oct 2015 22:28:29 +1000 Subject: [PATCH 8/9] Add extra tests for invalid streams --- tests/Transport/Base.php | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/Transport/Base.php b/tests/Transport/Base.php index da60d4eda..70e76e29d 100755 --- a/tests/Transport/Base.php +++ b/tests/Transport/Base.php @@ -235,6 +235,29 @@ public function testPOSTWithInvalidStream() { fclose($stream); } + public function testPOSTStreamInQuery() { + $stream = fopen('php://memory', 'r'); + $options = array( + // Attempt to use the stream for query data + 'data_format' => 'query', + ); + + // This should fail, as streams can't be used for query data + $this->setExpectedException('Requests_Exception', 'Streams can only be sent in the request body.'); + + $request = Requests::post(httpbin('/post'), array(), $stream, $this->getOptions($options)); + } + + public function testPOSTWithInvalidResource() { + // Use a socket stream instead of a file + $stream = socket_create(AF_UNIX, SOCK_STREAM, 0); + + // This should fail, as the resource isn't a stream + $this->setExpectedException('Requests_Exception', 'Invalid stream resource for request body.'); + + $request = Requests::post(httpbin('/post'), array(), $stream, $this->getOptions()); + } + public function testRawPUT() { $data = 'test'; $request = Requests::put(httpbin('/put'), array(), $data, $this->getOptions()); From fa39ad06901955f9f8ee01c0f666079a417eab3b Mon Sep 17 00:00:00 2001 From: Ryan McCue Date: Sun, 31 Jan 2016 17:27:06 +0000 Subject: [PATCH 9/9] Add workaround for HHVM bug --- tests/Transport/Base.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/Transport/Base.php b/tests/Transport/Base.php index 9b82be985..d5eaa59fe 100755 --- a/tests/Transport/Base.php +++ b/tests/Transport/Base.php @@ -255,6 +255,11 @@ public function testPOSTWithInvalidResource() { // This should fail, as the resource isn't a stream $this->setExpectedException('Requests_Exception', 'Invalid stream resource for request body.'); + // https://github.com/facebook/hhvm/issues/4036 + if (defined('HHVM_VERSION')) { + $this->setExpectedException('Requests_Exception', 'Body stream resource does not support stat.'); + } + $request = Requests::post(httpbin('/post'), array(), $stream, $this->getOptions()); }