diff --git a/src/core/net/dns_types.cpp b/src/core/net/dns_types.cpp index 7c3241e95..dcbb65a53 100644 --- a/src/core/net/dns_types.cpp +++ b/src/core/net/dns_types.cpp @@ -336,7 +336,7 @@ Error Name::ReadName(const Message &aMessage, uint16_t &aOffset, char *aNameBuff } labelLength = static_cast(Min(static_cast(kMaxLabelSize), aNameBufferSize)); - SuccessOrExit(error = iterator.ReadLabel(aNameBuffer, labelLength, /* aAllowDotCharInLabel */ false)); + SuccessOrExit(error = iterator.ReadLabel(aNameBuffer, labelLength, /* aAllowDotCharInLabel */ firstLabel)); aNameBuffer += labelLength; aNameBufferSize -= labelLength; firstLabel = false; diff --git a/src/core/net/dns_types.hpp b/src/core/net/dns_types.hpp index d2b2bcf32..55dfc2646 100644 --- a/src/core/net/dns_types.hpp +++ b/src/core/net/dns_types.hpp @@ -853,8 +853,8 @@ public: * On successful read, the read name follows "...", i.e., a sequence of labels separated by * dot '.' character. The read name will ALWAYS end with a dot. * - * Verifies that the read labels in message do not contain any dot character, otherwise it returns - * `kErrorParse`). + * Verifies that the labels after the first label in message do not contain any dot character. If they do, + * returns `kErrorParse`. * * @param[in] aMessage The message to read the name from. `aMessage.GetOffset()` MUST point to * the start of DNS header (this is used to handle compressed names). diff --git a/src/core/net/srp_server.cpp b/src/core/net/srp_server.cpp index fc4dbc311..9699d84ae 100644 --- a/src/core/net/srp_server.cpp +++ b/src/core/net/srp_server.cpp @@ -43,6 +43,7 @@ #include "common/new.hpp" #include "common/num_utils.hpp" #include "common/random.hpp" +#include "common/string.hpp" #include "net/dns_types.hpp" #include "thread/thread_netif.hpp" @@ -915,12 +916,14 @@ Error Server::ProcessServiceDiscoveryInstructions(Host &aHost, for (uint16_t numRecords = aMetadata.mDnsHeader.GetUpdateRecordCount(); numRecords > 0; numRecords--) { - char serviceName[Dns::Name::kMaxNameSize]; - char instanceName[Dns::Name::kMaxNameSize]; - Dns::PtrRecord ptrRecord; - const char *subServiceName; - Service *service; - bool isSubType; + char serviceName[Dns::Name::kMaxNameSize]; + char instanceLabel[Dns::Name::kMaxLabelSize]; + char instanceServiceName[Dns::Name::kMaxNameSize]; + String instanceName; + Dns::PtrRecord ptrRecord; + const char *subServiceName; + Service *service; + bool isSubType; SuccessOrExit(error = Dns::Name::ReadName(aMessage, offset, serviceName, sizeof(serviceName))); VerifyOrExit(Dns::Name::IsSubDomainOf(serviceName, GetDomain()), error = kErrorSecurity); @@ -937,7 +940,9 @@ Error Server::ProcessServiceDiscoveryInstructions(Host &aHost, SuccessOrExit(error); - SuccessOrExit(error = Dns::Name::ReadName(aMessage, offset, instanceName, sizeof(instanceName))); + SuccessOrExit(error = ptrRecord.ReadPtrName(aMessage, offset, instanceLabel, sizeof(instanceLabel), + instanceServiceName, sizeof(instanceServiceName))); + instanceName.Append("%s.%s", instanceLabel, instanceServiceName); VerifyOrExit(ptrRecord.GetClass() == Dns::ResourceRecord::kClassNone || ptrRecord.GetClass() == aMetadata.mDnsZone.GetClass(), @@ -957,13 +962,14 @@ Error Server::ProcessServiceDiscoveryInstructions(Host &aHost, } // Verify that instance name and service name are related. - VerifyOrExit(Dns::Name::IsSubDomainOf(instanceName, isSubType ? subServiceName : serviceName), + VerifyOrExit(Dns::Name::IsSubDomainOf(instanceName.AsCString(), isSubType ? subServiceName : serviceName), error = kErrorFailed); // Ensure the same service does not exist already. - VerifyOrExit(aHost.FindService(serviceName, instanceName) == nullptr, error = kErrorFailed); + VerifyOrExit(aHost.FindService(serviceName, instanceName.AsCString()) == nullptr, error = kErrorFailed); - service = aHost.AddNewService(serviceName, instanceName, isSubType, aMetadata.mRxTime); + service = + aHost.AddNewService(serviceName, instanceName.AsCString(), instanceLabel, isSubType, aMetadata.mRxTime); VerifyOrExit(service != nullptr, error = kErrorNoBufs); // This RR is a "Delete an RR from an RRset" update when the CLASS is NONE. @@ -1854,8 +1860,10 @@ void Server::Service::Log(Action) const {} //--------------------------------------------------------------------------------------------------------------------- // Server::Service::Description -Error Server::Service::Description::Init(const char *aInstanceName, Host &aHost) +Error Server::Service::Description::Init(const char *aInstanceName, const char *aInstanceLabel, Host &aHost) { + Error error; + mNext = nullptr; mHost = &aHost; mPriority = 0; @@ -1867,7 +1875,11 @@ Error Server::Service::Description::Init(const char *aInstanceName, Host &aHost) mUpdateTime = TimerMilli::GetNow().GetDistantPast(); mTxtData.Free(); - return mInstanceName.Set(aInstanceName); + SuccessOrExit(error = mInstanceLabel.Set(aInstanceLabel)); + error = mInstanceName.Set(aInstanceName); + +exit: + return error; } bool Server::Service::Description::Matches(const char *aInstanceName) const @@ -2042,6 +2054,7 @@ const Server::Service *Server::Host::FindNextService(const Service *aPrevService Server::Service *Server::Host::AddNewService(const char *aServiceName, const char *aInstanceName, + const char *aInstanceLabel, bool aIsSubType, TimeMilli aUpdateTime) { @@ -2050,7 +2063,7 @@ Server::Service *Server::Host::AddNewService(const char *aServiceName, if (desc == nullptr) { - desc.Reset(Service::Description::AllocateAndInit(aInstanceName, *this)); + desc.Reset(Service::Description::AllocateAndInit(aInstanceName, aInstanceLabel, *this)); VerifyOrExit(desc != nullptr); } @@ -2063,6 +2076,12 @@ exit: return service; } +Server::Service *Server::Host::AddNewService(const Service &aService, TimeMilli aUpdateTime) +{ + return AddNewService(aService.GetServiceName(), aService.GetInstanceName(), aService.GetInstanceLabel(), + aService.IsSubType(), aUpdateTime); +} + void Server::Host::RemoveService(Service *aService, RetainName aRetainName, NotifyMode aNotifyServiceHandler) { Server &server = Get(); @@ -2103,8 +2122,7 @@ Error Server::Host::AddCopyOfServiceAsDeletedIfNotPresent(const Service &aServic VerifyOrExit(FindService(aService.GetServiceName(), aService.GetInstanceName()) == nullptr); - newService = - AddNewService(aService.GetServiceName(), aService.GetInstanceName(), aService.IsSubType(), aUpdateTime); + newService = AddNewService(aService, aUpdateTime); VerifyOrExit(newService != nullptr, error = kErrorNoBufs); @@ -2156,9 +2174,7 @@ Error Server::Host::MergeServicesAndResourcesFrom(Host &aHost) // Add/Merge `service` into the existing service or a allocate a new one - newService = (existingService != nullptr) ? existingService - : AddNewService(service.GetServiceName(), service.GetInstanceName(), - service.IsSubType(), service.GetUpdateTime()); + newService = (existingService != nullptr) ? existingService : AddNewService(service, service.GetUpdateTime()); VerifyOrExit(newService != nullptr, error = kErrorNoBufs); diff --git a/src/core/net/srp_server.hpp b/src/core/net/srp_server.hpp index f68e5da8e..ba5665b0c 100644 --- a/src/core/net/srp_server.hpp +++ b/src/core/net/srp_server.hpp @@ -245,7 +245,15 @@ public: * @returns A pointer service instance name (as a null-terminated C string). * */ - const char *GetInstanceName(void) const { return mDescription->mInstanceName.AsCString(); } + const char *GetInstanceName(void) const { return mDescription->GetInstanceName(); } + + /** + * Gets the service instance label of the service. + * + * @returns A pointer service instance label (as a null-terminated C string). + * + */ + const char *GetInstanceLabel(void) const { return mDescription->GetInstanceLabel(); } /** * Gets the full service name of the service. @@ -399,8 +407,9 @@ public: public RetainCountable, private NonCopyable { - Error Init(const char *aInstanceName, Host &aHost); + Error Init(const char *aInstanceName, const char *aInstanceLabel, Host &aHost); const char *GetInstanceName(void) const { return mInstanceName.AsCString(); } + const char *GetInstanceLabel(void) const { return mInstanceLabel.AsCString(); } bool Matches(const char *aInstanceName) const; void ClearResources(void); void TakeResourcesFrom(Description &aDescription); @@ -408,6 +417,7 @@ public: Description *mNext; Heap::String mInstanceName; + Heap::String mInstanceLabel; Host *mHost; Heap::Data mTxtData; uint16_t mPriority; @@ -600,8 +610,10 @@ public: LinkedList &GetServices(void) { return mServices; } Service *AddNewService(const char *aServiceName, const char *aInstanceName, + const char *aInstanceLabel, bool aIsSubType, TimeMilli aUpdateTime); + Service *AddNewService(const Service &aService, TimeMilli aUpdateTime); void RemoveService(Service *aService, RetainName aRetainName, NotifyMode aNotifyServiceHandler); Error AddCopyOfServiceAsDeletedIfNotPresent(const Service &aService, TimeMilli aUpdateTime); void FreeAllServices(void); diff --git a/tests/unit/test_dns.cpp b/tests/unit/test_dns.cpp index 089b4861b..7a87b25ea 100644 --- a/tests/unit/test_dns.cpp +++ b/tests/unit/test_dns.cpp @@ -478,6 +478,7 @@ void TestDnsCompressedName(void) static const char kExpectedReadName1[] = "F.ISI.ARPA."; static const char kExpectedReadName2[] = "FOO.F.ISI.ARPA."; static const char kExpectedReadName3[] = "ISI.ARPA."; + static const char kExpectedReadName4[] = "Human.Readable.F.ISI.ARPA."; static const char kBadName[] = "bad.name"; @@ -745,10 +746,12 @@ void TestDnsCompressedName(void) VerifyOrQuit(labelLength == strlen(label), "Name::ReadLabel() returned incorrect label length"); } - // `ReadName()` for name-4 should fails due to first label containing dot char. + // `ReadName()` for name-4 should still succeed since only the first label contains dot char offset = name4Offset; - VerifyOrQuit(Dns::Name::ReadName(*message, offset, name, sizeof(name)) == kErrorParse, - "Name::ReadName() did not fail with invalid label"); + SuccessOrQuit(Dns::Name::ReadName(*message, offset, name, sizeof(name))); + printf("Read name =\"%s\"\n", name); + VerifyOrQuit(strcmp(name, kExpectedReadName4) == 0, "Name::ReadName() did not return expected name"); + VerifyOrQuit(offset == name4Offset + kName4EncodedSize, "Name::ParseName() returned incorrect offset"); offset = name4Offset; @@ -826,9 +829,9 @@ void TestHeaderAndResourceRecords(void) const char kServiceLabels[] = "_service._udp"; const char kServiceName[] = "_service._udp.example.com."; const char kInstance1Label[] = "inst1"; - const char kInstance2Label[] = "instance2"; + const char kInstance2Label[] = "instance.2"; // Instance label includes dot '.' character. const char kInstance1Name[] = "inst1._service._udp.example.com."; - const char kInstance2Name[] = "instance2._service._udp.example.com."; + const char kInstance2Name[] = "instance.2._service._udp.example.com."; const char kHostName[] = "host.example.com."; const uint8_t kTxtData[] = {9, 'k', 'e', 'y', '=', 'v', 'a', 'l', 'u', 'e', 0}; const char kHostAddress[] = "fd00::abcd:"; @@ -988,16 +991,18 @@ void TestHeaderAndResourceRecords(void) VerifyOrQuit(offset == answerSectionOffset, "answer section offset is incorrect"); - for (const char *instanceName : kInstanceNames) + for (const char *instanceLabel : kInstanceLabels) { SuccessOrQuit(Dns::Name::CompareName(*message, offset, kServiceName)); SuccessOrQuit(Dns::ResourceRecord::ReadRecord(*message, offset, ptrRecord)); VerifyOrQuit(ptrRecord.GetTtl() == kTtl, "Read PTR is incorrect"); - SuccessOrQuit(ptrRecord.ReadPtrName(*message, offset, name, sizeof(name))); - VerifyOrQuit(strcmp(name, instanceName) == 0, "Inst1 name is incorrect"); + SuccessOrQuit(ptrRecord.ReadPtrName(*message, offset, label, sizeof(label), name, sizeof(name))); + VerifyOrQuit(strcmp(label, instanceLabel) == 0, "Inst label is incorrect"); + VerifyOrQuit(strcmp(name, kServiceName) == 0); - printf(" \"%s\" PTR %u %d \"%s\"\n", kServiceName, ptrRecord.GetTtl(), ptrRecord.GetLength(), name); + printf(" \"%s\" PTR %u %d \"%s.%s\"\n", kServiceName, ptrRecord.GetTtl(), ptrRecord.GetLength(), label, + name); } VerifyOrQuit(offset == additionalSectionOffset, "offset is incorrect after answer section parse"); diff --git a/tests/unit/test_srp_server.cpp b/tests/unit/test_srp_server.cpp index cbe9199d3..2cac23a58 100644 --- a/tests/unit/test_srp_server.cpp +++ b/tests/unit/test_srp_server.cpp @@ -287,7 +287,7 @@ static const char kHostName[] = "myhost"; void PrepareService1(Srp::Client::Service &aService) { static const char kServiceName[] = "_srv._udp"; - static const char kInstanceLabel[] = "srv-instance"; + static const char kInstanceLabel[] = "srv.instance"; static const char kSub1[] = "_sub1"; static const char kSub2[] = "_V1234567"; static const char kSub3[] = "_XYZWS";