-
Notifications
You must be signed in to change notification settings - Fork 95
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
Move URI type from pion/ice to pion/stun #134
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #134 +/- ##
===========================================
- Coverage 98.16% 58.21% -39.95%
===========================================
Files 19 18 -1
Lines 1577 1716 +139
===========================================
- Hits 1548 999 -549
- Misses 23 684 +661
- Partials 6 33 +27
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
8bb4109
to
73f2585
Compare
c5c6a03
to
627cdbc
Compare
This change currently breaks the API of the module. As we are still with v0, I dont think this is a huge deal. The breaking changes are also really minor.. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First pass done. Basically LGTM except for some potential style changes!
This makes it easier for users of pion/stun or pion/turn packages to handle URLs. Next step, would be to remove the URL type from pion/ice
Depending on the schema and transport, this function establishes also encrypted connections via TLS/DTLS.
Until now, TLS and DTLS backed STUN clients did not use a custom transport.Net.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR moves the URL type from the pion/ice module to pion/stun and renames it to URI.
Hence this module gains full support for RFC7064 and RFC7065 (see #65).
The PR also adds a new function
stun.DialURI(u *stun.URI, cfg *stun.DialConfig)
which create the underlying connection for the client for you based on the URI Scheme and transport protocol. Hence it can also use thedtls
andtls
modules to establish an encrypted connection.Currently, the connection establishment is done by hand in various places:
This is also related to #35 (comment)
Furthermore, we now fully support passing a custom
transport.Net
for testing which was previously not supported for TLS / DTLS connections.