From c048042e79b0de8fd9992c0c3eeb119937219e8c Mon Sep 17 00:00:00 2001 From: Matt Butcher Date: Mon, 23 Jan 2012 18:38:28 -0600 Subject: [PATCH] Fixed a bug in POST handling. POST data was not being sent appropriately. Also made it possible to send configuration params into CURL from the config. --- src/HPCloud/Bootstrap.php | 16 +++++ src/HPCloud/Transport/CURLTransport.php | 78 +++++++++++++++++-------- test/Tests/RemoteObjectTest.php | 6 +- test/Tests/StreamWrapperTest.php | 4 ++ 4 files changed, 78 insertions(+), 26 deletions(-) diff --git a/src/HPCloud/Bootstrap.php b/src/HPCloud/Bootstrap.php index b4e5c83..d932b67 100644 --- a/src/HPCloud/Bootstrap.php +++ b/src/HPCloud/Bootstrap.php @@ -87,6 +87,18 @@ class Bootstrap { * All of the HPCloud classes share the same configuration. This * ensures that a stable runtime environment is maintained. * + * Common configuration directives: + * + * - 'transport': The namespaced classname for the transport that + * should be used. Example: `\HPCloud\Transport\CURLTransport` + * - 'transport.debug': The integer 1 for enabling debug, 0 for + * disabling. Enabling will turn on verbose debugging output + * for any transport that supports it. + * - 'transport.timeout': An integer value indicating how long + * the transport layer should wait for an HTTP request. A + * transport MAY ignore this parameter, but the ones included + * with the library honor it. + * * @param array $array * An associative array of configuration directives. */ @@ -167,4 +179,8 @@ class Bootstrap { // Otherwise, just return the default value. return $default; } + + public static function hasConfig($name) { + return isset(self::$config[$name]); + } } diff --git a/src/HPCloud/Transport/CURLTransport.php b/src/HPCloud/Transport/CURLTransport.php index 2fe97f4..a0a561c 100644 --- a/src/HPCloud/Transport/CURLTransport.php +++ b/src/HPCloud/Transport/CURLTransport.php @@ -6,6 +6,8 @@ namespace HPCloud\Transport; +use \HPCloud\Bootstrap; + /** * Provide HTTP transport with CURL. * @@ -33,16 +35,25 @@ class CURLTransport implements Transporter { const HTTP_USER_AGENT_SUFFIX = ' (c93c0a) CURL/1.0'; - public function doRequest($uri, $method = 'GET', $headers = array(), $body = '') { + public function doRequest($uri, $method = 'GET', $headers = array(), $body = NULL) { $in = NULL; if (!empty($body)) { - // First we turn our body into a temp-backed buffer. - $in = fopen('php://temp', 'wr', FALSE); - fwrite($in, $body, strlen($body)); - rewind($in); + // For whatever reason, CURL seems to want POST request data to be + // a string, not a file handle. So we adjust. PUT, on the other hand, + // needs to be in a file handle. + if ($method == 'POST') { + $in = $body; + } + else { + // First we turn our body into a temp-backed buffer. + $in = fopen('php://temp', 'wr', FALSE); + fwrite($in, $body, strlen($body)); + rewind($in); + } } return $this->handleDoRequest($uri, $method, $headers, $in); + //return $this->handleDoRequest($uri, $method, $headers, $body); } @@ -51,9 +62,17 @@ class CURLTransport implements Transporter { $in = open($resource, 'rb', FALSE); } else { - $in = $resource; + // FIXME: Is there a better way? + // There is a bug(?) in CURL which prevents it + // from writing the same stream twice. But we + // need to be able to flush a file multiple times. + // So we have to create a new temp buffer for each + // write operation. + $in = fopen('php://temp', 'rb+'); //tmpfile(); + stream_copy_to_stream($resource, $in); + rewind($in); } - return $this->handleDoRequest($uri, $method, $headers, $resource); + return $this->handleDoRequest($uri, $method, $headers, $in); } /** @@ -77,18 +96,19 @@ class CURLTransport implements Transporter { // Set the upload $copy = NULL; - if (!empty($in)) { - // FIXME: Is there a better way? - // There is a bug(?) in CURL which prevents it - // from writing the same stream twice. But we - // need to be able to flush a file multiple times. - // So we have to create a new temp buffer for each - // write operation. - $copy = fopen('php://temp', 'rb+'); //tmpfile(); - stream_copy_to_stream($in, $copy); - rewind($copy); - curl_setopt($curl, CURLOPT_INFILE, $copy); + // If we get a string, we send the string + // data. + if (is_string($in)) { + curl_setopt($curl, CURLOPT_POSTFIELDS, $in); + if (!isset($headers['Content-Length'])) { + $headers['Content-Length'] = strlen($in); + } + } + // If we get a resource, we treat it like a stream + // and pass it into CURL as a file. + elseif (is_resource($in)) { + curl_setopt($curl, CURLOPT_INFILE, $in); // Tell CURL about the content length if we know it. if (!empty($headers['Content-Length'])) { @@ -97,7 +117,7 @@ class CURLTransport implements Transporter { } } - // Set headers + // Set headers. $this->setHeaders($curl, $headers); // Get the output. @@ -114,9 +134,6 @@ class CURLTransport implements Transporter { // Timeout if the remote has not connected in 30 sec. //CURLOPT_CONNECTTIMEOUT => 30, - // Max time to allow CURL to do the transaction. - // CURLOPT_TIMEOUT => 120, - // If this is set, CURL will auto-deflate any encoding it can. // CURLOPT_ENCODING => '', @@ -127,11 +144,24 @@ class CURLTransport implements Transporter { // Limit curl to only these protos. // CURLOPT_PROTOCOLS => CURLPROTO_HTTP | CURLPROTO_HTTPS, - // If you are desparate and need to debug, uncomment this. - //CURLOPT_VERBOSE => 1, ); curl_setopt_array($curl, $opts); + if (Bootstrap::hasConfig('transport.debug')) { + $debug = Bootstrap::config('transport.debug', NULL); + curl_setopt($curl, CURLOPT_VERBOSE, (int) $debug); + } + + if (Bootstrap::hasConfig('transport.timeout')) { + curl_setopt($curl, CURLOPT_TIMEOUT, (int) Bootstrap::config('transport.timeout')); + } + + if (Bootstrap::hasConfig('transport.ssl.verify')) { + $validate = (boolean) Bootstrap::config('transport.ssl.verify', TRUE); + curl_setopt($curl, CURLOPT_SSL_VERIFYPEER, $validate); + } + + $ret = curl_exec($curl); $info = curl_getinfo($curl); $status = $info['http_code']; diff --git a/test/Tests/RemoteObjectTest.php b/test/Tests/RemoteObjectTest.php index 3459859..3ff2114 100644 --- a/test/Tests/RemoteObjectTest.php +++ b/test/Tests/RemoteObjectTest.php @@ -150,7 +150,8 @@ class RemoteObjectTest extends \HPCloud\Tests\TestCase { // This will be HTTP if we are using the PHP stream // wrapper, but for CURL this will be PHP. - if (self::$settings['transport'] == '\HPCloud\Transport\PHPStreamTransport') { + $klass = \HPCloud\Bootstrap::config('transport', NULL); + if ($klass == '\HPCloud\Transport\PHPStreamTransport') { $expect = 'http'; } else { @@ -224,7 +225,8 @@ class RemoteObjectTest extends \HPCloud\Tests\TestCase { // The CURL, though, backs its up with a temp file wrapped in a PHP // stream. Other backends are likely to do the same. So this test // is weakened for CURL backends. - if (self::$settings['transport'] == '\HPCloud\Transport\PHPStreamTransport') { + $transport = \HPCloud\Bootstrap::config('transport'); + if ($transport == '\HPCloud\Transport\PHPStreamTransport') { $expect = 'http'; } else { diff --git a/test/Tests/StreamWrapperTest.php b/test/Tests/StreamWrapperTest.php index d24bf3b..101c26a 100644 --- a/test/Tests/StreamWrapperTest.php +++ b/test/Tests/StreamWrapperTest.php @@ -45,6 +45,10 @@ class StreamWrapperTest extends \HPCloud\Tests\TestCase { $scheme = StreamWrapper::DEFAULT_SCHEME; } + if (empty(self::$ostore)) { + throw new \Exception('OStore is gone.'); + } + $params = $add + array( 'token' => self::$ostore->token(), 'swift_endpoint' => self::$ostore->url(),