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

TXT records with periods in them are mistaken for multiple txt records #84

Open
haakonnessjoen opened this issue Jul 2, 2019 · 6 comments

Comments

@haakonnessjoen
Copy link

As previously defined in closed issue #70:

TXT records are split if they have periods in the value part. This is allowed by spec, as all 8bit values should be allowed after =.

		this.advertisement = mdns.createAdvertisement(mdns.tcp('myproto'), 1234, {
			name: 'myproto-app',
			txt: {
				protocol_version: '1.0.0',
				network_uid: this.options.network_uid || '',
				application_name: this.options.applicationName
			}
		});

The resulting record is:

{ addresses: [ '10.0.0.x' ],
  query: [],
  port: 1234,
  fullname: 'myproto-app._myproto._tcp.local',
  txt:
   [ 'protocol_version=1',
     '0',
     '0',
     'network_uid=abcdef0123456',
     'application_name=my-testscript' ],
  type:
   [ { name: 'myproto',
       protocol: 'tcp',
       subtypes: [],
       description: undefined },
     { name: 'myproto',
       protocol: 'tcp',
       subtypes: [],
       description: undefined } ],
  host: 'myproto-app.local',
  interfaceIndex: 1,
  networkInterface: 'pseudo multicast' }

As you see, protocol version should be protocol_version=1.0.0, but instead it is incorrectly split up in multiple txt records.

@phatpaul
Copy link

I debugged the javascript, and the period is intact when the data is handed over to this.networking.send(packet);

Looking at the wireshark dump, the period is sent as a 0x04 (but should be ASCII 0x2E).
It seems 0x04 is used elsewhere in the protocol to separate parts like in:
myproto-app._myproto._tcp.local
The first period is sent as 0x04 and the second period is sent as 0x05.

I compared with wireshark the packet sent by a working mdns implementation vs. this one. The working implementation correctly sends 0x2e for a period within a txt record.
I'll keep digging...

@phatpaul
Copy link

I traced it to another module node-dns-js, and it looks like you already fixed it.
mdns-js/node-dns-js#11
How can I get that fix, do you have a fork I can use in npm?

@kmpm
Copy link
Collaborator

kmpm commented Aug 12, 2022

@phatpaul, could you test the code in PR #85.

@phatpaul
Copy link

Glad to see someone is still here.
I just tested PR #85

I had previously fixed my issue with TXT records by doing:
npm i --save-dev bitfocus/node-dns-js

But to test your new PR I removed that one first.

npm r dns-js
npm i --save-dev mdns-js/node-mdns-js#pull/85/head

Your new code works as much as it did before, but still fails on the periods within TXT records.

BTW, mdns-js/node-dns-js#11 did fix it.

@kmpm
Copy link
Collaborator

kmpm commented Aug 17, 2022

@phatpaul - please try again

@phatpaul
Copy link

Yes, that fixed the issue with TXT records. But I'm still not able to get this to work with my application... I'm going back to node-mdns and deal with the tricky install.

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

No branches or pull requests

3 participants