Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
support juice_bind_stun to reflect stun requests from unbound client. #248
base: master
Are you sure you want to change the base?
support juice_bind_stun to reflect stun requests from unbound client. #248
Changes from 12 commits
94272e4
59957aa
06a925b
63651ed
50fa27a
5b818b1
f17129c
20755dc
9ede09b
2c155ae
4a0a896
1fb20f7
a9f5fbc
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Since they are specific to the mux implementation, they should be moved to
struct registry_impl
inconn_mux.c
instead, and you can then drop themux_
prefix in the names.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.
We can't do this at the moment because we need to not clean up the socket in release_registry when cb_mux_incoming is present, otherwise release_registry will shut down the listen socket.
If we move cb_mux_incoming to conn_mux, we need to add a method to tell release_registry not to clean up the socket, which may be more complicated.
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.
I found an issue here.
When connecting to the server using a browser and the server closes the peer connection, the browser will continue to send STUN messages, and the incoming event will be triggered again.
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.
According to the RFC document:
An agent must not include the USE-CANDIDATE attribute when sending a binding request. I tested this in practice, the binding request message received has use_candidate=0. After the connection is closed, the browser continues to send stun messages with use_candidate=1.
I will proceed to test further based on this strategy.
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.
===========================================================================
After successfully intercepting dozens of messages, the browser still stubbornly sent a binding request message with USE-CANDIDATE set to 0.
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.
I examined numerous attributes in the STUN message, but did not identify any additional characteristics that could be utilized to detect the binding request after the connection is terminated.
@paullouisageneau need your help, do you have any suggestions?
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.
I think you misunderstood what USE-CANDIDATE means. The RFC says "The controlled agent MUST NOT include the USE-CANDIDATE attribute in a Binding request". Only the controlling agent includes it to nominate a candidate pair (i.e. to tell the controlled agent which pair to use), otherwise it's not present.
To my knowledge there is no way to know the connection state from STUN attributes, since the ICE layer is unaware of upper layers.
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.
Okay, I have already canceled this commit. I cached the information of the just closed RTCPeerConnection at the business layer for a while, and blocked the same request from being accepted.