dotnet / dotnet/aspnetcore

[SignalR] Unknown message type wedges the connection when MaximumReceiveMessageSize is set (the default)

Open
#69,285 0 comments 0 reactions 0 assignees View on GitHub
area-signalr
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.