Closed
Bug 922245
Opened 11 years ago
Closed 11 years ago
SDP longer than 4k in length is truncated, causing errors
Categories
(Core :: WebRTC: Signaling, defect)
Tracking
()
RESOLVED
FIXED
mozilla27
People
(Reporter: abr, Assigned: abr)
Details
Attachments
(2 files)
9.34 KB,
text/html
|
Details | |
10.70 KB,
patch
|
ehugg
:
review+
|
Details | Diff | Splinter Review |
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).
Assignee | ||
Comment 1•11 years ago
|
||
Assignee | ||
Comment 2•11 years ago
|
||
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....
Assignee | ||
Comment 3•11 years ago
|
||
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.
Comment 4•11 years ago
|
||
See bug 767380 and bug 798873.
Assignee | ||
Comment 5•11 years ago
|
||
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → adam
Status: NEW → ASSIGNED
Assignee | ||
Updated•11 years ago
|
Attachment #812272 -
Flags: review?(ethanhugg)
Comment 6•11 years ago
|
||
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+
Assignee | ||
Comment 7•11 years ago
|
||
Comment 8•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
You need to log in
before you can comment on or make changes to this bug.
Description
•