Closed Bug 844929 Opened 8 years ago Closed 8 years ago

Accept numeric parameters > UINT_MAX in SDP o-lines

Categories

(Core :: WebRTC: Signaling, defect, P2)

x86
macOS
defect

Tracking

()

RESOLVED FIXED
mozilla22

People

(Reporter: ekr, Assigned: ehugg)

Details

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

Attachments

(1 file, 4 obsolete files)

No description provided.
Attached patch Allow SDP session ids > MAX_UINT (obsolete) — Splinter Review
Assignee: nobody → ekr
Attached patch Allow SDP session ids > MAX_UINT (obsolete) — Splinter Review
Attachment #717948 - Attachment is obsolete: true
Attached patch Allow SDP session ids > MAX_UINT (obsolete) — Splinter Review
Attachment #717996 - Attachment is obsolete: true
Attachment #718003 - Flags: review?(ethanhugg)
Comment on attachment 718003 [details] [diff] [review]
Allow SDP session ids > MAX_UINT

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

::: media/webrtc/signaling/src/sipcc/core/sdp/sdp_token.c
@@ +65,5 @@
> +
> +    if (ptr[end] == '\0')
> +	return SDP_SUCCESS;
> +
> +    return SDP_INVALID_PARAMETER;

The spec 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.

Should we be doing a strtoul instead and checking against this max value here?
Attachment #718003 - Flags: review?(ethanhugg) → review+
(In reply to Ethan Hugg [:ehugg] from comment #4)
> Should we be doing a strtoul instead and checking against this max value
> here?

A long is 32 bits on Windows. That is one of the many problems this patch is fixing. I'm not against being stricter in what we accept, but it would probably mean writing our own string->int64 function (not that this is difficult, since we don't really care about speed).
Priority: -- → P2
Whiteboard: [webrtc][blocking-webrtc+]
Since this is SIPCC, we can use PR_sscanf, which seems to support PRUint64.

Can you take this?
Assignee: ekr → ethanhugg
Attachment #718003 - Attachment is obsolete: true
Attachment #718495 - Flags: review?(ekr)
Comment on attachment 718495 [details] [diff] [review]
Accept numeric parameters > UINT_MAX in SDP o-lines

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

Would it make sense to make these strings by string concatenation so we don't have so much cut-and-paste.
Attachment #718495 - Flags: review?(ekr) → review+
Attachment #718495 - Attachment is obsolete: true
Attachment #718515 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/a1d01526d344
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Flags: in-testsuite+
Whiteboard: [webrtc][blocking-webrtc+] → [webrtc][blocking-webrtc+][qa-]
You need to log in before you can comment on or make changes to this bug.