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

[mdns] fix mDNS type length #6771

Merged
merged 2 commits into from
May 13, 2021
Merged

Conversation

wgtdkp
Copy link
Contributor

@wgtdkp wgtdkp commented May 13, 2021

Problem

The type parameter in MdnsAvahi::HandleBrowse includes the protocol (_tcp or _udp) as suffix
but the MdnsService::mType fields includes only the service type. The Linux avahi implementation
results in failures in browse and resolve handler.

Summary of Changes

This PR uses a local buffer for copying the type argument and parsing it into true service type
and protocol fields.

Fixes #6773

@wgtdkp wgtdkp force-pushed the fix-mdns-type-length branch from 9b2c5ee to dd0119c Compare May 13, 2021 10:54
@boring-cyborg boring-cyborg bot added the linux label May 13, 2021
@andy31415 andy31415 added the hotfix urgent fix needed, can bypass review label May 13, 2021
service.mName[kMdnsNameMaxSize] = 0;
strncpy(typeAndProtocol, type, sizeof(typeAndProtocol));
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need a separate copy?

The original code is suspect because it uses a local variable probably due to const-correctness. However getting the protocol from type seems totally doable in const char *.

@andy31415 andy31415 merged commit 2e2e213 into project-chip:master May 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hotfix urgent fix needed, can bypass review lib linux openthread platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[tests] unit test "TestMdns" is failing
4 participants