fix(mdns): don't hardcode mDNS instance name #3084
Conversation
src/network.cpp
Outdated
| */ | ||
| std::string | ||
| mdns_instance_name() { | ||
| std::string hostname = boost::asio::ip::host_name(); |
There was a problem hiding this comment.
If this became an argument for the function, the unit testing could be parametrized with various hostnames to check if it will work for all the scenarios provided.
There was a problem hiding this comment.
Added in the latest commit.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3084 +/- ##
=========================================
- Coverage 9.66% 9.54% -0.13%
=========================================
Files 101 77 -24
Lines 17942 14040 -3902
Branches 8381 6442 -1939
=========================================
- Hits 1734 1340 -394
+ Misses 13328 10082 -3246
+ Partials 2880 2618 -262
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| @@ -105,7 +105,8 @@ namespace platf::publish { | |||
| &serviceRef, | |||
| 0, // flags | |||
| 0, // interfaceIndex | |||
| SERVICE_NAME, SERVICE_TYPE, | |||
| nullptr, // name | |||
There was a problem hiding this comment.
Since you're giving it a nullptr, it won't use net::mdns_instance_name() for macOS at all, and the tests aren't useful for macOS right?
There was a problem hiding this comment.
Correct, the function is unused on macOS.
ReenigneArcher
force-pushed
the
unique_mdns_names
branch
from
87b5ede to
a7b43d3
Compare
|
Description
All platform mDNS implemenations currently hardcode the instance name to
Sunshine. This causes reliable name collisions during mDNS registration when more than one Sunshine host is running on the same network. The collision is eventually resolved by appending a number to the name, but this is unnecessary work and delay for nothing. We should just register using our hostname as the instance name most other mDNS services do.Screenshot
Issues Fixed or Closed
Type of Change
.github/...)Checklist