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