Skip to content
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

fix(mdns): don't hardcode mDNS instance name #3084

Merged
merged 2 commits into from
Aug 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions src/network.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -206,4 +206,34 @@ namespace net {

return mapped_port;
}

/**
* @brief Returns a string for use as the instance name for mDNS.
* @param hostname The hostname to use for instance name generation.
* @return Hostname-based instance name or "Sunshine" if hostname is invalid.
*/
std::string
mdns_instance_name(const std::string_view &hostname) {
// Start with the unmodified hostname
std::string instancename { hostname.data(), hostname.size() };

// Truncate to 63 characters per RFC 6763 section 7.2.
if (instancename.size() > 63) {
instancename.resize(63);
}

for (auto i = 0; i < instancename.size(); i++) {
// Replace any spaces with dashes
if (instancename[i] == ' ') {
instancename[i] = '-';
}
else if (!std::isalnum(instancename[i]) && instancename[i] != '-') {
// Stop at the first invalid character
instancename.resize(i);
break;
}
}

return !instancename.empty() ? instancename : "Sunshine";
}
} // namespace net
8 changes: 8 additions & 0 deletions src/network.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,4 +105,12 @@ namespace net {
*/
int
encryption_mode_for_address(boost::asio::ip::address address);

/**
* @brief Returns a string for use as the instance name for mDNS.
* @param hostname The hostname to use for instance name generation.
* @return Hostname-based instance name or "Sunshine" if hostname is invalid.
*/
std::string
mdns_instance_name(const std::string_view &hostname);
} // namespace net
3 changes: 2 additions & 1 deletion src/platform/linux/publish.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,8 @@ namespace platf::publish {
return nullptr;
}

name.reset(avahi::strdup(SERVICE_NAME));
auto instance_name = net::mdns_instance_name(boost::asio::ip::host_name());
name.reset(avahi::strdup(instance_name.c_str()));

client.reset(
avahi::client_new(avahi::simple_poll_get(poll.get()), avahi::ClientFlags(0), client_callback, nullptr, &avhi_error));
Expand Down
3 changes: 2 additions & 1 deletion src/platform/macos/publish.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,8 @@ namespace platf::publish {
&serviceRef,
0, // flags
0, // interfaceIndex
SERVICE_NAME, SERVICE_TYPE,
nullptr, // name
Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Correct, the function is unused on macOS.

SERVICE_TYPE,
nullptr, // domain
nullptr, // host
htons(net::map_port(nvhttp::PORT_HTTP)),
Expand Down
6 changes: 3 additions & 3 deletions src/platform/windows/publish.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ constexpr auto DNS_QUERY_RESULTS_VERSION1 = 0x1;

#define SERVICE_DOMAIN "local"

constexpr auto SERVICE_INSTANCE_NAME = SV(SERVICE_NAME "." SERVICE_TYPE "." SERVICE_DOMAIN);
constexpr auto SERVICE_TYPE_DOMAIN = SV(SERVICE_TYPE "." SERVICE_DOMAIN);

#ifndef __MINGW32__
Expand Down Expand Up @@ -107,10 +106,11 @@ namespace platf::publish {
service(bool enable, PDNS_SERVICE_INSTANCE &existing_instance) {
auto alarm = safe::make_alarm<PDNS_SERVICE_INSTANCE>();

std::wstring name { SERVICE_INSTANCE_NAME.data(), SERVICE_INSTANCE_NAME.size() };
std::wstring domain { SERVICE_TYPE_DOMAIN.data(), SERVICE_TYPE_DOMAIN.size() };

auto host = from_utf8(boost::asio::ip::host_name() + ".local");
auto hostname = boost::asio::ip::host_name();
auto name = from_utf8(net::mdns_instance_name(hostname) + '.') + domain;
auto host = from_utf8(hostname + ".local");

DNS_SERVICE_INSTANCE instance {};
instance.pszInstanceName = name.data();
Expand Down
26 changes: 26 additions & 0 deletions tests/unit/test_network.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/**
* @file tests/unit/test_network.cpp
* @brief Test src/network.*
*/
#include <src/network.h>

#include "../tests_common.h"

struct MdnsInstanceNameTest: testing::TestWithParam<std::tuple<std::string, std::string>> {};

TEST_P(MdnsInstanceNameTest, Run) {
auto [input, expected] = GetParam();
ASSERT_EQ(net::mdns_instance_name(input), expected);
}

INSTANTIATE_TEST_SUITE_P(
MdnsInstanceNameTests,
MdnsInstanceNameTest,
testing::Values(
std::make_tuple("shortname-123", "shortname-123"),
std::make_tuple("space 123", "space-123"),
std::make_tuple("hostname.domain.test", "hostname"),
std::make_tuple("&", "Sunshine"),
std::make_tuple("", "Sunshine"),
std::make_tuple("😁", "Sunshine"),
std::make_tuple(std::string(128, 'a'), std::string(63, 'a'))));
Loading