Bug 159207

Summary: WebRTC: ice-char can not contain '=' characters for credentials
Product: WebKit Reporter: Alejandro G. Castro <alex>
Component: WebCore Misc.Assignee: Alejandro G. Castro <alex>
Status: RESOLVED FIXED    
Severity: Normal CC: adam.bergkvist, commit-queue, eric.carlson
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 143211    
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Description Alejandro G. Castro 2016-06-28 03:20:14 PDT
We are generating a wrong pass string because we are using base64 random string with padding, and paddings are filled with '=' characters, it is a problem because the sdp parser does not work corrently in this cases and we are not sending good credentials. The parser in the test we have was modified to avoid this issue.
Comment 1 Alejandro G. Castro 2016-06-28 04:13:31 PDT
Created attachment 282242 [details]
Patch
Comment 2 Adam Bergkvist 2016-06-28 11:10:43 PDT
Comment on attachment 282242 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=282242&action=review

> Source/WebCore/Modules/mediastream/MediaEndpointPeerConnection.cpp:75
> +    , m_icePassword(randomString(24)) // From 22 to 256 ice-chars, we use base64 to generate we need to avoid padding.

Feels like a word is missing to make these sentences complete
Comment 3 Alejandro G. Castro 2016-06-29 01:42:55 PDT
(In reply to comment #2)
> Comment on attachment 282242 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=282242&action=review
> 
> > Source/WebCore/Modules/mediastream/MediaEndpointPeerConnection.cpp:75
> > +    , m_icePassword(randomString(24)) // From 22 to 256 ice-chars, we use base64 to generate we need to avoid padding.
> 
> Feels like a word is missing to make these sentences complete

Right, I'll fix it, thanks for the review!
Comment 4 Alejandro G. Castro 2016-06-29 02:02:57 PDT
Created attachment 282333 [details]
Patch
Comment 5 Eric Carlson 2016-06-29 08:17:55 PDT
Comment on attachment 282333 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=282333&action=review

> Source/WebCore/ChangeLog:8
> +        Avoided the general calculation to get a base64 without padding

Nit: "Avoided the general calculation..." -> "Avoid a general calculation..."

> Source/WebCore/ChangeLog:9
> +        which was wrong in the randomString function. Each parameter using

Nit: "Each parameter using..." -> "Because each parameter using..."

> Source/WebCore/ChangeLog:16
> +        Covered by existing test modified to match the correct behavior.

Nit: "Covered by existing test modified to..." -> "Existing test modified to..."
Comment 6 Alejandro G. Castro 2016-06-29 10:06:47 PDT
Created attachment 282355 [details]
Patch for landing
Comment 7 WebKit Commit Bot 2016-06-29 10:36:55 PDT
Comment on attachment 282355 [details]
Patch for landing

Clearing flags on attachment: 282355

Committed r202628: <http://trac.webkit.org/changeset/202628>
Comment 8 WebKit Commit Bot 2016-06-29 10:36:59 PDT
All reviewed patches have been landed.  Closing bug.