Skip to content

Handle Service types that end with another service type#1041

Merged
bdraco merged 8 commits into
python-zeroconf:masterfrom
apworks1:patch-1
Dec 24, 2021
Merged

Handle Service types that end with another service type#1041
bdraco merged 8 commits into
python-zeroconf:masterfrom
apworks1:patch-1

Conversation

@apworks1
Copy link
Copy Markdown

@apworks1 apworks1 commented Dec 23, 2021

Fixes erroneous match with service names that have a type that ends with a shorter type.
Given "%NAS._asustor-looksgood_http._tcp.local.",
type "_asustor-looksgood_http._tcp.local." should not match "_http._tcp.local."

My example: currently with an Asustor NAS the "%NAS._asustor-looksgood_http._tcp.local." service appears to have a "_http._tcp.local." service_type while its actual type is "_asustor-looksgood_http._tcp.local."
This causes an exception because the ServiceInfo init checks:

if not type_.endswith(service_type_name(name, strict=False)):
            raise BadTypeInNameException

 File "/usr/src/homeassistant/homeassistant/components/zeroconf/__init__.py", line 405, in _process_service_update
    async_service_info = AsyncServiceInfo(service_type, name)
  File "/usr/local/lib/python3.9/site-packages/zeroconf/_services/info.py", line 131, in __init__
    raise BadTypeInNameException

Fixes #1040

@apworks1 apworks1 changed the title Service types that ends with another service type Handle Service types that end with another service type Dec 23, 2021
Copy link
Copy Markdown
Member

@bdraco bdraco left a comment

Choose a reason for hiding this comment

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

Looks like a good fix. Please add a test for this case.

Thanks 👍

Comment thread zeroconf/_services/browser.py Outdated
@bdraco
Copy link
Copy Markdown
Member

bdraco commented Dec 23, 2021

Looks like we don't handle subtypes correctly

@bdraco
Copy link
Copy Markdown
Member

bdraco commented Dec 23, 2021

That test is wrong. Its passing the registration name instead of the type

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Dec 23, 2021

Codecov Report

Merging #1041 (6990818) into master (631a6f7) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1041      +/-   ##
==========================================
- Coverage   99.88%   99.88%   -0.01%     
==========================================
  Files          23       23              
  Lines        2542     2538       -4     
  Branches      472      470       -2     
==========================================
- Hits         2539     2535       -4     
  Misses          3        3              
Impacted Files Coverage Δ
zeroconf/_handlers.py 100.00% <100.00%> (ø)
zeroconf/_services/browser.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 631a6f7...6990818. Read the comment docs.

apworks1 and others added 4 commits December 23, 2021 13:12
Fixes erroneous match with service names that have a type that ends with a shorter type.
Given "%NAS._asustor-looksgood_http._tcp.local.",
type "_asustor-looksgood_http._tcp.local." should not match "_http._tcp.local."
@bdraco
Copy link
Copy Markdown
Member

bdraco commented Dec 23, 2021

I fixed the subtypes not being matched correctly. We still do need a test for the original issue

@bdraco bdraco merged commit a4d619a into python-zeroconf:master Dec 24, 2021
@apworks1 apworks1 deleted the patch-1 branch December 24, 2021 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Type containing different type as substring

3 participants