From f9db4f2ed34615b265613b3495c42b17a2324792 Mon Sep 17 00:00:00 2001 From: Luis Lavena Date: Sat, 8 Mar 2025 17:22:59 +0100 Subject: [PATCH 1/3] Avoids duplicate responses to ping frames Before this change, when using `#on_ping` callback and defining your own block of code to handle it, resulted in two `pong` frames being sent to the other site of the WebSocket. This change setups a default ping <-> pong response cycle but once you override it, it will use yours instead. --- src/http/web_socket.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/http/web_socket.cr b/src/http/web_socket.cr index e2d56b8a7610..004ee23424f0 100644 --- a/src/http/web_socket.cr +++ b/src/http/web_socket.cr @@ -14,6 +14,7 @@ class HTTP::WebSocket def initialize(@ws : Protocol) @buffer = Bytes.new(4096) @current_message = IO::Memory.new + @on_ping = ->(message : String) { pong(message) unless closed? } end # Opens a new websocket using the information provided by the URI. This will also handle the handshake @@ -163,7 +164,6 @@ class HTTP::WebSocket if info.final message = @current_message.to_s @on_ping.try &.call(message) - pong(message) unless closed? @current_message.clear end in .pong? From a444127c84ea1d3b7a85b24d0ed8174a0a1ce16f Mon Sep 17 00:00:00 2001 From: Luis Lavena Date: Tue, 8 Apr 2025 23:50:03 +0200 Subject: [PATCH 2/3] Extract implicit actions around ping and close events Per RFC 6455, for any PING frame, a similar PONG frame with same payload is expected as response. Previous change broke that if the developer used `#on_ping` callback to setup their own event, resulting a broken implementation. This extracts both ping and close events as private helpers to ease reutilization/override. --- src/http/web_socket.cr | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/http/web_socket.cr b/src/http/web_socket.cr index 004ee23424f0..c4ba45f5f48e 100644 --- a/src/http/web_socket.cr +++ b/src/http/web_socket.cr @@ -163,7 +163,7 @@ class HTTP::WebSocket @current_message.write @buffer[0, info.size] if info.final message = @current_message.to_s - @on_ping.try &.call(message) + do_ping(message) @current_message.clear end in .pong? @@ -197,8 +197,7 @@ class HTTP::WebSocket end message = @current_message.gets_to_end - @on_close.try &.call(code, message) - close + do_close(code, message) @current_message.clear break @@ -208,6 +207,16 @@ class HTTP::WebSocket end end end + + private def do_close(code, message) + @on_close.try &.call(code, message) + close + end + + private def do_ping(message) + @on_ping.try &.call(message) + pong(message) unless closed? + end end require "./web_socket/*" From 277f6e1fbc79ccdecbdea8f558f7c2bbdb8b48fb Mon Sep 17 00:00:00 2001 From: Luis Lavena Date: Wed, 9 Apr 2025 09:26:12 +0200 Subject: [PATCH 3/3] Remove default on_ping callback definition This is no longer relevant and avoids breaking RFC 6455 --- src/http/web_socket.cr | 1 - 1 file changed, 1 deletion(-) diff --git a/src/http/web_socket.cr b/src/http/web_socket.cr index c4ba45f5f48e..752c4482b40a 100644 --- a/src/http/web_socket.cr +++ b/src/http/web_socket.cr @@ -14,7 +14,6 @@ class HTTP::WebSocket def initialize(@ws : Protocol) @buffer = Bytes.new(4096) @current_message = IO::Memory.new - @on_ping = ->(message : String) { pong(message) unless closed? } end # Opens a new websocket using the information provided by the URI. This will also handle the handshake