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

Status: RESOLVED FIXED

(Core :: WebRTC, defect, P2)

Description

[Peter Nagy]

Attachment: 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.

Comment 1

[Peter Nagy]

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.

Comment 2

[Matthias Versen [:Matti]]

Please add a better general description about your issue before going into the details. I guess that you are talking about a webrtc connection...

Comment 3

[Jason Smith [:jsmith]]

Can you give me a URL or test case I could reference where you got this error?

Comment 4

[Jason Smith [:jsmith]]

(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.

Comment 5

[Jason Smith [:jsmith]]

Not blocking until we get a test case or URL to test against to understand the impact of the bug.

Comment 6

[Randell Jesup [:jesup] (needinfo me)]

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.

Comment 7

[Peter Nagy]

Thank you for the fast answer. I will report this bug in the chrome's issue tracker.

Comment 8

[Peter Nagy]

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.

Comment 9

[Randell Jesup [:jesup] (needinfo me)]

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

Comment 10

[Ethan Hugg [:ehugg]]

Attachment: Signaling allow 63bit session ids

Comment 11

[Randell Jesup [:jesup] (needinfo me)]

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

Comment 12

[Ethan Hugg [:ehugg]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f272d2441a3b

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f272d2441a3b

Comment 14

[Randell Jesup [:jesup] (needinfo me)]

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

Comment 15

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/2dbc6565f6d6