Closed Bug 922245 Opened 7 years ago Closed 7 years ago

SDP longer than 4k in length is truncated, causing errors

Categories

(Core :: WebRTC: Signaling, defect)

Other Branch
x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla27

People

(Reporter: abr, Assigned: abr)

Details

Attachments

(2 files)

Currently, our RTCSessionDescription object is defined like this:

> interface mozRTCSessionDescription {
>   attribute RTCSdpType? type;
>   attribute DOMString? sdp;
> 
>   jsonifier;
>};

It would appear that Firefox has a restriction on DOMString that limits its total length to 4k. This is much smaller than SDP can realistically get. When a large SDP is passed through this interface, the SDP is truncated, and becomes syntactically incorrect (causing parse problems further down in the WebRTC stack).
A little poking around with unit tests, and the problem appears to be lower than the webidl -- I can get the SDP to truncate at ~4k even without any browser involved. The problem is likely in the SIPCC code; still digging....
And there it is. ccapi.h defines SDP_SIZE to be 4096, and cc_feature_t allocates a "char sdp[SDP_SIZE]". The quick and easy fix is to bump the constant up. I think the more correct fix would be to dynamically allocate the buffer. I'm going to investigate how much work dynamic allocation would take.
Assignee: nobody → adam
Status: NEW → ASSIGNED
Attachment #812272 - Flags: review?(ethanhugg)
Comment on attachment 812272 [details] [diff] [review]
Make SDP buffer allocation dynamic in feature message

Review of attachment 812272 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good.  Build and worked for me.

::: media/webrtc/signaling/src/sipcc/core/gsm/ccapi.c
@@ +1221,1 @@
>      }

I would be inclined to put an else { pmsg->sdp = NULL; } here.  I know that cc_get_msg_buf currently zeros out the struct, but I think there might be value in making it explicit.  The cpr_free of this above relies on it being NULL when not malloced here.  Your call.
Attachment #812272 - Flags: review?(ethanhugg) → review+
https://hg.mozilla.org/mozilla-central/rev/3b0cd8c4e0a4
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
You need to log in before you can comment on or make changes to this bug.