[SignalR] Unknown message type wedges the connection when MaximumReceiveMessageSize is set (the default)
- Dominant language
- C#
- Stars
- 38.4k
- Forks
- 10.9k
- Avg merge
- 2d 10h
- Merged PRs (30d)
- 281
Description
**Observed on:** `release/10.0` @ 10.0.13, commit `1307f36b2990bf5c81aace7c6a1535a2be46d7ef`. I have not checked the 8.0/9.0 branches.
### Summary
Both hub protocols deliberately return `null` for an unrecognised message type so
that it can be *ignored*, but in the default configuration the handler treats that
as "no message was parsed" and never advances past it. The connection stops
processing messages permanently.
The forward-compatibility behaviour the protocols intend only works if you
explicitly disable `MaximumReceiveMessageSize`.
### Detail
Both protocols return `null` for an unknown message type, with the same comment:
- `src/SignalR/common/Protocols.Json/src/Protocol/JsonHubProtocol.cs:468-470`
- `src/SignalR/common/Protocols.MessagePack/src/Protocol/MessagePackHubProtocolWorker.cs:67-69`
```csharp
default:
// Future protocol changes can add message types, old clients can ignore them
return null;
```
Both `TryParseMessage` implementations then `return message != null` — i.e. `false`
— *after* the framing parser (`TextMessageParser` / `BinaryMessageParser`) has
already advanced the input by ref past the record.
`HubConnectionHandler.DispatchMessagesAsync` has two paths that treat that advance
differently.
**Unlimited path (`_maximumMessageSize == null`) — behaves as intended:**
```csharp
while (protocol.TryParseMessage(ref buffer, binder, out var message))
```
`buffer` is passed by ref, so the framing advance survives the `false` return. The
`finally` then calls `input.AdvanceTo(buffer.Start, buffer.End)` with `buffer.Start`
positioned after the unknown message, and it is skipped.
**Size-limited path (`_maximumMessageSize != null`) — wedges:**
```csharp
var segment = buffer;
...
if (protocol.TryParseMessage(ref segment, binder, out var message)) { ... }
else if (overLength) { throw ... }
else
{
// No need to update the buffer since we didn't parse anything
break; // segment.Start is discarded here
}
// Update the buffer to the remaining segment
buffer = buffer.Slice(segment.Start); // not reached on the break path
```
`segment` is a copy. The framing parser advanced `segment`, but `break` skips the
line that copies that position back into `buffer`, so the comment "we didn't parse
anything" does not hold in this case — the framing layer did consume a record.
`input.AdvanceTo(buffer.Start, buffer.End)` therefore consumes nothing and marks
everything examined. The next `ReadAsync` waits for more data and then re-parses the
same unknown message, with the same result, indefinitely. No further message on that
connection is ever dispatched.
### Why this is the default path
`MaximumReceiveMessageSize` defaults to 32 KB
(`src/SignalR/server/Core/src/HubOptionsSetup.cs:21`), so the affected branch is the
one almost all apps take.
### Impact
Correctness and forward compatibility, not security. A client can only wedge its own
connection, and buffered data is bounded by transport backpressure
(`TransportMaxBufferSize`, 64 KB default), so there is no unbounded growth. The
practical consequence is that the documented intent fails: a newer client that sends
a newly-added message type wedges an older server instead of having that message
ignored.
### Suggested fix
In the `else` branch, distinguish "framing incomplete" (buffer genuinely unchanged —
break and wait for more data) from "record consumed but message ignored" (advance
`buffer` to `segment.Start` and continue). For example, only `break` when
`segment.Start` equals the current `buffer.Start`; otherwise advance and loop.
### Verification status
Source-level trace, not an executed repro. Building this servicing branch requires
the runtime toolset `10.0.13-servicing.26454.107`, which is not available on the
public feed, so `restore.sh` cannot complete outside Microsoft-internal
infrastructure. Noting that explicitly rather than implying I observed the stall at
runtime. The asymmetry between the two paths is, I think, clear enough in source to
be worth your look regardless.
Contributor guide
Research direction
Start with HubConnectionHandler.DispatchMessagesAsync and compare its unlimited and size-limited paths, then read TryParseMessage in JsonHubProtocol.cs and MessagePackHubProtocolWorker.cs alongside HubOptionsSetup.cs. Verify the default 32 KB configuration with an unknown message followed by a valid one; done means the unknown record is skipped and subsequent messages are dispatched without disabling the size limit.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- backend-api-design
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 76/100