Closed Bug 859303 Opened 8 years ago Closed 8 years ago

Cannot set remoteDescription because the session id is 63-bit long in the o= line of the sdp

Categories

(Core :: WebRTC, defect, P2)

23 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla22
Tracking Status
firefox22 --- fixed

People

(Reporter: peternagyxx, Assigned: ehugg)

References

Details

(Whiteboard: [WebRTC][blocking-webrtc-][qa-])

Attachments

(2 files)

Attached file sdps.txt
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.31 (KHTML, like Gecko) Chrome/26.0.1410.43 Safari/537.31

Steps to reproduce:

I tried to create peer connection between Firefox nightly 23.0a1 (2013-04-07)
and Google Chrome 28.0.1468.0 canary.



Actual results:

The connection can't be established, and I got the following exception: 

Object { name= "rw", message="rw"}
message: "Unspecified Error processing setRemoteDescription"
name: "INTERNAL_ERROR"



Expected results:

The connection has created.
It has worked with previous versions of chrome. Now I tried to workaround it and it is succeed when I decrease the length of the session id half of its size. The reason maybe that the session id must be in the NTP fromat according to the RFC and that format can contain 64-bit long id.
Please add a better general description about your issue before going into the details. I guess that you are talking about a webrtc connection...
Component: Untriaged → WebRTC
Product: Firefox → Core
QA Contact: jsmith
Can you give me a URL or test case I could reference where you got this error?
Flags: needinfo?(peternagyxx)
(In reply to Matthias Versen [:Matti] from comment #2)
> Please add a better general description about your issue before going into
> the details. I guess that you are talking about a webrtc connection...

Yup, that's right. The bug is in the right spot here - he's talking about session descriptions with the peer connection handshakes.
Not blocking until we get a test case or URL to test against to understand the impact of the bug.
Whiteboard: [WebRTC][blocking-webrtc-]
RFC 3264 (Offer/Answer) says this:
   The numeric value of the session id
   and version in the o line MUST be representable with a 64 bit signed
   integer.  The initial value of the version MUST be less than
   (2**62)-1, to avoid rollovers.

So a 63-bit sess-id is invalid (though it's valid SDP in general, it's not valid for an offer/answer SDP).  The signaling code in Mozilla enforces this limit (sdp_token.c).  If Chrome accepts it they're in a gray area (accepting invalid SDP); if they generate a 63-bit o= line they're wrong.  There is actually a reason for this limit; it's not totally arbitrary.
Status: UNCONFIRMED → RESOLVED
Closed: 8 years ago
Resolution: --- → INVALID
Flags: needinfo?(peternagyxx)
Thank you for the fast answer. I will report this bug in the chrome's issue tracker.
I looked at it again. As I understand the version and the session id must be representable with a 64-bit signed integer separately. So it's ok.

"The initial value of the __version__ MUST be less than (2**62)-1, to avoid rollovers." The version is ok in the answer of chrome, it's 2. This constraint must be applied to the version not to the session id.
Status: RESOLVED → UNCONFIRMED
Resolution: INVALID → ---
Aha.  Thanks, totally glossed over that subtlety.

Looks like the code in sdp_token.c is using max_value_sessid_version (2^62-1) for both sessid and version, instead of just version.

-> ehugg

Interop-with-chrome issue
Assignee: nobody → ethanhugg
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 7 → All
Priority: -- → P2
Hardware: x86_64 → All
Whiteboard: [WebRTC][blocking-webrtc-] → [WebRTC][blocking-webrtc-][webrtc-uplift]
Target Milestone: --- → mozilla22
Attachment #735202 - Flags: review?(rjesup)
Comment on attachment 735202 [details] [diff] [review]
Signaling allow 63bit session ids

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

r+; feel free to const them

::: media/webrtc/signaling/src/sipcc/core/sdp/sdp_token.c
@@ +92,5 @@
>          integer.  The initial value of the version MUST be less than
>          (2**62)-1, to avoid rollovers.
>      */
> +    uint64_t     max_value_sessid = ((((uint64_t) 1) << 63) - 1);
> +    uint64_t     max_value_version = ((((uint64_t) 1) << 62) - 2);

While not necessary, these are really const
Attachment #735202 - Flags: review?(rjesup) → review+
https://hg.mozilla.org/mozilla-central/rev/f272d2441a3b
Status: NEW → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Target Milestone: mozilla22 → mozilla23
Whiteboard: [WebRTC][blocking-webrtc-][webrtc-uplift] → [WebRTC][blocking-webrtc-][webrtc-uplift][qa-]
Comment on attachment 735202 [details] [diff] [review]
Signaling allow 63bit session ids

[Approval Request Comment]
Bug caused by (feature/regressing bug #): n/a

User impact if declined: Interoperability problems with 3rd-party webrtc clients that comply with the SDP spec but use all allowed 63 bits of o= value.

Testing completed (on m-c, etc.): Tested by original reporter.  On m-c.

Risk to taking this patch (and alternatives if risky): nil

String or IDL/UUID changes made by this patch: none
Attachment #735202 - Flags: approval-mozilla-aurora?
Attachment #735202 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/2dbc6565f6d6
Whiteboard: [WebRTC][blocking-webrtc-][webrtc-uplift][qa-] → [WebRTC][blocking-webrtc-][qa-]
Target Milestone: mozilla23 → mozilla22
Duplicate of this bug: 862094
You need to log in before you can comment on or make changes to this bug.