Only collect single fingerprints and ICE credentials #2950
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2950 +/- ##
===========================================
- Coverage 77.74% 63.56% -14.18%
===========================================
Files 89 67 -22
Lines 10462 3928 -6534
===========================================
- Hits 8134 2497 -5637
+ Misses 1840 1305 -535
+ Partials 488 126 -362
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Amazing! I can fix lint issues, you can reproduce them locally using ‘golangci-lint run’ |
|
I will merge tonight! |
|
@Sean-Der should I put the errors back in, to avoid a breaking change (even if the errors are no longer used)? |
|
@nils-ohlmeier yea mind putting them back :( downstream users I bet are using them. Breaks always hurt |
|
Done. I think this should be ready for review now. @Sean-Der is there a way to flag the errors as deprecated, since they aren't used any more? Should this get mentioned in release notes to minimize surprises? |
Sean-Der
force-pushed
the
make_ufrag_spec_compliant
branch
from
860d961
to
9479a4d
Compare
|
@nils-ohlmeier lets get this merged :) I will squash and fix lints! Thank you so much for taking this on, not an easy first bug! |
|
Wow nice work, I learned a lot from your explanation 🫡 |
The way currently DTLS fingerprints and ICE credentials are picked is causing interop issues as described in pion#2621 Peers which don't use Bundle can use different fingerprints and credentials in each media section. Even though is not (yet) supported by Pion, receiving an SDP offer from such a peer is valid. Additionally if Bundle is being used the group attribute determines which media section is the master bundle section, which establishes the transport. Currently Pion always just uses the first credentials/fingerprint it can find in the SDP, which results in not spec compliant behavior. This PR attempts to fix the above issues and make Pion more spec compliant and interoperable. Fixes pion#2621
Sean-Der
force-pushed
the
make_ufrag_spec_compliant
branch
from
9479a4d
to
e071119
Compare
Description
The way currently DTLS fingerprints and ICE credentials are picked is causing interop issues as described in #2621
Peers which don't use Bundle can use different fingerprints and credentials in each media section. Even though is not (yet) supported by Pion, receiving an SDP offer from such a peer is valid.
Additionally if Bundle is being used the group attribute determines which media section is the master bundle section, which establishes the transport. Currently Pion always just uses the first credentials/fingerprint it can find in the SDP, which results in not spec compliant behavior.
This PR attempts to fix the above issues and make Pion more spec compliant and interoperable.
Reference issue
Fixes #2621