Closed Bug 1353028 Opened 3 years ago Closed 3 years ago

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

Categories

(Core :: WebRTC, defect, P1)

55 Branch
defect

Tracking

()

VERIFIED FIXED
mozilla55
Tracking Status
firefox52 --- unaffected
firefox-esr52 --- unaffected
firefox53 --- unaffected
firefox54 --- unaffected
firefox55 --- verified

People

(Reporter: pauly, Assigned: bwc)

References

Details

(Keywords: regression)

Attachments

(1 file)

[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
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)
(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...
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.
this is likely caused by an extra CRLF before addIceCandidate: -- the fix for talky is here: 
https://github.com/otalk/RTCPeerConnection/commit/76f64f23cece342709b6492faa68850a8ccd7adf
I'm pretty sure this has nothing to do with what is really going on. Something else is busted here.
I think I see a flaw in the extmap validation code.
Rank: 15
Priority: -- → P1
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 ~
Attachment #8854198 - Flags: review?(drno) → review+
Assignee: nobody → docfaraday
Pushed by bcampen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9a66f1e840d0
Fix broken extmap validation logic. r=drno
https://hg.mozilla.org/mozilla-central/rev/9a66f1e840d0
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Verified fixed Fx 55.0a1 (2017-04-06) on a call between Win 10 - OS X 10.11.
Status: RESOLVED → VERIFIED
Blocks: 1355010
You need to log in before you can comment on or make changes to this bug.