-
Notifications
You must be signed in to change notification settings - Fork 217
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Twilio advertises incorrect stun urls #1934
Comments
Hi @fippo, thanks for surfacing this one. We are going to notify customers of the upcoming URL change and plan to make this change shortly before Chrome 110 is released. |
Hi @seancoleman2, do you have an update on this issue? Chrome 110 is scheduled for release on Feb 1, 2023. Without a fix, Twilio's STUN and TURN service will break for Chrome 110 and later. |
Hi @markbackman - the change needed to the Network Traversal Service will be deployed on Jan 31st, before Chrome 110 early stable release goes out on Feb 1st. An email will be sent out to NTS customers with more details tomorrow. Note for others following:
|
No-one else feel like this is too short a timeframe to be rolling this out in? I got an email today and the rollout is at the end of the month. What is the harm in Chrome silently allowing completely benign parameters to exist in the URLs? The review ( https://chromium-review.googlesource.com/c/chromium/src/+/4037887 ) looks to have barely any discussion, besides someone mentioning it could break twilio and ending up here. Not sure I fully understand why this change needs to happen, ... at all really. |
@AeroNotix admittedly this issue was where we first became aware of the Chrome 110 change's impact on the NTS service, so I do understand the timeframe is far from ideal. We were hoping that it wouldn't impact many, if any, customers since the token endpoint is intended to take the response values as is. With that being said, we feared some customers could be parsing the query params from the URIs and that's why we wanted to send an email. Curious, will this update require a change on your end? I also cannot speak to the reasoning/urgency of this change from Chrome besides what is in PR you linked. Maybe @fippo can chime in there. |
@seancoleman2 oh please do not take the fact I am commenting here to mean that I lay any blame or ownership on yourself, or twilio in general. I am commenting here because I know how absolutely futile it would be to try to discuss with Googlers on their own forum. Hence starting a conversation here. |
The change was intended as a low-priority fix for a set of failing spec compliance tests. Nobody knew upfront that https://xkcd.com/1172/ would apply. @AeroNotix talking with Googlers instead of about them gives amazing results. |
Are you joking?
Beg to differ. This isn't my first rodeo. |
For the record, none of my systems were affected by this in a negative way. I just think the poor planning and rollout is just so indicative of Googlers it needed more attention than it seemed to be getting. |
We also got this email today. We're using Conversations SDK and Programmable Video SDK on client side and Twilio Node SDK on the backend side. We're not doing any direct API calls to Twilio or doing any interpretation of above mentioned query param ourselves. So do we assume that we'll not be affected? |
Hi, thanks for commenting on this thread, the discussion has been very useful. |
@harshit-j if you are not using the Network Traversal Service, then you should not have gotten an email (could have been a mistake on our end) and do not need to worry about this change. One nuance is that if you are using the RTC Diagnostics SDK, you may be gathering NTS credentials to run the @carl-meyer-paxton here are the contents of the email:
|
One additional update. To provide an option to test the change / verify no impact before the 31st in a pre-production environment, we are going to deploy this behind a feature flag next week (default disabled) and can enable it on a per-account SID basis. If you are interested, email me at scoleman at twilio.com with account SID(s) that you want to test it on (ie. your dev/staging environments) and I will enable the flag for you. It's currently in our QE process, but we are targeting next Wednesday (1/25) to make this available. Again, we believe it's very unlikely there will be any impact given the nature of the service + what is outlined in the documentation but we can't be 100% certain. |
That's very forward-thinking @seancoleman2. Interesting how a different billion-dollar company couldn't come up with the same thing 🤷🏻. Will reach out via email! |
Google recommends that application vendors and concerned users test with canary and beta versions of Chrome. |
Closing this issue as we deployed this change yesterday. Thanks all for the help! |
just uninstall and reinstall your simple-peer library to the latest one in my case it was 9.11.1 |
Remove query param for transport in twilio - twilio/twilio-video.js#1934
@makarandp0 @manjeshbhargav please route to the right team
https://www.twilio.com/docs/stun-turn shows a STUN server with a query component:
and I have seen this in a recent webrtc-internals dump as well.
This is not allowed in stun urls per
https://www.rfc-editor.org/rfc/rfc7064#section-3.1
This will
likelybreak in Chrome 110 (February 2023) whenhttps://chromiumdash.appspot.com/commits?commit=f4d463ca875f51c26a7de0e594eb1516f3e60111&platform=Windows
ships.
While you are updating the docs please also remove the
.url
which was removed from the spec prior to 2016The text was updated successfully, but these errors were encountered: