[1.x] Add Message Payload Validation and Improve Channel Data Handling (

#303) * fix: ensure channel_data is string before json decoding Adds type checking for channel_data to prevent JSON decode errors when the data is not a string format, improving the robustness of message handling in the CLI logger. * feat: add message payload validation in WebSocket server Add comprehensive validation for incoming WebSocket messages using Laravel Validator to: - Ensure correct message structure and data types - Prevent malformed data from causing server errors - Validate required fields (event, data.channel) - Enforce JSON format for channel_data - Improve overall server stability and security This change helps prevent server crashes and unexpected behavior caused by invalid message formats. * test: add validation test cases for WebSocket message payloads Add comprehensive test coverage for message payload validation including: - Event type validation - Data structure validation - Channel format validation - Auth data type checking - Channel data format verification Each test case ensures proper error responses for invalid message formats, maintaining consistency with Pusher protocol specifications. * updates validation * update validation * remove breaking change --------- Co-authored-by: Joe Dixon <[email protected]>
laravel · Jan 21, 2025 · c55c36a · Baggins800 · Jan 24, 2025 · c55c36a
1 parent c3a4961
commit c55c36a
Show file tree

Hide file tree

Showing 5 changed files with 182 additions and 5 deletions.
diff --git a/src/Loggers/CliLogger.php b/src/Loggers/CliLogger.php
@@ -52,7 +52,7 @@ public function message(string $message): void
             $message['data'] = json_decode($message['data'], true);
         }
 
-        if (isset($message['data']['channel_data'])) {
+        if (isset($message['data']['channel_data']) && is_string($message['data']['channel_data'])) {
             $message['data']['channel_data'] = json_decode($message['data']['channel_data'], true);
         }
 

diff --git a/src/Protocols/Pusher/ClientEvent.php b/src/Protocols/Pusher/ClientEvent.php
@@ -2,6 +2,7 @@
 
 namespace Laravel\Reverb\Protocols\Pusher;
 
+use Illuminate\Support\Facades\Validator;
 use Illuminate\Support\Str;
 use Laravel\Reverb\Contracts\Connection;
 
@@ -12,6 +13,12 @@ class ClientEvent
      */
     public static function handle(Connection $connection, array $event): ?ClientEvent
     {
+        Validator::make($event, [
+            'event' => ['required', 'string'],
+            'channel' => ['required', 'string'],
+            'data' => ['required', 'array'],
+        ])->validate();
+
         if (! Str::startsWith($event['event'], 'client-')) {
             return null;
         }

diff --git a/src/Protocols/Pusher/EventHandler.php b/src/Protocols/Pusher/EventHandler.php
@@ -3,6 +3,7 @@
 namespace Laravel\Reverb\Protocols\Pusher;
 
 use Exception;
+use Illuminate\Support\Facades\Validator;
 use Illuminate\Support\Str;
 use Laravel\Reverb\Contracts\Connection;
 use Laravel\Reverb\Protocols\Pusher\Channels\CacheChannel;
@@ -24,7 +25,9 @@ public function __construct(protected ChannelManager $channels)
      */
     public function handle(Connection $connection, string $event, array $payload = []): void
     {
-        match (Str::after($event, 'pusher:')) {
+        $event = Str::after($event, 'pusher:');
+
+        match ($event) {
             'connection_established' => $this->acknowledge($connection),
             'subscribe' => $this->subscribe(
                 $connection,
@@ -55,6 +58,16 @@ public function acknowledge(Connection $connection): void
      */
     public function subscribe(Connection $connection, string $channel, ?string $auth = null, ?string $data = null): void
     {
+        Validator::make([
+            'channel' => $channel,
+            'auth' => $auth,
+            'channel_data' => $data,
+        ], [
+            'channel' => ['nullable', 'string'],
+            'auth' => ['nullable', 'string'],
+            'channel_data' => ['nullable', 'json'],
+        ])->validate();
+
         $channel = $this->channels
             ->for($connection->app())
             ->findOrCreate($channel);

diff --git a/src/Protocols/Pusher/Server.php b/src/Protocols/Pusher/Server.php
@@ -3,6 +3,7 @@
 namespace Laravel\Reverb\Protocols\Pusher;
 
 use Exception;
+use Illuminate\Support\Facades\Validator;
 use Illuminate\Support\Str;
 use Laravel\Reverb\Contracts\Connection;
 use Laravel\Reverb\Events\MessageReceived;
@@ -12,6 +13,7 @@
 use Laravel\Reverb\Protocols\Pusher\Exceptions\PusherException;
 use Ratchet\RFC6455\Messaging\Frame;
 use Ratchet\RFC6455\Messaging\FrameInterface;
+use Throwable;
 
 class Server
 {
@@ -54,20 +56,21 @@ public function message(Connection $from, string $message): void
         try {
             $event = json_decode($message, associative: true, flags: JSON_THROW_ON_ERROR);
 
+            Validator::make($event, ['event' => ['required', 'string']])->validate();
+
             match (Str::startsWith($event['event'], 'pusher:')) {
                 true => $this->handler->handle(
                     $from,
                     $event['event'],
                     empty($event['data']) ? [] : $event['data'],
-                    $event['channel'] ?? null
                 ),
                 default => ClientEvent::handle($from, $event)
             };
 
             Log::info('Message Handled', $from->id());
 
             MessageReceived::dispatch($from, $message);
-        } catch (Exception $e) {
+        } catch (Throwable $e) {
             $this->error($from, $e);
         }
     }
@@ -104,7 +107,7 @@ public function close(Connection $connection): void
     /**
      * Handle an error.
      */
-    public function error(Connection $connection, Exception $exception): void
+    public function error(Connection $connection, Throwable $exception): void
     {
         if ($exception instanceof PusherException) {
             $connection->send(json_encode($exception->payload()));

diff --git a/tests/Unit/Protocols/Pusher/ServerTest.php b/tests/Unit/Protocols/Pusher/ServerTest.php
@@ -329,3 +329,157 @@
         ['localhost', '*.localhost'],
     ],
 ]);
+
+it('sends an error if something fails for event type', function () {
+    $this->server->message(
+        $connection = new FakeConnection,
+        json_encode([
+            'event' => [],
+        ])
+    );
+
+    $connection->assertReceived([
+        'event' => 'pusher:error',
+        'data' => json_encode([
+            'code' => 4200,
+            'message' => 'Invalid message format',
+        ]),
+    ]);
+});
+
+it('sends an error if something fails for data type', function () {
+    $this->server->message(
+        $connection = new FakeConnection,
+        json_encode([
+            'event' => 'pusher:subscribe',
+            'data' => 'sfsfsfs',
+        ])
+    );
+
+    $connection->assertReceived([
+        'event' => 'pusher:error',
+        'data' => json_encode([
+            'code' => 4200,
+            'message' => 'Invalid message format',
+        ]),
+    ]);
+});
+
+it('sends an error if something fails for data channel type', function () {
+    $this->server->message(
+        $connection = new FakeConnection,
+        json_encode([
+            'event' => 'pusher:subscribe',
+            'data' => [
+                'channel' => [],
+            ],
+        ])
+    );
+
+    $connection->assertReceived([
+        'event' => 'pusher:error',
+        'data' => json_encode([
+            'code' => 4200,
+            'message' => 'Invalid message format',
+        ]),
+    ]);
+
+    $this->server->message(
+        $connection = new FakeConnection,
+        json_encode([
+            'event' => 'pusher:subscribe',
+            'data' => [
+                'channel' => null,
+            ],
+        ])
+    );
+
+    $connection->assertReceived([
+        'event' => 'pusher:error',
+        'data' => json_encode([
+            'code' => 4200,
+            'message' => 'Invalid message format',
+        ]),
+    ]);
+});
+
+it('sends an error if something fails for data auth type', function () {
+    $this->server->message(
+        $connection = new FakeConnection,
+        json_encode([
+            'event' => 'pusher:subscribe',
+            'data' => [
+                'channel' => 'presence-test-channel',
+                'auth' => [],
+            ],
+        ])
+    );
+
+    $connection->assertReceived([
+        'event' => 'pusher:error',
+        'data' => json_encode([
+            'code' => 4200,
+            'message' => 'Invalid message format',
+        ]),
+    ]);
+});
+
+it('sends an error if something fails for data channel_data type', function () {
+    $this->server->message(
+        $connection = new FakeConnection,
+        json_encode([
+            'event' => 'pusher:subscribe',
+            'data' => [
+                'channel' => 'presence-test-channel',
+                'auth' => '',
+                'channel_data' => [],
+            ],
+        ])
+    );
+
+    $connection->assertReceived([
+        'event' => 'pusher:error',
+        'data' => json_encode([
+            'code' => 4200,
+            'message' => 'Invalid message format',
+        ]),
+    ]);
+
+    $this->server->message(
+        $connection = new FakeConnection,
+        json_encode([
+            'event' => 'pusher:subscribe',
+            'data' => [
+                'channel' => 'presence-test-channel',
+                'auth' => '',
+                'channel_data' => 'Hello',
+            ],
+        ])
+    );
+
+    $connection->assertReceived([
+        'event' => 'pusher:error',
+        'data' => json_encode([
+            'code' => 4200,
+            'message' => 'Invalid message format',
+        ]),
+    ]);
+});
+
+it('sends an error if something fails for channel type', function () {
+    $this->server->message(
+        $connection = new FakeConnection,
+        json_encode([
+            'event' => 'client-start-typing',
+            'channel' => [],
+        ])
+    );
+
+    $connection->assertReceived([
+        'event' => 'pusher:error',
+        'data' => json_encode([
+            'code' => 4200,
+            'message' => 'Invalid message format',
+        ]),
+    ]);
+});