From de8fa592f450bbce2a7e6ab96bc90ceb89f86da6 Mon Sep 17 00:00:00 2001 From: Michael Slezak Date: Fri, 7 Aug 2020 15:44:36 -0700 Subject: [PATCH 1/2] Adding persistent connection option. Always attempt to close socket when fwrite fails so that resource is let go --- lib/Base.php | 2 + lib/Client.php | 6 +- tests/ClientTest.php | 8 +++ tests/scripts/client.connect-persistent.json | 60 ++++++++++++++++++++ tests/scripts/send-broken-write.json | 7 ++- tests/scripts/send-failed-write.json | 7 ++- 6 files changed, 87 insertions(+), 3 deletions(-) create mode 100644 tests/scripts/client.connect-persistent.json diff --git a/lib/Base.php b/lib/Base.php index a179e9c..deeafda 100644 --- a/lib/Base.php +++ b/lib/Base.php @@ -277,11 +277,13 @@ protected function write($data) $written = fwrite($this->socket, $data); if ($written === false) { $length = strlen($data); + fclose($this->socket); $this->throwException("Failed to write $length bytes."); } if ($written < strlen($data)) { $length = strlen($data); + fclose($this->socket); $this->throwException("Could only write $written out of $length bytes."); } } diff --git a/lib/Client.php b/lib/Client.php index f8bb0b8..1375af7 100644 --- a/lib/Client.php +++ b/lib/Client.php @@ -13,6 +13,7 @@ class Client extends Base { // Default options protected static $default_options = [ + 'persistent' => false, 'timeout' => 5, 'fragment_size' => 4096, 'context' => null, @@ -95,13 +96,16 @@ protected function connect() $context = stream_context_create(); } + $flags = STREAM_CLIENT_CONNECT; + $flags = ($this->options['persistent'] === true) ? $flags | STREAM_CLIENT_PERSISTENT : $flags; + // Open the socket. @ is there to supress warning that we will catch in check below instead. $this->socket = @stream_socket_client( $host_uri . ':' . $port, $errno, $errstr, $this->options['timeout'], - STREAM_CLIENT_CONNECT, + $flags, $context ); diff --git a/tests/ClientTest.php b/tests/ClientTest.php index 17b5e25..af996ce 100644 --- a/tests/ClientTest.php +++ b/tests/ClientTest.php @@ -226,6 +226,14 @@ public function testReconnect() $this->assertTrue(MockSocket::isEmpty()); } + public function testPersistentConnection() + { + MockSocket::initialize('client.connect-persistent', $this); + $client = new Client('ws://localhost:8000/my/mock/path', ['persistent' => true]); + $client->send('Connect'); + $this->assertTrue(MockSocket::isEmpty()); + } + /** * @expectedException WebSocket\BadUriException * @expectedExceptionMessage Url should have scheme ws or wss diff --git a/tests/scripts/client.connect-persistent.json b/tests/scripts/client.connect-persistent.json new file mode 100644 index 0000000..0628caa --- /dev/null +++ b/tests/scripts/client.connect-persistent.json @@ -0,0 +1,60 @@ +[ + { + "function": "stream_context_create", + "params": [], + "return": "@mock-stream-context" + }, + { + "function": "stream_socket_client", + "params": [ + "tcp:\/\/localhost:8000", + null, + null, + 5, + 5, + "@mock-stream-context" + ], + "return": "@mock-stream" + }, + { + "function": "get_resource_type", + "params": [ + "@mock-stream" + ], + "return": "stream" + }, + { + "function": "stream_set_timeout", + "params": [ + "@mock-stream", + 5 + ], + "return": true + }, + { + "function": "fwrite", + "params": [ + "@mock-stream" + ], + "return-op": "key-save", + "return": 248 + }, + { + "function": "stream_get_line", + "params": [ + "@mock-stream", + 1024, + "\r\n\r\n" + ], + "return-op": "key-respond", + "return": "HTTP\/1.1 101 Switching Protocols\r\nUpgrade: websocket\r\nConnection: Upgrade\r\nSec-WebSocket-Accept: {key}" + }, + { + "function": "fwrite", + "params": [ + "@mock-stream" + ], + "return": 13 + } +] + diff --git a/tests/scripts/send-broken-write.json b/tests/scripts/send-broken-write.json index 4e2a4aa..8460cac 100644 --- a/tests/scripts/send-broken-write.json +++ b/tests/scripts/send-broken-write.json @@ -13,6 +13,11 @@ ], "return": 18 }, + { + "function": "fclose", + "params": [], + "return": true + }, { "function": "stream_get_meta_data", "params": [ @@ -28,4 +33,4 @@ "seekable": false } } -] \ No newline at end of file +] diff --git a/tests/scripts/send-failed-write.json b/tests/scripts/send-failed-write.json index 325e0fb..3d1d71b 100644 --- a/tests/scripts/send-failed-write.json +++ b/tests/scripts/send-failed-write.json @@ -13,6 +13,11 @@ ], "return": false }, + { + "function": "fclose", + "params": [], + "return": true + }, { "function": "stream_get_meta_data", "params": [ @@ -28,4 +33,4 @@ "seekable": false } } -] \ No newline at end of file +] From 2aa93892131369694677026a1cc5407ed1cc97b0 Mon Sep 17 00:00:00 2001 From: Michael Slezak Date: Tue, 11 Aug 2020 11:29:00 -0700 Subject: [PATCH 2/2] Add documentation around TimeoutException and persistent client option --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index c062c90..84973e2 100644 --- a/README.md +++ b/README.md @@ -82,6 +82,7 @@ The `$options` parameter in constructor accepts an associative array of options. * `fragment_size` - Maximum payload size. Default 4096 chars. * `context` - A stream context created using [stream_context_create](https://www.php.net/manual/en/function.stream-context-create). * `headers` - Additional headers as associative array name => content. +* `persistent` - Connection is re-used between requests until time out is reached. Default false. ```php $context = stream_context_create(); @@ -185,6 +186,7 @@ $server = new WebSocket\Server([ * `WebSocket\BadOpcodeException` - Thrown if provided opcode is invalid. * `WebSocket\BadUriException` - Thrown if provided URI is invalid. * `WebSocket\ConnectionException` - Thrown on any socket I/O failure. +* `WebSocket\TimeoutExeception` - Thrown when the socket experiences a time out. ## Development and contribution