1353028 - Nightly users can't see/hear stream on Talky.io

Status: VERIFIED FIXED

(Core :: WebRTC, defect, P1)

Description

[Paul Silaghi, QA [:pauly]]

[Affected versions]:
- 55.0a1
- works fine on 54.0a2, 53b8

[Affected platforms]:
- all

[Steps to reproduce]:
1. Launch Firefox.
2. Go to https://talky.io/ and start a meeting with an other user.

[Expected result]:
- The meeting starts and audio/video work properly

[Actual result]:
- The meeting starts, but the Nightly user is not able to see or hear the other user.
Browser console log:
"turn2.talky.io:443 uses an invalid security certificate.

The certificate is only valid for the following names:
  anon.talky.me, www.anon.talky.me  
The certificate expired on June 25, 2016 at 2:59 AM. The current time is April 3, 2017 at 5:42 PM.

Error code: <a id="errorCode" title="SSL_ERROR_BAD_CERT_DOMAIN">SSL_ERROR_BAD_CERT_DOMAIN</a>"

[Regression range]:
- recent regression, first bad nightly: 2017-03-29

Comment 1

[Paul Silaghi, QA [:pauly]]

pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=235e1d29916fc30848b796ef9b416ec3932a743b&tochange=8efb75a46b6204eb5c4f1f3e1f3b20226b0981ab

Comment 2

[Philipp Hancke [:fippo]]

Good catch! 

This is caused by FF55 no longer accepting empty lines in the SDP of the setRemoteDescription call. Those empty lines appear after every a=candidate line on talky.

I've pinged some folks who still work on talky to fix that (also to get that certificate fixed)

Comment 3

[Byron Campen [:bwc]]

(In reply to Philipp Hancke from comment #2)
> Good catch! 
> 
> This is caused by FF55 no longer accepting empty lines in the SDP of the
> setRemoteDescription call. Those empty lines appear after every a=candidate
> line on talky.
> 
> I've pinged some folks who still work on talky to fix that (also to get that
> certificate fixed)

   That's _really_ weird. I'm not seeing anything in that push that should have affected the basic SDP parsing logic...

Comment 4

[Nils Ohlmeier [:drno]]

I would be interested in finding the commit which caused this. Although I'm pretty sure Ekr pointed out way before 55 that we should ignore empty lines in SDP, even if they are not spec compliant.

Comment 5

[Philipp Hancke [:fippo]]

this is likely caused by an extra CRLF before addIceCandidate: -- the fix for talky is here: 
https://github.com/otalk/RTCPeerConnection/commit/76f64f23cece342709b6492faa68850a8ccd7adf

Comment 6

[Byron Campen [:bwc]]

I'm pretty sure this has nothing to do with what is really going on. Something else is busted here.

Comment 7

[Byron Campen [:bwc]]

I think I see a flaw in the extmap validation code.

Comment 8

[Philipp Hancke [:fippo]]

repro fiddle here: https://jsfiddle.net/8gegpqax/5/

Comment 9

[Byron Campen [:bwc]]

Attachment: Bug 1353028: Fix broken extmap validation logic.

Review commit: https://reviewboard.mozilla.org/r/126150/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/126150/

Comment 10

[Nils Ohlmeier [:drno]]

Comment on attachment 8854198 [details]
Bug 1353028: Fix broken extmap validation logic.

https://reviewboard.mozilla.org/r/126150/#review128754

Much more readable with the reverse() instead of ~

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9a66f1e840d0
Fix broken extmap validation logic. r=drno

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9a66f1e840d0

Comment 13

[Paul Silaghi, QA [:pauly]]

Verified fixed Fx 55.0a1 (2017-04-06) on a call between Win 10 - OS X 10.11.