For the origin ("o=") line of the SDP o=<username> <sess-id> <sess-version> <nettype> <addrtype> <unicast-address> RFC 3264 says: 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. RFC 4566 defines both values as: sess-id = 1*DIGIT ;should be unique for this username/host sess-version = 1*DIGIT According to the ABNF grammar defined in RFC 5234: 1*DIGIT means at least one ([1-inf]) numeric characters [0-9]. So, this means this values should be an integer value >= 0. The combination says only values from 0 - 2^63-1 pass both tests for the session-id and values from 0 - 2^62-1 for the version. Therefore I understand the following: - We should use int64_t for representing both values (sess-id and sess-version). - The session-id value generated must be an integer inside [0 - 2^63-1] - The version value generated must be an integer inside [0 - 2^62-1] Right now we are using uint_64t for the session id, and for the initial session id value we generate a random number that sometimes exceeds LLONG_MAX. And when that happens firefox rejects our SDP offer. I observed that firefox gives this fatal error on AppRTC when we generate a session-id bigger than LLONG_MAX setRemoteDescription: InvalidSessionDescriptionError: Failed to parse SDP: SDP Parse Error on line 2: Invalid owner session id specified for o=. References: https://bugzilla.mozilla.org/show_bug.cgi?id=861895#c5 https://bugs.chromium.org/p/webrtc/issues/detail?id=1628#c8
Created attachment 304523 [details] Patch
This seems like something that should be possible to test for, right?
Created attachment 304570 [details] Patch Add a check for the sess-id value on the tests
Comment on attachment 304570 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=304570&action=review Good catch! LGTM > LayoutTests/fast/mediastream/resources/sdp-utils.js:32 > + if (sessid > 0 && sessid <= 9223372036854775807) Can we comment what that number is or add it to a constant somewhere that explains?
Committed r214037: <http://trac.webkit.org/changeset/214037>