From 599885daf62b887b530ea1099ba9d5e682e45f0b Mon Sep 17 00:00:00 2001 From: frytimo Date: Wed, 27 Aug 2025 11:02:21 -0300 Subject: [PATCH] Fix bug by adding 'tries' counter so reading websocket data can return (#7471) When reading web socket data, the loop will run forever if there is no more data available in a chunk. This fix will add a counter to how many times it tries to read before a timeout is reached. Fixed a problem where the constructor parameter for permission_filter was not being fulfilled. This can cause a crash also leading to the read of web socket data to begin looping. Added better debugging information by using the topic of the websocket_message instead of the payload data. This would cause some messages to appear empty when they were not. Added a wait state for re-connecting to the web socket server for system status service so it would not exit immediately when the websocket server exited. Added more PHPDoc Block information. --- .../classes/active_calls_service.php | 4 ++- .../classes/system_dashboard_service.php | 25 +++++++++++++++---- .../classes/base_websocket_system_service.php | 2 +- .../resources/classes/websocket_client.php | 15 ++++++++--- .../resources/classes/websocket_service.php | 4 +-- 5 files changed, 38 insertions(+), 12 deletions(-) diff --git a/app/active_calls/resources/classes/active_calls_service.php b/app/active_calls/resources/classes/active_calls_service.php index b88993eecd..07a8a32b04 100644 --- a/app/active_calls/resources/classes/active_calls_service.php +++ b/app/active_calls/resources/classes/active_calls_service.php @@ -404,9 +404,11 @@ class active_calls_service extends service implements websocket_service_interfac // Web socket event if ($resource === $this->ws_client->socket()) { - $this->handle_websocket_event($this->ws_client); + $this->handle_websocket_event(); continue; } + + $this->debug('Unknown Event from ' . $resource); } } } diff --git a/app/system/resources/classes/system_dashboard_service.php b/app/system/resources/classes/system_dashboard_service.php index 4290f9c59f..5a0cf52ea6 100644 --- a/app/system/resources/classes/system_dashboard_service.php +++ b/app/system/resources/classes/system_dashboard_service.php @@ -115,8 +115,21 @@ class system_dashboard_service extends base_websocket_system_service { count($cpu_percent_per_core) )); - // Send the broadcast - $this->respond($response); + $show_disconnect_message = true; + try { + // Send the broadcast + $this->respond($response); + } catch (\socket_disconnected_exception $sde) { + // wait until we connect again + while (!$this->connect_to_ws_server()) { + if ($show_disconnect_message) { + $this->warn("Websocket server disconnected"); + $show_disconnect_message = false; + } + sleep(1); + } + $this->warn("Websocket server connected"); + } } @@ -129,16 +142,18 @@ class system_dashboard_service extends base_websocket_system_service { // Get the subscriber permissions $permissions = $subscriber->get_permissions(); - // Create a filter - $filter = filter_chain::and_link([new permission_filter()]); + // Create a filter for broadcaster => permission + $permission_filter = new permission_filter([self::get_service_name() => 'system_view_cpu']); // Match them to create a filter foreach (self::PERMISSIONS as $permission) { if (in_array($permission, $permissions)) { - $filter->add_permission($permission); + $permission_filter->add_permission($permission); } } + $filter = filter_chain::and_link([$permission_filter]); + // Return the filter with user permissions to ensure they can't receive information they shouldn't return $filter; } diff --git a/core/websockets/resources/classes/base_websocket_system_service.php b/core/websockets/resources/classes/base_websocket_system_service.php index 7b60a60b28..ee8ff489fe 100644 --- a/core/websockets/resources/classes/base_websocket_system_service.php +++ b/core/websockets/resources/classes/base_websocket_system_service.php @@ -165,7 +165,7 @@ abstract class base_websocket_system_service extends service implements websocke /** * Connects to the web socket server using a websocket_client object - * @return bool + * @return bool True if connected and False if not able to connect */ protected function connect_to_ws_server(): bool { if ($this->ws_client !== null && $this->ws_client->is_connected()) return true; diff --git a/core/websockets/resources/classes/websocket_client.php b/core/websockets/resources/classes/websocket_client.php index 0a466ca53e..02a39fa039 100644 --- a/core/websockets/resources/classes/websocket_client.php +++ b/core/websockets/resources/classes/websocket_client.php @@ -154,7 +154,7 @@ class websocket_client { */ public static function send($resource, ?string $payload): bool { if (!is_resource($resource)) { - throw new \RuntimeException("Not connected"); + throw new \socket_disconnected_exception($resource, 400); } // Check for a null message and send a disconnect frame @@ -254,8 +254,9 @@ class websocket_client { while (!$final_frame) { $header = $this->read_bytes(2); - if ($header === null) + if (strlen($header) !== 2) { return null; + } $byte1 = ord($header[0]); $byte2 = ord($header[1]); @@ -338,7 +339,9 @@ class websocket_client { $data = ''; $max_chunk_size = stream_get_chunk_size($this->resource); - while (strlen($data) < $length) { + // 20 tries waits 200 ms total per chunk + $tries = 0; + while (strlen($data) < $length && $tries < 20) { $remaining = $length - strlen($data); $read_size = min($max_chunk_size, $remaining); @@ -360,8 +363,14 @@ class websocket_client { usleep(10000); // Try again + $tries++; continue; } + + // reset count for successfully received chunk + $tries = 0; + + // append the chunk to the received data $data .= $chunk; } return $data; diff --git a/core/websockets/resources/classes/websocket_service.php b/core/websockets/resources/classes/websocket_service.php index 1ce547a9da..aa5ed82b10 100644 --- a/core/websockets/resources/classes/websocket_service.php +++ b/core/websockets/resources/classes/websocket_service.php @@ -260,7 +260,7 @@ class websocket_service extends service { foreach ($send_to as $subscriber) { try { // Notify of the message we are broadcasting - $this->debug("Broadcasting message '" . $message->payload['event_name'] . "' for service '" . $message->service_name . "' to subscriber $subscriber->id"); + $this->debug("Broadcasting message '" . $message->topic() . "' for service '" . $message->service_name . "' to subscriber $subscriber->id"); $subscriber->send_message($message); } catch (subscriber_token_expired_exception $ste) { $this->info("Subscriber $ste->id token expired"); @@ -806,6 +806,7 @@ class websocket_service extends service { // Ensure we have the correct number of bytes if (strlen($hdr) !== 2) { $this->warning('Header is empty!'); + $this->debug('Header content: ' . bin2hex($hdr) . '(' . strlen($hdr) . ' bytes)'); $this->update_connected_clients(); return ''; } @@ -884,7 +885,6 @@ class websocket_service extends service { */ public static function send($resource, ?string $payload): bool { if (!is_resource($resource)) { - self::log("Cannot send: invalid resource", LOG_ERR); return false; }