Skip to content

Make Payload case-insensitive — single streaming pass (Fixes #108)#113

Open
russ wants to merge 1 commit into
cable-cr:masterfrom
russ:fix-108-case-insensitive-payload
Open

Make Payload case-insensitive — single streaming pass (Fixes #108)#113
russ wants to merge 1 commit into
cable-cr:masterfrom
russ:fix-108-case-insensitive-payload

Conversation

@russ

@russ russ commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Problem

Fixes #108.

Crystal's JSON parsing is case-sensitive, so a payload like {"Command": "Subscribe"} failed two ways:

  1. Field namePayload.from_json raised Missing JSON attribute: command because the key was Command, not command.
  2. Command valueConnection#receive dispatched on payload.command == "subscribe", so a Subscribe value never matched.

Approach (and why not the obvious one)

Payload.from_json is the hot path — it runs for every inbound frame — so it has to stay fast. The tempting fix (parse the message into a JSON::Any tree, lowercase the keys, re-serialize, re-parse) is ~2.8× slower with ~2.5× the allocations, which isn't acceptable here.

Instead, this keeps the protocol keys as regular JSON::Serializable fields, so a correctly-cased message is matched directly in a single streaming pass with no extra work. A mis-cased key isn't matched, so it falls through to on_unknown_json_attribute, where String#compare(case_insensitive: true) matches it without allocating a downcased copy of the key — all still within the one pass, no re-parsing.

  • command / identifier become nilable internally so a mis-cased-only key isn't rejected by the "Missing JSON attribute" check before we can capture it. The public accessors re-enforce presence, so a genuinely missing command/identifier still raises JSON::SerializableError (now at access, still caught by Cable::Handler).
  • data (read from json_unmapped) is captured under its lowercase key the same way.
  • Connection#receive matches the command value case-insensitively with the same allocation-free compare.

Benchmark

Parsing a typical payload, vs the original case-sensitive parse:

Approach Throughput Alloc/op
original (case-sensitive) 1.53M/s 1.07 kB
tree rebuild (rejected) ~0.53M/s 2.71 kB
this PR — correctly-cased 1.48M/s 1.07 kB
this PR — mis-cased 1.33M/s 1.07 kB

The happy path is within ~3% of today's parse with identical allocations; a mis-cased payload is ~15% slower with no extra allocation.

Tests

  • Payload parses mixed-case top-level keys ("Command", "Identifier", "DATA").
  • A genuinely missing command still raises JSON::SerializableError.
  • End-to-end #receive accepts {"Command": "Subscribe", "Identifier": ...} and confirms the subscription.

Green locally on Crystal 1.10.0 and current stable.

Note

The nightly CI job is continue-on-error: true and fails repo-wide because ameba won't compile against Crystal nightly — unrelated to this change.

@russ russ marked this pull request as ready for review June 23, 2026 21:29
Comment thread src/cable/connection.cr Outdated
return message(payload) if payload.command == "message"
# Compare case-insensitively so a command like "Subscribe" still
# dispatches correctly (see issue #108).
case payload.command.downcase

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of doing this, we can add an after_initialize method that just does @command = @command.downcase. That gets set in the Payload

Comment thread src/cable/payload.cr Outdated
# Only the top-level keys (command/identifier/data) are touched; the nested
# values — channel names and the identifier/data blobs — are left exactly as
# sent, since those remain case-sensitive. See issue #108.
def self.from_json(string : String) : self

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is going to be a fairly large performance hit... I have a feeling that at the moment, this isn't really possible in Crystal without doing this method here.

JSON::Field supports a key option where you can do @[JSON::Field(key: "Command")], but now the lowercase version doesn't work. A "hacky" method would be to define both a @command : String? and @Command : String? then an instance method command which chooses one or the other 😂 but.... gross lol.

What would be cool is if the annotation allowed for a regex or something, but that would require a new feature added to Crystal. Maybe for now just having the command set to lowercase is a decent enough update and we just make sure to document that the key "command" must also be lowercase?

@russ russ force-pushed the fix-108-case-insensitive-payload branch from aa42b8a to 3abe314 Compare June 24, 2026 17:26
@russ russ changed the title Make Payload case-insensitive (Fixes #108) Make Payload case-insensitive — single streaming pass (Fixes #108) Jun 24, 2026
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 cable-cr#108

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BMW1EsdCMExBhm3tg7TQhs
@russ russ force-pushed the fix-108-case-insensitive-payload branch from 3abe314 to 4abe97d Compare June 24, 2026 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow Payload to be case-insensitive

2 participants