if_nametoindex(...) fails on windows

[agnat]

This is a follow-up to #99.

@y-lohse reported:

C:\Users\caldera\Documents\jdf\node_modules\mdns\lib\network_interface.js:36
      index = dns_sd.if_nametoindex(networkInterface);
                     ^
Error: failed to convert interface alias to luid
    at Error (native)
    at Object.interfaceIndex (C:\Users\caldera\Documents\jdf\node_modules\mdns\l
ib\network_interface.js:36:22)
    at new Browser (C:\Users\caldera\Documents\jdf\node_modules\mdns\lib\browser
.js:15:23)
    at Object.create [as createBrowser] (C:\Users\caldera\Documents\jdf\node_mod
ules\mdns\lib\browser.js:104:10)
    at exports.local advertisement invisible on external interfaces (C:\Users\ca
ldera\Documents\jdf\node_modules\mdns\tests\test_functional.js:377:30)

The interface name involved is Connexion au réseau local 3. It could be a unicode (normalization) issue (Note the accent) or a change in node/libuv. I'll try to review the code path involved but I can't do any actual testing, because I don't have the operating system in question.

[agnat]

The following loop should always work:

var interfaces = os.networkInterfaces();
for (var name in interfaces) {
  var index = dns_sd.if_nametoindex(name);
  t.ok( index > 0);
  t.strictEqual( dns_sd.if_indextoname(index), name);
}

That is, if_nametoindex(...) should cope with names returned by os.networkInterfaces()... but doesn't.

Pointers:

• node GetInterfaceAddresses() (src)
• libuv uv_interface_addresses() (src): Uses GetAdaptersAddresses to get the adapters FriendlyName. Conversion to UTF8 using WideCharToMultiByte

screech

Ok, I think I spotted it. This line in node treats interface names as Latin-1. I'll open an issue.

[agnat]

To finish the little walk-through above:

• The name is converted to javascript using v8::String::NewFromOneByte(...) (doc)
• In node_mdns::if_nametoindex (src) the UTF8 string is converted to unicode using MultiByteToWideChar
• ConvertInterfaceAliasToLuid fails

[ronkorving]

So does this actually fix it, or do we need to wait for Node to fix it?
Also, according to libuv (only checked the Linux variation), an interface is just uv_interface_address_t* address;. I'm not sure whether to interpret that as string or as bytes.

According to http://man7.org/linux/man-pages/man3/getifaddrs.3.html The ifa_name points to the null-terminated interface name. What are the rules here? Should Node interpret this as utf-8?

[donpark]

@ronkorving interface name is ANSI string (7-bit ASCII + codepage specific extra characters).

[agnat]

On 28.10.2014, at 01:54, Ron Korving notifications@github.com wrote:

So does this actually fix it, or do we need to wait for Node to fix it?

The leak fix is just cosmetic. I don’t have proof that it actually is a node bug. The Latin-1 thing just looks suspicious and it is a v0.11 change. Let’s see what they say.

Also, according to libuv (only checked the Linux variation), an interface is just uv_interface_address_t* address;. I'm not sure whether to interpret that as string or as bytes.

No, it’s not a string. It’s an array of structs (and a count). I’m just highlighting the important points in the walk-through.

According to http://man7.org/linux/man-pages/man3/getifaddrs.3.html The ifa_name points to the null-terminated interface name. What are the rules here? Should Node interpret this as utf-8?

All unix systems I’ve seen use 7 Bit ASCII strings (en0, eth0, &c. <-not an interface). So it’s OK to interpret them as UTF-8.

—
Reply to this email directly or view it on GitHub.

[agnat]

Sorry, that is confusing. I mean the output of GetInterfaceAddresses(...) is an array of structs.
uv_interface_address_t * is just a pointer to one of them...

[ronkorving]

If it's always gonna be 7-bit ASCII, isn't this the real problem: "In node_mdns::if_nametoindex (src) the UTF8 string is converted to unicode using MultiByteToWideChar" ? Maybe I'm missing something, I'm no V8 or libuv expert.

[agnat]

Uhm... no.

On unix interface names are always 7 bit. It is ok to use 7 bit ASCII as UTF-8. UTF-8 is designed that way. The unix/posix path is very compact:

NAN_METHOD(if_nametoindex) {
    NanScope();
    /* skipped arg checks */
    String::Utf8Value interfaceName(args[0]->ToString());  // 7 bit ASCII ...
    unsigned int index = ::if_nametoindex(*interfaceName); // ... used directly
    /* skipped error handling */
    NanReturnValue( NanNew<Integer>(index));
}

On windows, however, interface names are full unicode. The incoming javascript UTF-8 string is converted to wide char unicode using MultiByteToWideChar to call ConvertInterfaceAliasToLuid.

If it is not the Latin-1 thing it could be a unicode normalization issue. In unicode some strings (e.g. strings with accents) have more than one representation. That makes string comparison ... non trivial. Apparently ConvertInterfaceAliasToLuid fails to match the string. The process to resolve these ambiguities is called unicode normalization. On windows it is done using NormalizeString.

I hope that is a better explanation.

[ronkorving]

Understood. Thanks!

[agnat]

Reporting node bugs feels like talking to an empty closet. 7 days and still no response.

[y-lohse]

Just to be clear, even though I reported this bug, I don't actually care :p. It only seems to affect service advertising on network interfaces with utf8 names and we're doing none of those. If you think it's better to push the release as it is and fix this bug later, that's totally fine with me.

[agnat]

Hehe... just to be clear: I don't care much either. So @ronkorving, what do you think? Release it as is or wait on this?

The report, though, is excellent and I'm sure the issue will be resolved... one way or another.