Skip to content
This repository was archived by the owner on Jul 12, 2023. It is now read-only.

Fix STUN server usage when stunServerPort isn't set and the default value is to be used - #23

Merged
j1elo merged 1 commit into
Kurento:masterfrom
prlanzarin:upstream-613-nightly-fix-stunserverport
Mar 11, 2020
Merged

j1elo merged 1 commit into
Kurento:masterfrom
prlanzarin:upstream-613-nightly-fix-stunserverport

Conversation

@prlanzarin

Copy link
Copy Markdown
Contributor

What is the current behavior you want to change?

The stunServerPort configuration in WebRtcEndpoint.conf.ini is supposed to be an optional one since there's a DEFAULT_STUN_PORT (3478) set up in WebRtcEndpointImpl.cpp.

Expected behaviour: be able to configure only stunServerAddress if the STUN server's port is the default one (3478).

Current behaviour: if stunServerPort isn't configured and stunServerAddress is, Kurento will not set a STUN server in the NiceAgent due to a botched pair of if-else blocks in WebRtcEndpointImpl.cpp.

What is the new behavior provided by this change?

Configuring only stunServerAddress in WebRtcEndpoint.conf.ini will make KMS configure a STUN server in the NiceAgent interface with the default STUN port (3478).

How has this been tested?

Tested with the following variation of the stunServerAddress/stunServerPort configs:

  • No stunServerPort, with stunServerAddress -> STUN server set in WebRtcEndpoint with $stunServerAddress:3478
  • With stunServerAddress and stunServerPort: STUN server set in WebRtcEndpoint with $stunServerAddress:stunServerPort`
  • With stunServerPort, no stunServerAddress -> No STUN server set in WebRtcEndpoint
  • No stunServerPort, no stunServerAddress -> No STUN server set in WebRtcEndpoint

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature / enhancement (non-breaking change which improves the project)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • My change requires a change to the documentation
  • My change requires a change in other repository

Checklist

  • I have read the Contribution Guidelines
  • I have added an explanation of what the changes do and why they should be included
  • I have written new tests for the changes, as applicable, and have successfully run them locally

The if-else blocks were wrongly disposed, so when the stunServerPort config wasnt set, but the IP was, a STUN server would not be setup in the NiceAgent interface

Also some identation changes according to the git hook suggestions around the code I changed
@jenkinskurento

Copy link
Copy Markdown

Hi there, thanks for your Pull Request!

A Kurento member needs to verify that this patch is reasonable to test. In case it is, they should write a comment with the phrase test this please. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by Kurento members will still work. Regular contributors can be whitelisted to skip this step.

@j1elo j1elo self-assigned this Mar 11, 2020
@j1elo
j1elo merged commit 3b444e0 into Kurento:master Mar 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants