From 4abe97d043d6854e4e28eccfc204aefc993e4665 Mon Sep 17 00:00:00 2001 From: Russ Smith Date: Tue, 23 Jun 2026 14:11:36 -0700 Subject: [PATCH] Make Payload case-insensitive (single streaming pass) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Crystal's JSON parsing is case-sensitive, so a payload like `{"Command": "Subscribe"}` failed two ways: the `Command` key raised "Missing JSON attribute: command", and the `Subscribe` value never matched the lowercase dispatch comparison. This is the hot path for every inbound frame, so it has to stay fast. Rather than re-parsing the message (parse to a tree, downcase the keys, re-serialize, re-parse — ~2.8x slower with ~2.5x the allocations), the protocol keys stay as regular JSON::Serializable fields. Correctly-cased messages are matched directly in a single streaming pass with no extra work; a mis-cased key falls through to #on_unknown_json_attribute, where String#compare(case_insensitive:) matches it without allocating a downcased copy. command/identifier become nilable so a mis-cased-only key isn't rejected before we can capture it; the accessors re-enforce presence (a genuinely missing command/identifier still raises SerializableError). Connection#receive matches the command case-insensitively the same way, avoiding a per-message downcase allocation. Benchmarked against the original case-sensitive parse: correctly-cased payloads are within ~3% with identical allocations; mis-cased ones are ~15% slower with no extra allocation. Fixes #108 Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01BMW1EsdCMExBhm3tg7TQhs --- spec/cable/connection_spec.cr | 12 ++++++++++ spec/cable/payload_spec.cr | 22 ++++++++++++++++++ src/cable/connection.cr | 13 ++++++++--- src/cable/payload.cr | 44 ++++++++++++++++++++++++++++++++--- 4 files changed, 85 insertions(+), 6 deletions(-) diff --git a/spec/cable/connection_spec.cr b/spec/cable/connection_spec.cr index b56bc30..779102d 100644 --- a/spec/cable/connection_spec.cr +++ b/spec/cable/connection_spec.cr @@ -65,6 +65,18 @@ describe Cable::Connection do end end + it "accepts a case-insensitive command and keys (issue #108)" do + connect do |connection, socket| + connection.receive({"Command" => "Subscribe", "Identifier" => {channel: "ChatChannel", room: "1"}.to_json}.to_json) + sleep 100.milliseconds + + socket.messages.should contain({"type" => "confirm_subscription", "identifier" => {channel: "ChatChannel", room: "1"}.to_json}.to_json) + + connection.close + socket.close + end + end + it "accepts without params hash key" do connect do |connection, socket| connection.receive({"command" => "subscribe", "identifier" => {channel: "AppearanceChannel"}.to_json}.to_json) diff --git a/spec/cable/payload_spec.cr b/spec/cable/payload_spec.cr index 785fde1..9f0ff20 100644 --- a/spec/cable/payload_spec.cr +++ b/spec/cable/payload_spec.cr @@ -36,6 +36,20 @@ describe Cable::Payload do payload.action.should eq("invite") end + it "parses case-insensitive top-level keys (issue #108)" do + payload_json = { + "Command" => "message", + "Identifier" => {channel: "ChatChannel"}.to_json, + "DATA" => {invite_id: 3, action: "invite"}.to_json, + }.to_json + + payload = Cable::Payload.from_json(payload_json) + payload.command.should eq("message") + payload.channel.should eq("ChatChannel") + payload.data.should eq({"invite_id" => 3}) + payload.action.should eq("invite") + end + it "raises a SerializableError when the identifier is not a string" do payload_json = { command: "subscribe", @@ -50,4 +64,12 @@ describe Cable::Payload do Cable::Payload.from_json(payload_json) end end + + it "still raises a SerializableError for a missing command (issue #108)" do + payload_json = {identifier: {channel: "ChatChannel"}.to_json}.to_json + + expect_raises(JSON::SerializableError, "Missing JSON attribute: command") do + Cable::Payload.from_json(payload_json).command + end + end end diff --git a/src/cable/connection.cr b/src/cable/connection.cr index 25d4685..14baf1f 100644 --- a/src/cable/connection.cr +++ b/src/cable/connection.cr @@ -123,9 +123,16 @@ module Cable return unless message.presence payload = Cable::Payload.from_json(message) - return subscribe(payload) if payload.command == "subscribe" - return unsubscribe(payload) if payload.command == "unsubscribe" - return message(payload) if payload.command == "message" + # Match the command case-insensitively (see issue #108) without allocating + # a downcased copy of it on this hot path. + command = payload.command + if command.compare("subscribe", case_insensitive: true).zero? + subscribe(payload) + elsif command.compare("unsubscribe", case_insensitive: true).zero? + unsubscribe(payload) + elsif command.compare("message", case_insensitive: true).zero? + message(payload) + end end def subscribe(payload : Cable::Payload) diff --git a/src/cable/payload.cr b/src/cable/payload.cr index a87f5d3..3b339f0 100644 --- a/src/cable/payload.cr +++ b/src/cable/payload.cr @@ -30,15 +30,53 @@ module Cable property key : String = "" end - @[JSON::Field] - getter command : String + # The cable wire protocol uses lowercase keys (command/identifier/data), but + # some clients send them cased differently. These stay as regular mapped + # fields, so a correctly-cased message is matched directly by + # JSON::Serializable in a single streaming pass — the happy path does no + # extra work. A mis-cased key isn't matched, so it falls through to + # #on_unknown_json_attribute (below) and is captured there in that same pass, + # without re-parsing the message. They're nilable so a mis-cased-only key + # doesn't trip the "Missing JSON attribute" check before we can capture it; + # the accessors re-enforce presence. See issue #108. + @command : String? @[JSON::Field(converter: Cable::Payload::IdentifierConverter)] - getter identifier : Indentifier + @identifier : Indentifier? @[JSON::Field(ignore: true)] getter action : String = "" + def command : String + @command || raise_missing_attribute("command") + end + + def identifier : Indentifier + @identifier || raise_missing_attribute("identifier") + end + + # Mis-cased protocol keys land here (correctly-cased ones were already + # matched directly). `String#compare(case_insensitive: true)` matches without + # allocating a downcased copy of the key, so this stays cheap on the hot + # path. Anything genuinely unknown is handed back to + # JSON::Serializable::Unmapped. + protected def on_unknown_json_attribute(pull, key, key_location) + case + when key.compare("command", case_insensitive: true).zero? + @command = pull.read_string + when key.compare("identifier", case_insensitive: true).zero? + @identifier = Cable::Payload::IdentifierConverter.from_json(pull) + when key.compare("data", case_insensitive: true).zero? + json_unmapped["data"] = JSON::Any.new(pull) + else + super + end + end + + private def raise_missing_attribute(attribute : String) : NoReturn + raise JSON::SerializableError.new("Missing JSON attribute: #{attribute}", self.class.to_s, attribute, 0, 0, nil) + end + # After the Payload is deserialized, parse the data. # This will ensure we know if it's an action. def after_initialize