From 5e48c2592d0e2d4d9278514c295baaf74fd996e6 Mon Sep 17 00:00:00 2001 From: blade Date: Thu, 7 May 2015 11:14:58 +0200 Subject: [PATCH 1/5] Added ErrorInfo, refactored thrown exceptions to match it Better exception handling in tests (use error code instead of http code) Test for invalid message types --- composer.json | 1 - src/AblyRest.php | 4 +- src/Auth.php | 14 +++---- src/Exceptions/AblyException.php | 18 ++------- src/Exceptions/AblyRequestException.php | 4 +- src/Exceptions/ErrorInfo.php | 30 ++++++++++++++ src/Http.php | 6 +-- src/Models/BaseMessage.php | 12 +++--- src/Models/PaginatedResult.php | 2 +- src/Models/Stats.php | 2 +- tests/AblyRestTest.php | 16 ++++---- tests/AuthTest.php | 4 +- tests/ChannelMessagesTest.php | 54 ++++++++++++++++++++++++- tests/TokenTest.php | 28 ++++--------- tests/factories/TestApp.php | 12 +++--- 15 files changed, 133 insertions(+), 74 deletions(-) create mode 100644 src/Exceptions/ErrorInfo.php diff --git a/composer.json b/composer.json index 8f11f66..3a4cee7 100644 --- a/composer.json +++ b/composer.json @@ -2,7 +2,6 @@ "name": "ably/ably-php", "description": "Ably REST client library for PHP.", "keywords": ["messaging", "messages", "ably"], - "version": "1.0.1", "homepage": "https://www.ably.io/", "require": { "php": ">=5.4", diff --git a/src/AblyRest.php b/src/AblyRest.php index c9ba99d..72e004c 100644 --- a/src/AblyRest.php +++ b/src/AblyRest.php @@ -131,7 +131,7 @@ public function request( $method, $path, $headers = array(), $params = array(), $causedByExpiredToken = $auth && !$this->auth->isUsingBasicAuth() - && $e->getAblyCode() == 40140; + && $e->code == 40140; if ( $causedByExpiredToken ) { // renew the token $this->auth->authorise( array(), array(), $force = true ); @@ -167,7 +167,7 @@ protected function requestWithFallback( $method, $path, $headers = array(), $par return $this->http->request( $method, $server . $path, $headers, $params ); } catch (AblyRequestException $e) { - if ( $e->getAblyCode() >= 50000 ) { + if ( $e->code >= 50000 ) { if ( $attempt < count( $this->options->fallbackHosts ) ) { return $this->requestWithFallback( $method, $path, $headers, $params, $attempt + 1); } else { diff --git a/src/Auth.php b/src/Auth.php index f7728a3..4b63cbf 100644 --- a/src/Auth.php +++ b/src/Auth.php @@ -29,7 +29,7 @@ public function __construct( AblyRest $ably, ClientOptions $options ) { if ( !$options->tls ) { log::e( 'Auth: trying to use basic key auth over insecure connection' ); - throw new AblyException ( 'Trying to use basic key auth over insecure connection', 401, 40103 ); + throw new AblyException ( 'Trying to use basic key auth over insecure connection', 40103, 401 ); } return; } @@ -46,7 +46,7 @@ public function __construct( AblyRest $ably, ClientOptions $options ) { Log::d( 'Auth: using token auth with supplied token only' ); } else { Log::e( 'Auth: no authentication parameters supplied' ); - throw new AblyException ( 'No authentication parameters supplied', 401, 40103 ); + throw new AblyException ( 'No authentication parameters supplied', 40103, 401 ); } $this->tokenDetails = $this->authOptions->tokenDetails; @@ -93,7 +93,7 @@ public function getAuthHeaders() { $this->authorise(); $header = array( 'authorization: Bearer '. base64_encode( $this->tokenDetails->token ) ); } else { - throw new AblyException( 'Unable to provide auth headers. No auth parameters defined.', 401, 40101 ); + throw new AblyException( 'Unable to provide auth headers. No auth parameters defined.', 40101, 401 ); } return $header; } @@ -173,7 +173,7 @@ public function requestToken( $authOptions = array(), $tokenParams = array() ) { $signedTokenRequest = $this->createTokenRequest( $authOptions->toArray(), $tokenParams->toArray() ); } else { Log::e( 'Auth::requestToken:', 'Unable to request a Token, auth options don\'t provide means to do so' ); - throw new AblyException( 'Unable to request a Token, auth options don\'t provide means to do so', 401, 40101 ); + throw new AblyException( 'Unable to request a Token, auth options don\'t provide means to do so', 40101, 401 ); } // do the request @@ -193,7 +193,7 @@ public function requestToken( $authOptions = array(), $tokenParams = array() ) { ); if ( empty( $res->token ) ) { // just in case.. an AblyRequestException should be thrown on the previous step with a 4XX error code on failure - throw new AblyException( 'Failed to get a token', 401, 40100 ); + throw new AblyException( 'Failed to get a token', 40100, 401 ); } return new TokenDetails( $res ); @@ -213,7 +213,7 @@ public function createTokenRequest( $authOptions = array(), $tokenParams = array if ( count( $keyParts ) != 2 ) { Log::e( 'Auth::createTokenRequest', "Can't create signed token request, invalid key specified" ); - throw new AblyException( 'Invalid key specified', 401, 40101 ); + throw new AblyException( 'Invalid key specified', 40101, 401 ); } $keyName = $keyParts[0]; @@ -222,7 +222,7 @@ public function createTokenRequest( $authOptions = array(), $tokenParams = array $tokenRequest = new TokenRequest( $tokenParams ); if ( !empty( $tokenRequest->keyName ) && $tokenRequest->keyName != $keyName ) { - throw new AblyException( 'Incompatible keys specified', 401, 40102 ); + throw new AblyException( 'Incompatible keys specified', 40102, 401 ); } else { $tokenRequest->keyName = $keyName; } diff --git a/src/Exceptions/AblyException.php b/src/Exceptions/AblyException.php index 35fcca0..04a7e91 100644 --- a/src/Exceptions/AblyException.php +++ b/src/Exceptions/AblyException.php @@ -1,22 +1,12 @@ ablyCode = $ablyCode; - } +class AblyException extends ErrorInfo { - public function getAblyCode() { - return $this->ablyCode; + public function __construct($message, $code = 40000, $statusCode = 400 ) { + parent::__construct($message, $code, $statusCode); } } diff --git a/src/Exceptions/AblyRequestException.php b/src/Exceptions/AblyRequestException.php index 0802ef2..2c88c2b 100644 --- a/src/Exceptions/AblyRequestException.php +++ b/src/Exceptions/AblyRequestException.php @@ -8,8 +8,8 @@ class AblyRequestException extends AblyException { protected $response; - public function __construct( $message, $code, $ablyCode, $response = null ) { - parent::__construct( $message, $code, $ablyCode ); + public function __construct( $message, $code, $statusCode, $response = null ) { + parent::__construct( $message, $code, $statusCode ); $this->response = $response ? : array( 'headers' => '', 'body' => '' ); } diff --git a/src/Exceptions/ErrorInfo.php b/src/Exceptions/ErrorInfo.php new file mode 100644 index 0000000..855b974 --- /dev/null +++ b/src/Exceptions/ErrorInfo.php @@ -0,0 +1,30 @@ +$name; + } + + + public function __construct( $message, $code, $statusCode ) { + parent::__construct( $message, $code ); + + $this->statusCode = $statusCode; + } + + public function getStatusCode() { + return $this->statusCode; + } +} diff --git a/src/Http.php b/src/Http.php index 4124c88..59add15 100644 --- a/src/Http.php +++ b/src/Http.php @@ -146,7 +146,7 @@ public function request( $method, $url, $headers = array(), $params = array() ) if ($err) { // a connection error has occured (no data received) Log::e('cURL error:', $err, $errmsg); - throw new AblyRequestException( 'cURL error: ' . $errmsg, 500, 50003 ); + throw new AblyRequestException( 'cURL error: ' . $errmsg, 50003, 500 ); } $response = null; @@ -160,10 +160,10 @@ public function request( $method, $url, $headers = array(), $params = array() ) Log::v( 'cURL request response:', $info['http_code'], $response ); if ( !in_array( $info['http_code'], array(200,201) ) ) { - $ablyCode = empty( $decodedBody->error->code ) ? null : $decodedBody->error->code; + $ablyCode = empty( $decodedBody->error->code ) ? $info['http_code'] * 100 : $decodedBody->error->code * 1; $errorMessage = empty( $decodedBody->error->message ) ? 'cURL request failed' : $decodedBody->error->message; - throw new AblyRequestException( $errorMessage, $info['http_code'], $ablyCode, $response ); + throw new AblyRequestException( $errorMessage, $ablyCode, $info['http_code'], $response ); } return $response; diff --git a/src/Models/BaseMessage.php b/src/Models/BaseMessage.php index 00748bf..79649e6 100644 --- a/src/Models/BaseMessage.php +++ b/src/Models/BaseMessage.php @@ -75,7 +75,7 @@ public function fromJSON( $json, $keepOriginal = false ) { } else { $obj = @json_decode($json); if (!$obj) { - throw new AblyException( 'Invalid object or JSON encoded object', 400, 40000 ); + throw new AblyException( 'Invalid object or JSON encoded object' ); } } @@ -122,7 +122,7 @@ protected function encode() { $type = 'utf-8'; $msg->data = $this->data; } else { - throw new AblyException( 'Message data must be either, string, string with binary data, or JSON-encodable array or object.' ); + throw new AblyException( 'Message data must be either, string, string with binary data, or JSON-encodable array or object.', 40003, 400 ); } if ($this->cipherParams) { @@ -152,23 +152,23 @@ protected function decode() { $this->data = base64_decode( $this->data ); if ($this->data === false) { - throw new AblyException( 'Could not base64-decode message data', 400, 40000 ); + throw new AblyException( 'Could not base64-decode message data' ); } } else if ($encoding == 'json') { $this->data = json_decode( $this->data ); if ($this->data === null) { - throw new AblyException( 'Could not JSON-decode message data', 400, 40000 ); + throw new AblyException( 'Could not JSON-decode message data' ); } } else if (strpos( $encoding, 'cipher+' ) === 0) { if (!$this->cipherParams) { - throw new AblyEncryptionException( 'Could not decrypt message data, no cipherParams provided', 400, 40000 ); + throw new AblyEncryptionException( 'Could not decrypt message data, no cipherParams provided' ); } $this->data = Crypto::decrypt( $this->data, $this->cipherParams ); if ($this->data === false) { - throw new AblyEncryptionException( 'Could not decrypt message data', 400, 40000 ); + throw new AblyEncryptionException( 'Could not decrypt message data' ); } } } diff --git a/src/Models/PaginatedResult.php b/src/Models/PaginatedResult.php index ebeb83d..79f702b 100644 --- a/src/Models/PaginatedResult.php +++ b/src/Models/PaginatedResult.php @@ -153,7 +153,7 @@ private function parseHeaders($headers) { $rel = $m[2]; if (substr($link, 0, 2) != './') { - throw new AblyException( "Server error - only relative URLs are supported in pagination", 400, 40000 ); + throw new AblyException( "Server error - only relative URLs are supported in pagination" ); } $this->paginationHeaders[$rel] = $path.substr($link, 2); diff --git a/src/Models/Stats.php b/src/Models/Stats.php index 525c469..981ea0e 100644 --- a/src/Models/Stats.php +++ b/src/Models/Stats.php @@ -41,7 +41,7 @@ public function fromJSON( $json ) { } else { $obj = @json_decode($json); if (!$obj) { - throw new AblyException( 'Invalid object or JSON encoded object', 400, 40000 ); + throw new AblyException( 'Invalid object or JSON encoded object' ); } } diff --git a/tests/AblyRestTest.php b/tests/AblyRestTest.php index 8e1fa5f..c23fa09 100644 --- a/tests/AblyRestTest.php +++ b/tests/AblyRestTest.php @@ -174,8 +174,8 @@ public function testFallbackHosts400() { ); $ably = new AblyRest( $opts ); - $ably->http->errorCode = 401; - $ably->http->ablyErrorCode = 40101; // auth error + $ably->http->httpErrorCode = 401; + $ably->http->errorCode = 40101; // auth error try { $ably->time(); // make a request @@ -263,7 +263,7 @@ public function testHttpTimeout() { )); $ably->http->get('https://cdn.ably.io/lib/ably.js'); // should work - $this->setExpectedException('Ably\Exceptions\AblyRequestException', '', 500); + $this->setExpectedException('Ably\Exceptions\AblyRequestException', '', 50003); $ablyTimeout->http->get('https://cdn.ably.io/lib/ably.js'); // guaranteed to take more than 50 ms } } @@ -287,8 +287,8 @@ public function request($method, $url, $headers = array(), $params = array()) { class HttpMockInitTestTimeout extends Http { public $failedHosts = array(); public $failAttempts = 100; // number of attempts to time out before starting to return data - public $errorCode = 500; - public $ablyErrorCode = 50003; // timeout + public $httpErrorCode = 500; + public $errorCode = 50003; // timeout public function request($method, $url, $headers = array(), $params = array()) { @@ -298,12 +298,12 @@ public function request($method, $url, $headers = array(), $params = array()) { $this->failAttempts--; - throw new AblyRequestException( 'Fake error', $this->errorCode, $this->ablyErrorCode ); + throw new AblyRequestException( 'Fake error', $this->errorCode, $this->httpErrorCode ); } return array( - 'headers' => '', - 'body' => array( 999999, 0 ) + 'headers' => 'HTTP/1.1 200 OK'."\n", + 'body' => array( 999999, 0 ), ); } } \ No newline at end of file diff --git a/tests/AuthTest.php b/tests/AuthTest.php index f8ac7c8..6899830 100644 --- a/tests/AuthTest.php +++ b/tests/AuthTest.php @@ -26,7 +26,7 @@ public static function tearDownAfterClass() { * Init library with a key over unsecure connection */ public function testAuthWithKeyInsecure() { - $this->setExpectedException( 'Ably\Exceptions\AblyException', '', 401 ); + $this->setExpectedException( 'Ably\Exceptions\AblyException', '', 40103 ); $ably = new AblyRest( array_merge( self::$defaultOptions, array( 'key' => self::$testApp->getAppKeyDefault()->string, @@ -38,7 +38,7 @@ public function testAuthWithKeyInsecure() { * Init library without any valid auth */ public function testNoAuthParams() { - $this->setExpectedException( 'Ably\Exceptions\AblyException', '', 401 ); + $this->setExpectedException( 'Ably\Exceptions\AblyException', '', 40103 ); $ably = new AblyRest( ); } diff --git a/tests/ChannelMessagesTest.php b/tests/ChannelMessagesTest.php index a68a22c..3cc1eec 100644 --- a/tests/ChannelMessagesTest.php +++ b/tests/ChannelMessagesTest.php @@ -2,6 +2,7 @@ namespace tests; use Ably\AblyRest; use Ably\Channel; +use Ably\Http; use Ably\Models\CipherParams; use Ably\Models\Message; @@ -145,6 +146,43 @@ public function testPublishSingleMessageUnencrypted() { $this->assertEquals( $data, $messages->items[0]->data, 'Expected message contents to match' ); } + /** + * Verify that publishing invalid types fails + */ + public function testInvalidTypes() { + $channel = self::$ably->channel( 'persisted:unencryptedSingle' ); + + $msg = new Message(); + $msg->name = 'int'; + $msg->data = 81403; + + $this->setExpectedException( 'Ably\Exceptions\AblyException', '', 40003 ); + $channel->publish( $msg ); + + $msg = new Message(); + $msg->name = 'bool'; + $msg->data = true; + + $this->setExpectedException( 'Ably\Exceptions\AblyException', '', 40003 ); + $channel->publish( $msg ); + + $msg = new Message(); + $msg->name = 'float'; + $msg->data = 42.23; + + $this->setExpectedException( 'Ably\Exceptions\AblyException', '', 40003 ); + $channel->publish( $msg ); + + $msg = new Message(); + $msg->name = 'function'; + $msg->data = function($param) { + return "mock function"; + }; + + $this->setExpectedException( 'Ably\Exceptions\AblyException', '', 40003 ); + $channel->publish( $msg ); + } + /** * Encryption mismatch - publish message over encrypted channel, retrieve history over unencrypted channel * @@ -194,5 +232,19 @@ public function testEncryptionKeyMismatch() { $encrypted2 = self::$ably->channel( 'persisted:mismatch3', $options2 ); $messages = $encrypted2->history(); } +} + + +class HttpMockMsgCounter extends Http { + public $requests = 0; + + public function request($method, $url, $headers = array(), $params = array()) { -} \ No newline at end of file + $this->requests++; + + return array( + 'headers' => 'HTTP/1.1 200 OK'."\n", + 'body' => array(), + ); + } +} diff --git a/tests/TokenTest.php b/tests/TokenTest.php index 9028826..a4841dd 100644 --- a/tests/TokenTest.php +++ b/tests/TokenTest.php @@ -73,12 +73,8 @@ public function testRequestTokenWithExplicitInvalidTimestamp() { $params = array_merge(self::$tokenParams, array( 'timestamp' => $requestTime - 30 * 60 * 1000 )); - try { - self::$ably->auth->requestToken( array(), $params ); - $this->fail('Expected token request rejection'); - } catch (AblyException $e) { - $this->assertEquals( 401, $e->getCode(), 'Unexpected error code' ); - } + $this->setExpectedException( 'Ably\Exceptions\AblyException', 'Timestamp not current', 40101 ); + self::$ably->auth->requestToken( array(), $params ); } /** @@ -174,12 +170,8 @@ public function testTokenGenerationWithDefaultTTL() { */ public function testTokenGenerationWithExcessiveTTL() { $tokenParams = array( 'ttl' => 365*24*60*60*1000 ); - try { - self::$ably->auth->requestToken( array(), $tokenParams ); - $this->fail( 'Expected token request rejection' ); - } catch (AblyException $e) { - $this->assertEquals( 40003, $e->getAblyCode(), 'Unexpected error code' ); - } + $this->setExpectedException( 'Ably\Exceptions\AblyException', '', 40003 ); + self::$ably->auth->requestToken( array(), $tokenParams ); } /** @@ -187,12 +179,8 @@ public function testTokenGenerationWithExcessiveTTL() { */ public function testTokenGenerationWithInvalidTTL() { $tokenParams = array( 'ttl' => -1 * 1000 ); - try { - self::$ably->auth->requestToken( array(), $tokenParams ); - $this->fail( 'Expected token request rejection' ); - } catch (AblyException $e) { - $this->assertEquals( 40003, $e->getAblyCode(), 'Unexpected error code' ); - } + $this->setExpectedException( 'Ably\Exceptions\AblyException', '', 40003 ); + self::$ably->auth->requestToken( array(), $tokenParams ); } /** @@ -252,7 +240,7 @@ public function testFailingTokenRenewalKnownExpiration() { sleep(2); - $this->setExpectedException( 'Ably\Exceptions\AblyException', '', 401 ); + $this->setExpectedException( 'Ably\Exceptions\AblyException', '', 40101 ); $channel->publish( 'test', 'test' ); // this should fail } @@ -313,7 +301,7 @@ public function testFailingTokenRenewalUnknownExpiration() { sleep(2); - $this->setExpectedException( 'Ably\Exceptions\AblyException', '', 401 ); + $this->setExpectedException( 'Ably\Exceptions\AblyException', '', 40101 ); $channel->publish( 'test', 'test' ); // this should fail } } \ No newline at end of file diff --git a/tests/factories/TestApp.php b/tests/factories/TestApp.php index c34f7b8..fafe2a0 100644 --- a/tests/factories/TestApp.php +++ b/tests/factories/TestApp.php @@ -2,6 +2,7 @@ namespace tests; use Ably\AblyRest; use Ably\Log; +use Ably\Models\ClientOptions; use \stdClass; require_once __DIR__ . '/../../vendor/autoload.php'; @@ -24,16 +25,15 @@ public function __construct() { $settings = array(); - $settings['tls'] = true; - $settings['host'] = "staging-rest.ably.io"; - //$settings['host'] = "sandbox-rest.ably.io"; - //$settings['host'] = "rest.ably.io"; + $settings['environment'] = getenv( 'ABLY_ENV' ) ? : 'sandbox'; //$settings['logLevel'] = Log::DEBUG; + $clientOpts = new ClientOptions( $settings ); + $this->options = $settings; - $scheme = 'http' . ($settings['tls'] ? 's' : ''); - $this->server = $scheme .'://'. $settings['host']; + $scheme = 'http' . ($clientOpts->tls ? 's' : ''); + $this->server = $scheme .'://'. $clientOpts->host; $this->init(); return $this; From 2e6e21977bd31a7b1ef17f2af9d6152acf1cd810 Mon Sep 17 00:00:00 2001 From: blade Date: Thu, 7 May 2015 12:39:51 +0200 Subject: [PATCH 2/5] Support for partial decoding when decryption fails Default limit and order test for message history Tests for messages - decryption with mismatched params, request count on publishing an array of msgs, too large message Removed no longer necessary AblyEncryptionException --- src/Exceptions/AblyEncryptionException.php | 8 -- src/Models/BaseMessage.php | 22 +++-- tests/ChannelHistoryTest.php | 25 ++++++ tests/ChannelMessagesTest.php | 96 ++++++++++++++++++---- 4 files changed, 122 insertions(+), 29 deletions(-) delete mode 100644 src/Exceptions/AblyEncryptionException.php diff --git a/src/Exceptions/AblyEncryptionException.php b/src/Exceptions/AblyEncryptionException.php deleted file mode 100644 index 6863bcb..0000000 --- a/src/Exceptions/AblyEncryptionException.php +++ /dev/null @@ -1,8 +0,0 @@ -originalData = $this->data; @@ -154,26 +153,35 @@ protected function decode() { if ($this->data === false) { throw new AblyException( 'Could not base64-decode message data' ); } + + array_pop( $encodings ); } else if ($encoding == 'json') { $this->data = json_decode( $this->data ); if ($this->data === null) { throw new AblyException( 'Could not JSON-decode message data' ); } + + array_pop( $encodings ); } else if (strpos( $encoding, 'cipher+' ) === 0) { if (!$this->cipherParams) { - throw new AblyEncryptionException( 'Could not decrypt message data, no cipherParams provided' ); + Log::e( 'Could not decrypt message data, no cipherParams provided' ); + break; } - $this->data = Crypto::decrypt( $this->data, $this->cipherParams ); + $data = Crypto::decrypt( $this->data, $this->cipherParams ); - if ($this->data === false) { - throw new AblyEncryptionException( 'Could not decrypt message data' ); + if ($data === false) { + Log::e( 'Could not decrypt message data' ); + break; } + + $this->data = $data; + array_pop( $encodings ); } } - $this->encoding = null; + $this->encoding = count( $encodings ) ? implode( '/', $encodings ) : null; } } diff --git a/tests/ChannelHistoryTest.php b/tests/ChannelHistoryTest.php index d4e1b9a..be9c13c 100644 --- a/tests/ChannelHistoryTest.php +++ b/tests/ChannelHistoryTest.php @@ -82,6 +82,31 @@ public function testPublishEventsAndCheckOrderBackwards() { $this->assertEquals( $expected_message_history, $actual_message_history, 'Expect messages in backward order'); } + /** + * Test default limit (100 messages) and default order (backwards) + */ + public function testDefaults() { + $channel = self::$ably->channel('persisted:history_def_limit'); + + $msgsToSend = array(); + for ( $i = 0; $i < 101; $i++ ) { + $msg = new Message(); + $msg->data = ''.$i; + $msgsToSend[] = $msg; + } + $channel->publish( $msgsToSend ); + + $messages = $channel->history(); + $this->assertEquals( 100, count( $messages->items ), 'Expected 100 messages' ); + + # verify message order + $actual_message_history = array(); + foreach ($messages->items as $msg) { + array_push( $actual_message_history, $msg->data ); + } + $expected_message_history = range(100, 1, -1); + $this->assertEquals( $expected_message_history, $actual_message_history, 'Expect messages in backward order'); + } /** * Publish events, get limited history, check expected order (forwards) and pagination diff --git a/tests/ChannelMessagesTest.php b/tests/ChannelMessagesTest.php index 3cc1eec..cd33bda 100644 --- a/tests/ChannelMessagesTest.php +++ b/tests/ChannelMessagesTest.php @@ -3,6 +3,7 @@ use Ably\AblyRest; use Ably\Channel; use Ably\Http; +use Ably\Log; use Ably\Models\CipherParams; use Ably\Models\Message; @@ -146,11 +147,35 @@ public function testPublishSingleMessageUnencrypted() { $this->assertEquals( $data, $messages->items[0]->data, 'Expected message contents to match' ); } + /** + * Verify that batch sending messages actually makes just one request + */ + public function testMessageArraySingleRequest() { + $messages = array(); + + for ( $i = 0; $i < 10; $i++ ) { + $msg = new Message(); + $msg->name = 'msg'.$i; + $msg->data = 'test string'.$i; + $messages[] = $msg; + } + + $ably = new AblyRest( array_merge( self::$defaultOptions, array( + 'key' => self::$testApp->getAppKeyDefault()->string, + 'httpClass' => 'tests\HttpMockMsgCounter', + ) ) ); + + $channel = $ably->channel( 'singleReq' ); + $channel->publish( $messages ); + + $this->assertEquals( 1, $ably->http->requestCount, 'Expected 1 request to be made' ); + } + /** * Verify that publishing invalid types fails */ public function testInvalidTypes() { - $channel = self::$ably->channel( 'persisted:unencryptedSingle' ); + $channel = self::$ably->channel( 'invalidTypes' ); $msg = new Message(); $msg->name = 'int'; @@ -183,20 +208,49 @@ public function testInvalidTypes() { $channel->publish( $msg ); } + /** + * Verify that publishing too large message (>128KB) fails + */ + public function testTooLargeMessage() { + $channel = self::$ably->channel( 'huge' ); + + $msg = new Message(); + $msg->name = 'huge'; + $msg->data = str_repeat("~", 128 * 1024); // 128 kilobytes + message JSON + + $this->setExpectedException( 'Ably\Exceptions\AblyException', '', 40009 ); + $channel->publish( $msg ); + } + /** * Encryption mismatch - publish message over encrypted channel, retrieve history over unencrypted channel - * - * @expectedException Ably\Exceptions\AblyEncryptionException */ public function testEncryptedMessageUnencryptedHistory() { + $errorLogged = false; + + $ably = new AblyRest( array_merge( self::$defaultOptions, array( + 'key' => self::$testApp->getAppKeyDefault()->string, + 'logHandler' => function( $level, $args ) use ( &$errorLogged ) { + if ( $level == Log::ERROR ) { + $errorLogged = true; + } + }, + ) ) ); + $payload = 'This is a test message'; $options = array( 'encrypted' => true, 'cipherParams' => new CipherParams( 'password', 'aes-128-cbc' )); - $encrypted = self::$ably->channel( 'persisted:mismatch1', $options ); - $encrypted->publish( 'test', $payload ); + $encrypted1 = $ably->channel( 'persisted:mismatch1', $options ); + $encrypted1->publish( 'test', $payload ); - $unencrypted = self::$ably->channel( 'persisted:mismatch1', array() ); - $messages = $unencrypted->history(); + $options2 = array(); + $encrypted2 = $ably->channel( 'persisted:mismatch1', $options2 ); + $messages = $encrypted2->history(); + $msg = $messages->items[0]; + + $this->assertTrue( $errorLogged, 'Expected an error to be logged' ); + $this->assertEquals( 'utf-8/cipher+aes-128-cbc/base64', $msg->originalEncoding, 'Expected the original message to be encrypted + base64 encoded' ); + $this->assertEquals( 'utf-8/cipher+aes-128-cbc', $msg->encoding, 'Expected to receive the message still encrypted, but base64 decoded' ); } /** @@ -205,10 +259,10 @@ public function testEncryptedMessageUnencryptedHistory() { public function testUnencryptedMessageEncryptedHistory() { $payload = 'This is a test message'; - $options = array( 'encrypted' => true, 'cipherParams' => new CipherParams( 'password', 'aes-128-cbc' )); $encrypted = self::$ably->channel( 'persisted:mismatch2' ); $encrypted->publish( 'test', $payload ); + $options = array( 'encrypted' => true, 'cipherParams' => new CipherParams( 'password', 'aes-128-cbc' )); $unencrypted = self::$ably->channel( 'persisted:mismatch2', $options ); $messages = $unencrypted->history(); $this->assertNotNull( $messages, 'Expected non-null messages' ); @@ -218,29 +272,43 @@ public function testUnencryptedMessageEncryptedHistory() { /** * Encryption key mismatch - publish message with key1, retrieve history with key2 - * - * @expectedException Ably\Exceptions\AblyEncryptionException */ public function testEncryptionKeyMismatch() { + $errorLogged = false; + + $ably = new AblyRest( array_merge( self::$defaultOptions, array( + 'key' => self::$testApp->getAppKeyDefault()->string, + 'logHandler' => function( $level, $args ) use ( &$errorLogged ) { + if ( $level == Log::ERROR ) { + $errorLogged = true; + } + }, + ) ) ); + $payload = 'This is a test message'; $options = array( 'encrypted' => true, 'cipherParams' => new CipherParams( 'password', 'aes-128-cbc' )); - $encrypted1 = self::$ably->channel( 'persisted:mismatch3', $options ); + $encrypted1 = $ably->channel( 'persisted:mismatch3', $options ); $encrypted1->publish( 'test', $payload ); $options2 = array( 'encrypted' => true, 'cipherParams' => new CipherParams( 'DIFFERENT PASSWORD', 'aes-128-cbc' )); - $encrypted2 = self::$ably->channel( 'persisted:mismatch3', $options2 ); + $encrypted2 = $ably->channel( 'persisted:mismatch3', $options2 ); $messages = $encrypted2->history(); + $msg = $messages->items[0]; + + $this->assertTrue( $errorLogged, 'Expected an error to be logged' ); + $this->assertEquals( 'utf-8/cipher+aes-128-cbc/base64', $msg->originalEncoding, 'Expected the original message to be encrypted + base64 encoded' ); + $this->assertEquals( 'utf-8/cipher+aes-128-cbc', $msg->encoding, 'Expected to receive the message still encrypted, but base64 decoded' ); } } class HttpMockMsgCounter extends Http { - public $requests = 0; + public $requestCount = 0; public function request($method, $url, $headers = array(), $params = array()) { - $this->requests++; + $this->requestCount++; return array( 'headers' => 'HTTP/1.1 200 OK'."\n", From 746b97cda5a42d771e6ffd2ea266adc601d1d239 Mon Sep 17 00:00:00 2001 From: blade Date: Thu, 7 May 2015 13:14:23 +0200 Subject: [PATCH 3/5] Fixed logger relate errors in tests Changed hostTimeout to match the spec --- src/AblyRest.php | 2 ++ src/Log.php | 5 ----- src/Models/ClientOptions.php | 2 +- tests/LogTest.php | 7 +++++++ 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/AblyRest.php b/src/AblyRest.php index 72e004c..7c3225a 100644 --- a/src/AblyRest.php +++ b/src/AblyRest.php @@ -43,6 +43,8 @@ public function __construct( $options = array() ) { Log::setLogLevel( $this->options->logLevel ); if ( !empty( $this->options->logHandler ) ) { Log::setLogCallback( $this->options->logHandler ); + } else { + Log::setLogCallback( null ); } $httpClass = $this->options->httpClass; diff --git a/src/Log.php b/src/Log.php index 4c505c8..656e677 100644 --- a/src/Log.php +++ b/src/Log.php @@ -39,11 +39,6 @@ public static function setLogLevel( $logLevel ) { */ public static function setLogCallback( $function = null ) { self::$logCallback = $function; - if ( $function ) { - self::v( 'Set custom logging callback function' ); - } else { - self::v( 'Restored default logging function' ); - } } /** diff --git a/src/Models/ClientOptions.php b/src/Models/ClientOptions.php index 77849e5..1da186a 100644 --- a/src/Models/ClientOptions.php +++ b/src/Models/ClientOptions.php @@ -59,7 +59,7 @@ class ClientOptions extends AuthOptions { /** * @var integer connection timeout after which a next fallback host is used */ - public $hostTimeout = 10000; + public $hostTimeout = 15000; /** * @var string a class that should be used for making HTTP connections diff --git a/tests/LogTest.php b/tests/LogTest.php index 3954a97..450098b 100644 --- a/tests/LogTest.php +++ b/tests/LogTest.php @@ -7,6 +7,13 @@ class LogTest extends \PHPUnit_Framework_TestCase { + public static function tearDownAfterClass() { + // ensure the logger is reset to default + $ably = new AblyRest( array( + 'key' => 'fake.key:totallyFake' + ) ); + } + private function logMessages() { Log::v('This is a test verbose message.'); Log::d('This is a test debug message.'); From 7b5a247bc4d36c6a5c56e8482b221243b4f392e1 Mon Sep 17 00:00:00 2001 From: blade Date: Fri, 8 May 2015 11:48:05 +0200 Subject: [PATCH 4/5] Fixed ChannelMessagesTest::testInvalidTypes Explicit string casts in ChannelHistoryTest.php Refactored ErrorInfo and AblyException --- src/AblyRest.php | 16 ++++++++---- src/Exceptions/AblyException.php | 32 +++++++++++++++++++++--- src/Exceptions/ErrorInfo.php | 30 ---------------------- src/Models/BaseMessage.php | 27 ++++++++++---------- src/Models/ErrorInfo.php | 20 +++++++++++++++ tests/ChannelHistoryTest.php | 22 ++++++++-------- tests/ChannelMessagesTest.php | 43 ++++++++++++++++++++++++++------ 7 files changed, 119 insertions(+), 71 deletions(-) delete mode 100644 src/Exceptions/ErrorInfo.php create mode 100644 src/Models/ErrorInfo.php diff --git a/src/AblyRest.php b/src/AblyRest.php index 7c3225a..828c9c7 100644 --- a/src/AblyRest.php +++ b/src/AblyRest.php @@ -6,6 +6,9 @@ use Ably\Exceptions\AblyException; use Ably\Exceptions\AblyRequestException; +/** + * Ably REST client + */ class AblyRest { private $options; @@ -25,7 +28,7 @@ class AblyRest { /** * Constructor - * @param \Ably\Models\ClientOptions|string options or a string with app key or token + * @param \Ably\Models\ClientOptions|string array with options or a string with app key or token */ public function __construct( $options = array() ) { @@ -56,6 +59,7 @@ public function __construct( $options = array() ) { } /** + * Shorthand to $this->channels->get() * @return \Ably\Channel Channel */ public function channel( $name, $options = array() ) { @@ -72,7 +76,8 @@ public function stats( $params = array() ) { } /** - * @return integer server's time + * Retrieves server time + * @return integer server time in milliseconds */ public function time() { $res = $this->get( '/time', $params = array(), $headers = array(), $returnHeaders = false, $authHeaders = false ); @@ -80,7 +85,8 @@ public function time() { } /** - * @return integer system time + * Returns local time + * @return integer system time in milliseconds */ public function systemTime() { return round( microtime(true) * 1000 ); @@ -133,7 +139,7 @@ public function request( $method, $path, $headers = array(), $params = array(), $causedByExpiredToken = $auth && !$this->auth->isUsingBasicAuth() - && $e->code == 40140; + && $e->getCode() == 40140; if ( $causedByExpiredToken ) { // renew the token $this->auth->authorise( array(), array(), $force = true ); @@ -169,7 +175,7 @@ protected function requestWithFallback( $method, $path, $headers = array(), $par return $this->http->request( $method, $server . $path, $headers, $params ); } catch (AblyRequestException $e) { - if ( $e->code >= 50000 ) { + if ( $e->getCode() >= 50000 ) { if ( $attempt < count( $this->options->fallbackHosts ) ) { return $this->requestWithFallback( $method, $path, $headers, $params, $attempt + 1); } else { diff --git a/src/Exceptions/AblyException.php b/src/Exceptions/AblyException.php index 04a7e91..8a60da0 100644 --- a/src/Exceptions/AblyException.php +++ b/src/Exceptions/AblyException.php @@ -1,12 +1,38 @@ errorInfo = new ErrorInfo(); + $this->errorInfo->message = $message; + $this->errorInfo->code = $code; + $this->errorInfo->statusCode = $statusCode; + } - public function __construct($message, $code = 40000, $statusCode = 400 ) { - parent::__construct($message, $code, $statusCode); + public function getStatusCode() { + return $this->errorInfo->statusCode; } + + // PHP doesn't allow overriding these methods + + // public function getCode() { + // return $this->errorInfo->code; + // } + + // public function getMessage() { + // return $this->errorInfo->message; + // } } diff --git a/src/Exceptions/ErrorInfo.php b/src/Exceptions/ErrorInfo.php deleted file mode 100644 index 855b974..0000000 --- a/src/Exceptions/ErrorInfo.php +++ /dev/null @@ -1,30 +0,0 @@ -$name; - } - - - public function __construct( $message, $code, $statusCode ) { - parent::__construct( $message, $code ); - - $this->statusCode = $statusCode; - } - - public function getStatusCode() { - return $this->statusCode; - } -} diff --git a/src/Models/BaseMessage.php b/src/Models/BaseMessage.php index 6c6aa38..bc64142 100644 --- a/src/Models/BaseMessage.php +++ b/src/Models/BaseMessage.php @@ -104,30 +104,29 @@ protected function encode() { return $msg; } - if (is_array( $this->data ) || is_object( $this->data )) { - + if ( is_array( $this->data ) || $this->data instanceof \stdClass ) { $type = 'json/utf-8'; $msg->data = json_encode($this->data); - } else if (!mb_check_encoding( $this->data, 'UTF-8' )) { // non-UTF-8, most likely a binary string - - if (!$this->cipherParams) { - $type = 'base64'; - $msg->data = base64_encode( $this->data ); - } else { - $type = ''; + } else if ( is_string( $this->data ) ){ + if ( mb_check_encoding( $this->data, 'UTF-8' ) ) { // it's a UTF-8 string + $type = 'utf-8'; $msg->data = $this->data; + } else { // not UTF-8, assuming it's a binary string + if ($this->cipherParams) { // encryption will automatically base64 encode the data + $type = ''; + $msg->data = $this->data; + } else { + $type = 'base64'; + $msg->data = base64_encode( $this->data ); + } } - } else if ( is_string( $this->data ) ){ // it's a UTF-8 string - - $type = 'utf-8'; - $msg->data = $this->data; } else { throw new AblyException( 'Message data must be either, string, string with binary data, or JSON-encodable array or object.', 40003, 400 ); } if ($this->cipherParams) { $msg->data = base64_encode( Crypto::encrypt( $msg->data, $this->cipherParams ) ); - $msg->encoding = $type . '/cipher+' . $this->cipherParams->algorithm . '/base64'; + $msg->encoding = ( $type ? $type . '/' : '' ) . 'cipher+' . $this->cipherParams->algorithm . '/base64'; } else { $msg->encoding = $type; } diff --git a/src/Models/ErrorInfo.php b/src/Models/ErrorInfo.php new file mode 100644 index 0000000..18d4332 --- /dev/null +++ b/src/Models/ErrorInfo.php @@ -0,0 +1,20 @@ +name = 'history'.$i; - $msg->data = ''.$i; + $msg->data = (string) $i; $msgsToSend[] = $msg; } $history1->publish( $msgsToSend ); @@ -65,7 +65,7 @@ public function testPublishEventsAndCheckOrderBackwards() { for ( $i = 0; $i < 50; $i++ ) { $msg = new Message(); $msg->name = 'history'.$i; - $msg->data = ''.$i; + $msg->data = (string) $i; $msgsToSend[] = $msg; } $history2->publish( $msgsToSend ); @@ -91,7 +91,7 @@ public function testDefaults() { $msgsToSend = array(); for ( $i = 0; $i < 101; $i++ ) { $msg = new Message(); - $msg->data = ''.$i; + $msg->data = (string) $i; $msgsToSend[] = $msg; } $channel->publish( $msgsToSend ); @@ -119,7 +119,7 @@ public function testPublishEventsGetLimitedHistoryAndCheckOrderForwards() { for ( $i = 0; $i < 50; $i++ ) { $msg = new Message(); $msg->name = 'history'.$i; - $msg->data = ''.$i; + $msg->data = (string) $i; $msgsToSend[] = $msg; } $history3->publish( $msgsToSend ); @@ -183,7 +183,7 @@ public function testPublishEventsGetLimitedHistoryAndCheckOrderBackwards() { for ( $i = 0; $i < 50; $i++ ) { $msg = new Message(); $msg->name = 'history'.$i; - $msg->data = ''.$i; + $msg->data = (string) $i; $msgsToSend[] = $msg; } $history4->publish( $msgsToSend ); @@ -246,17 +246,17 @@ public function testPublishEventsAndCheckExpectedHistoryInTimeSlicesForwards() { # send batches of messages with short inter-message delay for ($i=0; $i<2; $i++) { - $history5->publish('history'.$i, sprintf('%s',$i)); + $history5->publish( 'history'.$i, (string) $i ); usleep(100000); // sleep for 0.1 of a second } $interval_start = self::$ably->time(); for ($i=2; $i<4; $i++) { - $history5->publish('history'.$i, sprintf('%s',$i)); + $history5->publish( 'history'.$i, (string) $i ); usleep(100000); // sleep for 0.1 of a second } $interval_end = self::$ably->time(); for ($i=4; $i<6; $i++) { - $history5->publish('history'.$i, sprintf('%s',$i)); + $history5->publish( 'history'.$i, (string) $i ); usleep(100000); // sleep for 0.1 of a second } @@ -288,17 +288,17 @@ public function testPublishEventsAndCheckExpectedHistoryInTimeSlicesBackwards() # send batches of messages with short inter-message delay for ($i=0; $i<2; $i++) { - $history6->publish('history'.$i, sprintf('%s',$i)); + $history6->publish( 'history'.$i, (string) $i ); usleep(100000); // sleep for 0.1 of a second } $interval_start = self::$ably->time(); for ($i=2; $i<4; $i++) { - $history6->publish('history'.$i, sprintf('%s',$i)); + $history6->publish( 'history'.$i, (string) $i ); usleep(100000); // sleep for 0.1 of a second } $interval_end = self::$ably->time(); for ($i=4; $i<6; $i++) { - $history6->publish('history'.$i, sprintf('%s',$i)); + $history6->publish( 'history'.$i, (string) $i ); usleep(100000); // sleep for 0.1 of a second } diff --git a/tests/ChannelMessagesTest.php b/tests/ChannelMessagesTest.php index cd33bda..09a75a9 100644 --- a/tests/ChannelMessagesTest.php +++ b/tests/ChannelMessagesTest.php @@ -4,6 +4,7 @@ use Ably\Channel; use Ably\Http; use Ably\Log; +use Ably\Exceptions\AblyException; use Ably\Models\CipherParams; use Ably\Models\Message; @@ -181,22 +182,34 @@ public function testInvalidTypes() { $msg->name = 'int'; $msg->data = 81403; - $this->setExpectedException( 'Ably\Exceptions\AblyException', '', 40003 ); - $channel->publish( $msg ); + try { + $channel->publish( $msg ); + $this->fail( 'Expected an exception' ); + } catch (AblyException $e) { + if ( $e->getCode() != 40003 ) $this->fail('Expected exception error code 40003'); + } $msg = new Message(); $msg->name = 'bool'; $msg->data = true; - $this->setExpectedException( 'Ably\Exceptions\AblyException', '', 40003 ); - $channel->publish( $msg ); + try { + $channel->publish( $msg ); + $this->fail( 'Expected an exception' ); + } catch (AblyException $e) { + if ( $e->getCode() != 40003 ) $this->fail('Expected exception error code 40003'); + } $msg = new Message(); $msg->name = 'float'; $msg->data = 42.23; - $this->setExpectedException( 'Ably\Exceptions\AblyException', '', 40003 ); - $channel->publish( $msg ); + try { + $channel->publish( $msg ); + $this->fail( 'Expected an exception' ); + } catch (AblyException $e) { + if ( $e->getCode() != 40003 ) $this->fail('Expected exception error code 40003'); + } $msg = new Message(); $msg->name = 'function'; @@ -204,8 +217,22 @@ public function testInvalidTypes() { return "mock function"; }; - $this->setExpectedException( 'Ably\Exceptions\AblyException', '', 40003 ); - $channel->publish( $msg ); + try { + $channel->publish( $msg ); + $this->fail( 'Expected an exception' ); + } catch (AblyException $e) { + if ( $e->getCode() != 40003 ) $this->fail('Expected exception error code 40003'); + } + + $msg = new Message(); + $msg->name = 'null'; + + try { + $channel->publish( $msg ); + $this->fail( 'Expected an exception' ); + } catch (AblyException $e) { + if ( $e->getCode() != 40003 ) $this->fail('Expected exception error code 40003'); + } } /** From e1b952e11270e22b1d317040aff52dad37af421e Mon Sep 17 00:00:00 2001 From: blade Date: Fri, 8 May 2015 13:06:13 +0200 Subject: [PATCH 5/5] Empty AblyException defaults --- src/Exceptions/AblyException.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Exceptions/AblyException.php b/src/Exceptions/AblyException.php index 8a60da0..d9ce4fc 100644 --- a/src/Exceptions/AblyException.php +++ b/src/Exceptions/AblyException.php @@ -14,7 +14,7 @@ class AblyException extends Exception { */ public $errorInfo; - public function __construct( $message, $code = 40000, $statusCode = 400 ) { + public function __construct( $message, $code = null, $statusCode = null ) { parent::__construct( $message, $code ); $this->errorInfo = new ErrorInfo(); $this->errorInfo->message = $message;