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 parser: skip a 'Record' on parse error #39

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

asashnov
Copy link

@asashnov asashnov commented Mar 24, 2022

Skipping an unknown Record is much better then ignoring the entire Message.

I'm not sure what caused the parse error. Sometimes it happens, sometime don't, maybe somehow interfere with my local avahi server and used cache in code.

I dumped the example of such UDP payload (QByteArray packet dump):
mDNS_UDP_packet.zip

But whatever the reason of parse error, it makes sense to proceed with records which parsed successfully. Otherwise mDNS browser will not show anything.

Here was my research, comparsion 'offset' in qmdns parser and offset the same records in WireShark:

Received message: QTime("23:00:03.420")
   no queries
   attached records:
    Record(PTR _unipro-laptimer._tcp.local.)4500
    Record(SRV unigo-12345._unipro-laptimer._tcp.local.)120
    Record(NSEC unigo-12345.local.)4500
    Record(? )146821888

44 bytes is difference between IP packet and UDP payload start.

PTR start: 3*16 + 8 - 44  = 12 - OK ( record 0 )
SRV start: 6*16 + 13 - 44 = 65 - OK ( record 1 )
NSEC start: 8*16 + 13 - 44 = 97 - OK ( record 2 )  - NSEC parsed incorrectly
A start: 10*16 + 3 - 44 = 119  : wrong in qmdnsentine: 113 (NSEC wasn't fully processed)

Screenshot from 2022-04-16 23-13-23

Skipping an unknown Record is much better then ignoring the entire Message.
Add a real-life UDP packed data captured from Python zeroconf module.

The current 'master' (0ca8011) incorrectly determines the end of NSEC
record, causing next 'A' record unparsed.
That will allow to skip unknown or incorrectly parsed records, and move one
to a next record to the right place.
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.

1 participant