From 92e8f691bd30015fe610318d66bc0ecd2908eb88 Mon Sep 17 00:00:00 2001 From: Gareth Sylvester-Bradley Date: Wed, 25 Mar 2026 13:21:58 +0000 Subject: [PATCH 1/3] Fix service name generation to enforce RFC 6763 length limit Truncate mDNS service instance names exceeding 63 bytes with a hash suffix to ensure RFC 6763 compliance, since Avahi does not automatically truncate like mDNSResponder. Also use underscore rather than colon as the host/port separator, and declare service_name in the header. Add unit tests for service_name. Made-with: Cursor --- Development/cmake/NmosCppTest.cmake | 1 + Development/nmos/mdns.cpp | 37 +++++- Development/nmos/mdns.h | 3 + Development/nmos/test/mdns_test.cpp | 198 ++++++++++++++++++++++++++++ 4 files changed, 236 insertions(+), 3 deletions(-) create mode 100644 Development/nmos/test/mdns_test.cpp diff --git a/Development/cmake/NmosCppTest.cmake b/Development/cmake/NmosCppTest.cmake index b2328fd7..2acecf83 100644 --- a/Development/cmake/NmosCppTest.cmake +++ b/Development/cmake/NmosCppTest.cmake @@ -55,6 +55,7 @@ set(NMOS_CPP_TEST_NMOS_TEST_SOURCES nmos/test/json_validator_test.cpp nmos/test/jwt_generator_test.cpp nmos/test/jwt_validation_test.cpp + nmos/test/mdns_test.cpp nmos/test/paging_utils_test.cpp nmos/test/query_api_test.cpp nmos/test/sdp_test_utils.cpp diff --git a/Development/nmos/mdns.cpp b/Development/nmos/mdns.cpp index 2549e5f4..8d4ebf3c 100644 --- a/Development/nmos/mdns.cpp +++ b/Development/nmos/mdns.cpp @@ -1,5 +1,7 @@ #include "nmos/mdns.h" +#include +#include #include #include #include @@ -330,6 +332,13 @@ namespace nmos return utility::us2s(nmos::fields::service_name_prefix(settings)) + "_" + service_api(service); } + inline std::string hash_string(const std::string& s) + { + std::ostringstream os; + os << std::hex << std::setfill('0') << std::setw(8) << (std::hash{}(s) & 0xFFFFFFFF); + return os.str(); + } + inline std::set service_versions(const nmos::service_type& service, const nmos::settings& settings) { // the System API is defined by IS-09 (having been originally specified in JT-NM TR-1001-1:2018 Annex A) @@ -341,11 +350,33 @@ namespace nmos } } + // generate a stable unique name for the specified service, based on the service type, host and port std::string service_name(const nmos::service_type& service, const nmos::settings& settings) { - // this just serves as an example of a possible service naming strategy - // replacing '.' with '-', since although '.' is legal in service names, some DNS-SD implementations just don't like it - return boost::algorithm::replace_all_copy(details::service_base_name(service, settings) + "_" + utility::us2s(nmos::get_host(settings)) + ":" + utility::us2s(utility::ostringstreamed(details::service_port(service, settings))), ".", "-"); + // replace '.' with '-', since although '.' is legal in service names, some DNS-SD implementations just don't like it + const auto name = boost::algorithm::replace_all_copy(details::service_base_name(service, settings) + "_" + utility::us2s(nmos::get_host(settings)) + "_" + utility::us2s(utility::ostringstreamed(details::service_port(service, settings))), ".", "-"); + + // RFC 6763 Section 4.1.1 specifies that instance names must not exceed 63 bytes + // see https://tools.ietf.org/html/rfc6763#section-4.1.1 + const size_t max_length = 63; + if (name.size() <= max_length) return name; + + // truncate over-long names, because Avahi does not automatically truncate like mDNSResponder does, + // and include a unique suffix based on the full name, to reduce collisions + + // RFC 6763 explains that DNS-SD names are intended to be user-visible, "short and descriptive", + // and avoid "incomprehensible hexadecimal strings"; because this is likely to result in collisions, + // RFC 6762 defines a conflict resolution mechanism designed to keep names human-readable, + // implemented by Avahi (" #2") and mDNSResponder (" (2)"), and recommends the newly + // chosen name is recorded in persistent storage so that the service will use the same name when it + // restarts... + // because NMOS service names are unlikely to be presented to the user, maximizing the likelihood + // of a unique name without the need for persistent storage is higher priority than no hex strings! + + const auto suffix = details::hash_string(name); + const auto max_prefix_length = max_length - 1 - suffix.size(); + const auto prefix_length = name.find_last_not_of("-_", max_prefix_length - 1) + 1; + return name.substr(0, prefix_length) + "-" + suffix; } // helper function for registering addresses when the host name is explicitly configured diff --git a/Development/nmos/mdns.h b/Development/nmos/mdns.h index 8267ab85..b591c382 100644 --- a/Development/nmos/mdns.h +++ b/Development/nmos/mdns.h @@ -148,6 +148,9 @@ namespace nmos namespace experimental { + // generate a stable unique name for the specified service, based on the service type, host and port + std::string service_name(const nmos::service_type& service, const nmos::settings& settings); + // helper function for registering addresses when the host name is explicitly configured void register_addresses(mdns::service_advertiser& advertiser, const nmos::settings& settings); diff --git a/Development/nmos/test/mdns_test.cpp b/Development/nmos/test/mdns_test.cpp new file mode 100644 index 00000000..7feb8aa0 --- /dev/null +++ b/Development/nmos/test/mdns_test.cpp @@ -0,0 +1,198 @@ +// The first "test" is of course whether the header compiles standalone +#include "nmos/mdns.h" + +#include "bst/test/test.h" +#include "cpprest/basic_utils.h" + +//////////////////////////////////////////////////////////////////////////////////////////// +BST_TEST_CASE(testServiceNameFormat) +{ + using web::json::value_of; + + // with default settings, service_name should be "___" + // with dots replaced by dashes + const nmos::settings settings; + + BST_CHECK_EQUAL("nmos-cpp_node_127-0-0-1_3212", nmos::experimental::service_name(nmos::service_types::node, settings)); + BST_CHECK_EQUAL("nmos-cpp_query_127-0-0-1_3211", nmos::experimental::service_name(nmos::service_types::query, settings)); + BST_CHECK_EQUAL("nmos-cpp_registration_127-0-0-1_3210", nmos::experimental::service_name(nmos::service_types::registration, settings)); + BST_CHECK_EQUAL("nmos-cpp_system_127-0-0-1_10641", nmos::experimental::service_name(nmos::service_types::system, settings)); + BST_CHECK_EQUAL("nmos-cpp_auth_127-0-0-1_443", nmos::experimental::service_name(nmos::service_types::authorization, settings)); +} + +//////////////////////////////////////////////////////////////////////////////////////////// +BST_TEST_CASE(testServiceNameCustomPrefix) +{ + using web::json::value_of; + + // dots in prefix should be replaced with dashes + const nmos::settings settings = value_of({ + { nmos::fields::service_name_prefix, U("my.prefix") } + }); + + BST_CHECK_EQUAL("my-prefix_node_127-0-0-1_3212", nmos::experimental::service_name(nmos::service_types::node, settings)); +} + +//////////////////////////////////////////////////////////////////////////////////////////// +BST_TEST_CASE(testServiceNameDotReplacement) +{ + using web::json::value_of; + + // dots in host name should be replaced with dashes + const nmos::settings settings = value_of({ + { nmos::fields::host_name, U("my-server.example.com") }, + { nmos::experimental::fields::href_mode, 1 } + }); + + BST_CHECK_EQUAL("nmos-cpp_node_my-server-example-com_3212", nmos::experimental::service_name(nmos::service_types::node, settings)); +} + +//////////////////////////////////////////////////////////////////////////////////////////// +BST_TEST_CASE(testServiceNameCustomPort) +{ + using web::json::value_of; + + const nmos::settings settings = value_of({ + { nmos::fields::node_port, 8080 } + }); + + BST_CHECK_EQUAL("nmos-cpp_node_127-0-0-1_8080", nmos::experimental::service_name(nmos::service_types::node, settings)); +} + +//////////////////////////////////////////////////////////////////////////////////////////// +BST_TEST_CASE(testServiceNameMaxLength) +{ + using web::json::value_of; + + // RFC 6763 Section 4.1.1 specifies instance names must not exceed 63 bytes + const size_t max_length = 63; + + // a name that fits within 63 bytes should pass through unchanged + { + const nmos::settings settings; + const auto name = nmos::experimental::service_name(nmos::service_types::node, settings); + BST_REQUIRE(name.size() <= max_length); + // no hash suffix + BST_CHECK_EQUAL("nmos-cpp_node_127-0-0-1_3212", name); + } + + // a name that would exceed 63 bytes should be truncated with a hash suffix + { + const nmos::settings settings = value_of({ + { nmos::fields::host_name, U("a-host-name-that-is-itself-valid-but-already-63-characters-long.example.com") }, + { nmos::experimental::fields::href_mode, 1 } + }); + + const auto name = nmos::experimental::service_name(nmos::service_types::registration, settings); + BST_REQUIRE(name.size() <= max_length); + } +} + +//////////////////////////////////////////////////////////////////////////////////////////// +BST_TEST_CASE(testServiceNameTruncationStable) +{ + using web::json::value_of; + + // the truncated name should be stable (consistent) even across runs of the same program + // but that's hard to test (and actually, std::hash is only required to produce the same + // result for the same input within a single execution of a program; this allows salted + // hashes that prevent collision denial-of-service attacks, which would be unfortunate + // for this use case, but common standard library implementations don't do that...) + + const nmos::settings settings = value_of({ + { nmos::fields::host_name, U("a-host-name-that-is-itself-valid-but-already-63-characters-long.example.com") }, + { nmos::experimental::fields::href_mode, 1 } + }); + + const auto name1 = nmos::experimental::service_name(nmos::service_types::registration, settings); + const auto name2 = nmos::experimental::service_name(nmos::service_types::registration, settings); + BST_CHECK_EQUAL(name1, name2); +} + +//////////////////////////////////////////////////////////////////////////////////////////// +BST_TEST_CASE(testServiceNameTruncationSuffix) +{ + using web::json::value_of; + + const size_t max_length = 63; + + // the truncated name should end with a dash followed by an 8-character hex hash + const nmos::settings settings = value_of({ + { nmos::fields::host_name, U("a-host-name-that-is-itself-valid-but-already-63-characters-long.example.com") }, + { nmos::experimental::fields::href_mode, 1 } + }); + + const auto name = nmos::experimental::service_name(nmos::service_types::registration, settings); + BST_REQUIRE(name.size() <= max_length); + + const auto last_dash = name.rfind('-'); + BST_REQUIRE(std::string::npos != last_dash); + + // the suffix after the last dash should be exactly 8 hex characters + const auto suffix = name.substr(last_dash + 1); + BST_CHECK_EQUAL(8, suffix.size()); + BST_CHECK(suffix.end() == std::find_if(suffix.begin(), suffix.end(), [](char c) + { + return !std::isxdigit(static_cast(c)); + })); +} + +//////////////////////////////////////////////////////////////////////////////////////////// +BST_TEST_CASE(testServiceNameExactly63Bytes) +{ + using web::json::value_of; + + // a name that is exactly 63 bytes should not be truncated + // "nmos-cpp_node_" = 14 chars, "_3212" = 5 chars, so host needs to be 63 - 14 - 5 = 44 chars + // after dot replacement: e.g. a host name with no dots that is 44 chars + const std::string host_44(44, 'x'); + const nmos::settings settings = value_of({ + { nmos::fields::host_name, utility::s2us(host_44) }, + { nmos::experimental::fields::href_mode, 1 } + }); + + const auto name = nmos::experimental::service_name(nmos::service_types::node, settings); + BST_CHECK_EQUAL(63, name.size()); + BST_CHECK_EQUAL("nmos-cpp_node_" + host_44 + "_3212", name); +} + +//////////////////////////////////////////////////////////////////////////////////////////// +BST_TEST_CASE(testServiceNameOneOver63Bytes) +{ + using web::json::value_of; + + // a name that is 64 bytes should be truncated + const std::string host_45(45, 'x'); + const nmos::settings settings = value_of({ + { nmos::fields::host_name, utility::s2us(host_45) }, + { nmos::experimental::fields::href_mode, 1 } + }); + + const auto name = nmos::experimental::service_name(nmos::service_types::node, settings); + BST_REQUIRE(name.size() <= 63); +} + +//////////////////////////////////////////////////////////////////////////////////////////// +BST_TEST_CASE(testServiceNameTruncationDistinct) +{ + using web::json::value_of; + + // even when truncated, names that differ only in the port (which is past the truncation point) + // should produce distinct names due to the hash suffix being based on the full untruncated name + const auto make_settings = [](int port) + { + return value_of({ + { nmos::fields::host_name, U("a-host-name-that-is-itself-valid-but-already-63-characters-long.example.com") }, + { nmos::experimental::fields::href_mode, 1 }, + { nmos::fields::node_port, port } + }); + }; + + const auto name1 = nmos::experimental::service_name(nmos::service_types::node, make_settings(3212)); + const auto name2 = nmos::experimental::service_name(nmos::service_types::node, make_settings(3213)); + const auto name3 = nmos::experimental::service_name(nmos::service_types::node, make_settings(3214)); + + BST_CHECK(name1 != name2); + BST_CHECK(name1 != name3); + BST_CHECK(name2 != name3); +} From 516c7386db10e235f181a328b262dd1ad265ff6c Mon Sep 17 00:00:00 2001 From: Gareth Sylvester-Bradley Date: Fri, 27 Mar 2026 12:42:54 +0000 Subject: [PATCH 2/3] Generate a slightly more compact hash string --- Development/nmos/mdns.cpp | 15 ++++++++++----- Development/nmos/test/mdns_test.cpp | 7 +++---- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/Development/nmos/mdns.cpp b/Development/nmos/mdns.cpp index 8d4ebf3c..ff502c0b 100644 --- a/Development/nmos/mdns.cpp +++ b/Development/nmos/mdns.cpp @@ -1,7 +1,6 @@ #include "nmos/mdns.h" #include -#include #include #include #include @@ -332,11 +331,17 @@ namespace nmos return utility::us2s(nmos::fields::service_name_prefix(settings)) + "_" + service_api(service); } - inline std::string hash_string(const std::string& s) + // generate a hash string (slightly more compact and possibly slightly more memorable than hex) + inline std::string hash_string(const std::string& s, size_t len = 5) { - std::ostringstream os; - os << std::hex << std::setfill('0') << std::setw(8) << (std::hash{}(s) & 0xFFFFFFFF); - return os.str(); + auto hash = std::hash{}(s); + // vowels (and vowel-ish digits) are omitted from the set of available characters + // to reduce the chances of "bad words" being formed + static const char alphanums[] = "bcdfghjklmnpqrstvwxz2456789"; + static const size_t base = sizeof(alphanums) - 1; + std::string result(len, ' '); + for (auto& c : result) { c = alphanums[hash % base]; hash /= base; } + return result; } inline std::set service_versions(const nmos::service_type& service, const nmos::settings& settings) diff --git a/Development/nmos/test/mdns_test.cpp b/Development/nmos/test/mdns_test.cpp index 7feb8aa0..ab789272 100644 --- a/Development/nmos/test/mdns_test.cpp +++ b/Development/nmos/test/mdns_test.cpp @@ -116,7 +116,7 @@ BST_TEST_CASE(testServiceNameTruncationSuffix) const size_t max_length = 63; - // the truncated name should end with a dash followed by an 8-character hex hash + // the truncated name should end with a dash followed by a 5-character alphanumeric hash const nmos::settings settings = value_of({ { nmos::fields::host_name, U("a-host-name-that-is-itself-valid-but-already-63-characters-long.example.com") }, { nmos::experimental::fields::href_mode, 1 } @@ -128,12 +128,11 @@ BST_TEST_CASE(testServiceNameTruncationSuffix) const auto last_dash = name.rfind('-'); BST_REQUIRE(std::string::npos != last_dash); - // the suffix after the last dash should be exactly 8 hex characters const auto suffix = name.substr(last_dash + 1); - BST_CHECK_EQUAL(8, suffix.size()); + BST_CHECK_EQUAL(5, suffix.size()); BST_CHECK(suffix.end() == std::find_if(suffix.begin(), suffix.end(), [](char c) { - return !std::isxdigit(static_cast(c)); + return !std::isdigit(static_cast(c)) && !std::islower(static_cast(c)); })); } From e16cbd74d0358a6dbd2afc87aeb261ea0c45622b Mon Sep 17 00:00:00 2001 From: Gareth Sylvester-Bradley Date: Fri, 27 Mar 2026 20:47:53 +0000 Subject: [PATCH 3/3] Clarify that the hash suffix isn't guaranteed to be unique --- Development/nmos/test/mdns_test.cpp | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/Development/nmos/test/mdns_test.cpp b/Development/nmos/test/mdns_test.cpp index ab789272..acb9e485 100644 --- a/Development/nmos/test/mdns_test.cpp +++ b/Development/nmos/test/mdns_test.cpp @@ -176,8 +176,8 @@ BST_TEST_CASE(testServiceNameTruncationDistinct) { using web::json::value_of; - // even when truncated, names that differ only in the port (which is past the truncation point) - // should produce distinct names due to the hash suffix being based on the full untruncated name + // settings that produce distinct over-long names before truncation usually result in + // distinct names because the hash suffix is based on the full untruncated name const auto make_settings = [](int port) { return value_of({ @@ -189,9 +189,25 @@ BST_TEST_CASE(testServiceNameTruncationDistinct) const auto name1 = nmos::experimental::service_name(nmos::service_types::node, make_settings(3212)); const auto name2 = nmos::experimental::service_name(nmos::service_types::node, make_settings(3213)); - const auto name3 = nmos::experimental::service_name(nmos::service_types::node, make_settings(3214)); BST_CHECK(name1 != name2); - BST_CHECK(name1 != name3); - BST_CHECK(name2 != name3); +} + +//////////////////////////////////////////////////////////////////////////////////////////// +BST_TEST_CASE(testServiceNameTruncationExample) +{ + using web::json::value_of; + + const nmos::settings settings = value_of({ + { nmos::fields::host_name, U("a-host-name-that-is-itself-valid-but-already-63-characters-long.xz6zx.example.com") }, + { nmos::experimental::fields::href_mode, 1 } + }); + + const auto name = nmos::experimental::service_name(nmos::service_types::node, settings); + BST_REQUIRE_EQUAL("nmos-cpp_node_a-host-name-that-is-itself-valid-but-alread-", name.substr(0, name.size() - 5)); +#ifdef __GLIBCXX__ + BST_CHECK_EQUAL("cr4ck", name.substr(name.size() - 5)); + // ...e4y8q.example.com produces the same hash suffix with this std::hash implementation + // but anything could happen on another platform... +#endif }