Add SetCandidateTypes to control what candidates to gather #3305
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3305 +/- ##
==========================================
+ Coverage 84.47% 84.68% +0.21%
==========================================
Files 80 80
Lines 9279 9370 +91
==========================================
+ Hits 7838 7935 +97
+ Misses 1020 1010 -10
- Partials 421 425 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
JoeTurki
force-pushed
the
with-candidates-types
branch
from
984e744 to
a76805f
Compare
JoeTurki
changed the title
JoeTurki
force-pushed
the
with-candidates-types
branch
from
a76805f to
68b7823
Compare
JoeTurki
changed the title
JoeTurki
force-pushed
the
with-candidates-types
branch
from
68b7823 to
c018a20
Compare
| if g.gatherPolicy == ICETransportPolicyRelay { | ||
| for _, candidate := range candidateTypes { | ||
| if candidate != ice.CandidateTypeRelay { | ||
| g.log.Warnf("ICETransportPolicyRelay is set. gathering is limited to relay candidates") | ||
|
|
||
| break | ||
| } | ||
| } | ||
|
|
||
| return []ice.CandidateType{ice.CandidateTypeRelay} | ||
| } | ||
|
|
||
| g.agent = agent | ||
| if g.api.settingEngine.candidates.ICELite { | ||
| return []ice.CandidateType{ice.CandidateTypeHost} | ||
| } | ||
|
|
||
| if len(candidateTypes) > 0 { | ||
| return candidateTypes | ||
| } |
There was a problem hiding this comment.
if relay mode and ice lite are enabled at the same time, we return []ice.CandidateType{ice.CandidateTypeRelay} which isn't spec compliant, but safer so we don't leak IPs if the user didn't intended to do that, we should log a warning tho.
There was a problem hiding this comment.
Is it worth commenting it in the function so it's written in the code that we intentionally "break" the spec?
There was a problem hiding this comment.
We don't break the spec, If a user turns on relay-only-mode and ice-lite at the same time, they are breaking the ice-lite spec, We should log an error when that happens, It's not a new behavior, it's how it works currently if you set both.
There was a problem hiding this comment.
I see, I'm in favor of logging if the user accidentally breaks the spec by setting both at the same time.
JoeTurki
force-pushed
the
with-candidates-types
branch
from
c018a20 to
466b458
Compare
JoeTurki
force-pushed
the
with-candidates-types
branch
from
466b458 to
0d2c610
Compare
| if g.gatherPolicy == ICETransportPolicyRelay { | ||
| for _, candidate := range candidateTypes { | ||
| if candidate != ice.CandidateTypeRelay { | ||
| g.log.Warnf("ICETransportPolicyRelay is set. gathering is limited to relay candidates") | ||
|
|
||
| break | ||
| } | ||
| } | ||
|
|
||
| return []ice.CandidateType{ice.CandidateTypeRelay} | ||
| } | ||
|
|
||
| g.agent = agent | ||
| if g.api.settingEngine.candidates.ICELite { | ||
| return []ice.CandidateType{ice.CandidateTypeHost} | ||
| } | ||
|
|
||
| if len(candidateTypes) > 0 { | ||
| return candidateTypes | ||
| } |
There was a problem hiding this comment.
Is it worth commenting it in the function so it's written in the code that we intentionally "break" the spec?
2 participants
Description
Currently, we don't have a way to limit which candidates we gather beyond lite-mode (host candidates only) and relay mode (relay only). As a result, users end up relying on hacky workarounds like #3176 to limit to srflx only, which aren't reliable due to how ICE actually works.
Added many tests to make sure that it actually works.
based on #3303.
this resolves #3176