Bug 159207 – WebRTC: ice-char can not contain '=' characters for credentials

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Alejandro G. Castro]

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.

[Alejandro G. Castro]

Created attachment 282242 [details]
Patch

[Adam Bergkvist]

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

[Alejandro G. Castro]

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

[Alejandro G. Castro]

Created attachment 282333 [details]
Patch

[Eric Carlson]

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

[Alejandro G. Castro]

Created attachment 282355 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 282355 [details]
Patch for landing

Clearing flags on attachment: 282355

Committed r202628: <http://trac.webkit.org/changeset/202628>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.