Skip to content

Fixed typo in rtc_config_example.json #345

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

shommey
Copy link

@shommey shommey commented Oct 27, 2024

"urls": "turns::5349" changed to
"urls": "turn::5349"

This little oversight costed me a few hours of my life I'll never get back. :)

"urls": "turns:<DOMAIN>:5349" changed to
"urls": "turn:<DOMAIN>:5349"
@schlagmichdoch
Copy link
Owner

turns::5349 is the correct way if you want TURN over TLS. (See https://github.com/schlagmichdoch/PairDrop/blob/master/docs/host-your-own.md#coturn-and-pairdrop-via-docker-compose)

If you want to implement TURN without additional TLS encryption (which is fine) you should use turn::3478 as this is the standard port for STUN and TURN without TLS or change the port to whatever you have specified in your turnserver.conf. (See

# Main listening port for STUN and TURN
)

So I can't merge this PR as it is.

Maybe the turn::3478 config should be included additionally to the two existing or we should have different config files for TURN with and w/o TLS. What do you think?

Where did you take a look for the configuration? I guess this could also be solved by a simple reference of the different versions (stun:, turn:, turns:) in the docs here: https://github.com/schlagmichdoch/PairDrop/blob/master/docs/host-your-own.md#specify-stunturn-servers

@shommey
Copy link
Author

shommey commented Oct 28, 2024

I see, sorry for the mix up then.
I think the docs should mention that "s" in the "turns" stands for turn tls.
It could be great if turn: and turns: difference could be pointed out in .json itself.

@schlagmichdoch
Copy link
Owner

schlagmichdoch commented Feb 11, 2025

Sorry for the late reply. Trying to catch up with the issue built-up.

It could be great if turn: and turns: difference could be pointed out in .json itself.

How about we duplicate the turn server part and describe it in the "domain name"?

{
  "sdpSemantics": "unified-plan",
  "iceServers": [
    {
      "urls": "stun:<DOMAIN-OF-STUN-SERVER>:3478"
    },
    {
      "urls": "turn:<DOMAIN-OF-TURN-SERVER>:5349",
      "username": "username",
      "credential": "password"
    },
    {
      "urls": "turns:<DOMAIN-OF-TURN-OVER-TLS-SERVER>:5349",
      "username": "username",
      "credential": "password"
    }
  ]
}

Copy link
Owner

@schlagmichdoch schlagmichdoch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

turn over tls description. See comment in conversation

@schlagmichdoch schlagmichdoch marked this pull request as draft February 13, 2025 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants