Skip to content

Reject oversized data messages before they break the data channel - #1137

Merged
ladvoc merged 5 commits into
mainfrom
datamessage_size
Jun 8, 2026
Merged

Reject oversized data messages before they break the data channel#1137
ladvoc merged 5 commits into
mainfrom
datamessage_size

Conversation

@cnderrauber

Copy link
Copy Markdown
Contributor

Sending a data message larger than the negotiated SCTP max message size made libwebrtc abruptly close the publisher data channel. Because DataChannelInterface::Send returns true regardless, publish_data() returned Ok while the channel was silently torn down; every subsequent publish then failed with "could not establish publisher connection: timeout" (15s each) since the channel is never recreated once has_published is set.

  • Parse the negotiated max message size from the publisher answer SDP (RFC 8841 a=max-message-size), defaulting to 65535 when absent.
  • Reject packets whose encoded size exceeds it in publish_data(), returning an error to the caller instead of silently aborting the channel. A value of 0 means "no limit".
  • Log an info when a publisher data channel closes while the session and peer connection are still up (the signature of this failure), without attempting renegotiation.

Sending a data message larger than the negotiated SCTP max message size
made libwebrtc abruptly close the publisher data channel. Because
DataChannelInterface::Send returns true regardless, publish_data() returned
Ok while the channel was silently torn down; every subsequent publish then
failed with "could not establish publisher connection: timeout" (15s each)
since the channel is never recreated once has_published is set.

- Parse the negotiated max message size from the publisher answer SDP
  (RFC 8841 a=max-message-size), defaulting to 65535 when absent.
- Reject packets whose encoded size exceeds it in publish_data(), returning
  an error to the caller instead of silently aborting the channel. A value
  of 0 means "no limit".
- Log an info when a publisher data channel closes while the session and
  peer connection are still up (the signature of this failure), without
  attempting renegotiation.
@cnderrauber
cnderrauber requested a review from ladvoc as a code owner June 5, 2026 10:43
@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Changeset

The following package versions will be affected by this PR:

Package Bump
livekit patch
livekit-ffi patch

Comment thread livekit/src/rtc_engine/rtc_session.rs Outdated
return;
};
if !inner.closed.load(Ordering::Acquire) && inner.publisher_pc.is_connected() {
log::info!(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

suggestion: This should probably be logged at error level.

Comment thread livekit/src/rtc_engine/rtc_session.rs Outdated
// Log when a publisher data channel closes without the engine or peer
// connection tearing it down
for (dc, label) in
[(&inner.reliable_dc, RELIABLE_DC_LABEL), (&inner.lossy_dc, LOSSY_DC_LABEL)]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

suggestion: For completeness, this should probably also track the data track DC.

pblazej added a commit to livekit/client-sdk-swift that referenced this pull request Jun 8, 2026
…nnel

Sending more than the negotiated SCTP max-message-size makes libwebrtc
return success from `sendData` and then abruptly tear down the publisher
data channel; every subsequent publish then fails until the next full
reconnect. Mirrors the fix from livekit/rust-sdks#1137.

Parse `a=max-message-size` (RFC 8841) from the publisher answer SDP in
`Room+SignalClientDelegate.didReceiveAnswer`, clamp to a 64000-byte
default, and enforce the cap in `DataChannelPair.makeRequest` after
serialization — the encoded byte count is what actually goes on the
wire. A value of 0 means "no limit". Also logs an error when the
publisher channel transitions to `.closed` without an SDK-initiated
reset, surfacing the remaining failure modes for diagnosis.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pblazej added a commit to livekit/client-sdk-swift that referenced this pull request Jun 8, 2026
…nnel

Sending more than the negotiated SCTP max-message-size makes libwebrtc
return success from `sendData` and then abruptly tear down the publisher
data channel; every subsequent publish then fails until the next full
reconnect. Mirrors the fix from livekit/rust-sdks#1137.

Parse `a=max-message-size` (RFC 8841) from the publisher answer SDP in
`Room+SignalClientDelegate.didReceiveAnswer`, clamp to a 64000-byte
default, and enforce the cap in `DataChannelPair.makeRequest` after
serialization — the encoded byte count is what actually goes on the
wire. A value of 0 means "no limit". Also logs an error when the
publisher channel transitions to `.closed` without an SDK-initiated
reset, surfacing the remaining failure modes for diagnosis.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@xianshijing-lk

Copy link
Copy Markdown
Contributor

@1egoman , could you please review this PR ?

@1egoman 1egoman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Makes sense to me!

@ladvoc
ladvoc merged commit c675112 into main Jun 8, 2026
22 checks passed
@ladvoc
ladvoc deleted the datamessage_size branch June 8, 2026 18:34
pblazej added a commit to livekit/client-sdk-swift that referenced this pull request Jun 9, 2026
…nnel (#1031)

## Summary

Sending more than the negotiated SCTP max-message-size makes libwebrtc
return success from `sendData` and then abruptly tear down the publisher
data channel — every subsequent publish fails until a full reconnect.

- Parse `a=max-message-size` (RFC 8841) from the publisher answer SDP,
clamp to a 64000-byte default.
- Reject in `DataChannelPair.makeRequest` after serialization (encoded
byte count = what goes on the wire). `0` means "no limit".
- Log an error when the publisher channel transitions to `.closed`
without an SDK-initiated reset.

Ports the fix from
[livekit/rust-sdks#1137](livekit/rust-sdks#1137).
Android does the simpler hardcoded-constant variant at
[`LocalParticipant.kt:984-986`](https://github.com/livekit/client-sdk-android/blob/46da6784bec2ac124972738764a8e18702a78ee3/livekit-android-sdk/src/main/java/io/livekit/android/room/participant/LocalParticipant.kt#L984-L986).

## Test plan

- [x] Unit tests: parameterized SDP parser (6 cases) + size-guard tests
(`oversizedSendRejectedWithInvalidParameter`,
`zeroLimitDisablesTheSizeGuard`)
- [x] `xcodebuild build` macOS clean
- [x] SwiftLint clean
- [ ] CI matrix

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

4 participants