mafintosh / multicast-dns

Low level multicast-dns implementation in pure javascript
MIT License
512 stars 91 forks source link

SRV record causes crash #36

Closed montehansen closed 8 years ago

montehansen commented 8 years ago

I added support for SRV records to the mdns-proxy (which multicast-dns supports), but results in a crash with this error:

resolved:  _kerberos._udp.NETWORK.LOCAL [SRV] [ { name: '_kerberos._udp.NETWORK.LOCAL',
    type: 'SRV',
    ttl: 1800,
    data:
     { name: 'domctrlr.network.local',
       port: 88,
       priority: 0,
       weight: 100 } } ]

C:\Users\mhansen\AppData\Roaming\npm\node_modules\mdns-proxy\node_modules\multicast-dns\packets.js:33
  var list = n.split('.')
              ^
TypeError: Cannot read property 'split' of undefined
    at Object.name.encode (C:\Users\mhansen\AppData\Roaming\npm\node_modules\mdns-proxy\node_modules\multicast-dns\packets.js:33:15)
    at Object.rsrv.encode (C:\Users\mhansen\AppData\Roaming\npm\node_modules\mdns-proxy\node_modules\multicast-dns\packets.js:203:8)
    at Object.answer.encode (C:\Users\mhansen\AppData\Roaming\npm\node_modules\mdns-proxy\node_modules\multicast-dns\packets.js:329:7)
    at encodeList (C:\Users\mhansen\AppData\Roaming\npm\node_modules\mdns-proxy\node_modules\multicast-dns\packets.js:381:9)
    at Object.exports.encode (C:\Users\mhansen\AppData\Roaming\npm\node_modules\mdns-proxy\node_modules\multicast-dns\packets.js:410:12)
    at C:\Users\mhansen\AppData\Roaming\npm\node_modules\mdns-proxy\node_modules\multicast-dns\index.js:62:29
    at apply (C:\Users\mhansen\AppData\Roaming\npm\node_modules\mdns-proxy\node_modules\thunky\index.js:16:28)
    at C:\Users\mhansen\AppData\Roaming\npm\node_modules\mdns-proxy\node_modules\thunky\index.js:27:3
    at EventEmitter.that.send (C:\Users\mhansen\AppData\Roaming\npm\node_modules\mdns-proxy\node_modules\multicast-dns\index.js:60:5)
    at EventEmitter.that.response.that.respond (C:\Users\mhansen\AppData\Roaming\npm\node_modules\mdns-proxy\node_modules\multicast-dns\index.js:73:10)
mafintosh commented 8 years ago

Looks like a bug where you are using .name instead of .target to encode SRV records.

mafintosh commented 8 years ago

If I'm wrong feel free to re-open

montehansen commented 8 years ago

Thank you so much for your response. It's greatly appreciated.

Where would you suspect that to originate from? The source (iOS device), the mdns-proxy, or the multicast-dns packets class?

The mdns-proxy doesn't act differently based on record type, and I fear the rsrv change I made merely masked the symptom.

Monte

The symptomatic change in multicast-dns (undefined test)

rsrv.encode = function(data, buf, offset) {
  buf.writeUInt16BE(data.priority || 0, offset+2)
  buf.writeUInt16BE(data.weight || 0, offset+4)
  buf.writeUInt16BE(data.port || 0, offset+6)
  if( data.target ) // <-- added this test
    name.encode(data.target, buf, offset+8)

  var len = name.encode.bytes+6
  buf.writeUInt16BE(len, offset)

  rsrv.encode.bytes = len+2
  return buf
}

The mdns-proxy code

        // resolve the query using DNS
        dns.resolve(q.name, q.type, function (err, addresses) {
            if (err != null) {
                // on error, don't respond but add entry to our cache
                console.log("not found: ", key, err);
                cache[key] = { time: time, response: null };
            } else {
                // respond with the first address found
                var address = addresses[0];
                if (typeof address === 'undefined') {
                    // on empty record, just add entry to our cache
                    console.log("empty record: ", key, err);
                    cache[key] = { time: time, response: null };
                } else {
                    var response = [{ name: q.name, type: q.type, ttl: ttl, data: address }];
                    console.log("resolved: ", key, response);
                    mdns.response(response);

                    // add entry to our cache
                    cache[key] = { time: time, response: null };
                }
            }
        });
montehansen commented 8 years ago

You were right. Duh, the response didn't conform to the SRV record spec. Thanks. Been a while since I coded in the DNS world =).