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

Implement TXT record parsing for properties #3

Closed
wants to merge 8 commits into from

Conversation

clairernovotny
Copy link
Contributor

This is a pretty big PR, mainly because there was some strangeness in the parsing of TXT records. There weren't many pure C# DNS implementations around - most P/Invoked to dnsds.dll.

The DNS code included is a subset of the files here:
http://www.codeproject.com/Articles/23673/DNS-NET-Resolver-C

They're allowed to be used under the codeproject license. The author simply requests that the namespaces of the types not change.

The internals now correctly read the necessary DNS records and can now return the TXT properties as well.

I've also removed the dependency on Rx to make the package self-contained. It provides a configurable scan time and retry count/delay.

Most scanning is less than 1-2 seconds, so it's not generally necessary to keep an open stream.

@saldoukhov
Copy link
Owner

Not sure what "strangeness" in TXT parsing you noticed, but this PR is essentially a full replacement of the current code. Is such situation, you probably want to create your own project.
When creating this one, I have looked at DNS .Net resolver and decided to start from scratch to make the code simpler and easier to understand. This PR goes against this line. You're welcome to create a "targeted" PR addressing the exact issue you're observing or report it and I will take a look.

@saldoukhov saldoukhov closed this Sep 2, 2013
@clairernovotny
Copy link
Contributor Author

Hi,

I know its alot if code, but I figured I'd offer it up to you as I did base it off of yours.

There are two issues with the txt records.

1 Mult values per record are not split.

2 Because of 1, mDNS properties can't be parsed.

Look at ichats messages to see the key/value properties.

You might want to look at how DNS.net parses the txt record. It reads the str Len as a short, then n chats. It repeats for the rest of the txt data segment. Then you have a list of txt values per record.

Up to you; I'm happy to keep the package separate and I'll be sure to give you credit.

Regards,
Oren

@saldoukhov
Copy link
Owner

Ok, I'll try to see what is happening with iChat. Do I understand correctly
that iChat is a sample program from the Bonjour SDK?

On Mon, Sep 2, 2013 at 3:47 PM, Oren Novotny notifications@github.comwrote:

Hi,

I know its alot if code, but I figured I'd offer it up to you as I did
base it off of yours.

There are two issues with the txt records.

1 Mult values per record are not split.

2 Because of 1, mDNS properties can't be parsed.

Look at ichats messages to see the key/value properties.

You might want to look at how DNS.net parses the txt record. It reads the
str Len as a short, then n chats. It repeats for the rest of the txt data
segment. Then you have a list of txt values per record.

Up to you; I'm happy to keep the package separate and I'll be sure to give
you credit.

Regards,
Oren


Reply to this email directly or view it on GitHubhttps://github.com//pull/3#issuecomment-23682133
.

@clairernovotny
Copy link
Contributor Author

It's not just iChat - that's one example. Many/most mDNS/Bonjour reponses contain key/value pairs. You can see an example here:
http://upload.wikimedia.org/wikipedia/commons/e/e0/Bonjour_Browser_window.png

For example, I have a network printer on my LAN on Ethernet. Both _printer._tcp.local. and _pdl-datastream._tcp.local. return data with key value pairs.

According to the mDNS spec though, text record values may contain an =, or the values may be scalar. A txt record string cannot start with an =. Any other char is valid though.

@saldoukhov saldoukhov mentioned this pull request May 7, 2014
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.

2 participants