GNOME Bugzilla – Bug 730352
Use inet_pton(), if_nametoindex() and if_indextoname() for newer versions of Windows
Last modified: 2015-06-23 10:55:45 UTC
Hi, As the Winsock2 library on Windows Vista (and later) support more functions that the Unix socket library provides, such as inet_pton(), if_nametoindex(), if_indextoname(), probably we should make use of them when we are building against Vista or later (i.e. when _WIN32_WINNT >= 0x0600). However, as there is still intention to support XP/Server 2003[1], we still want to keep the former code paths that is used around and working. [1]: https://mail.gnome.org/archives/gtk-devel-list/2013-October/msg00149.html
Created attachment 276743 [details] [review] gio/ginetaddress.c on Windows: Use inet_pton() On Newer Windows, and fix when inet_pton() is not available. Hi, This attempts to update gio/ginetaddress.c in two ways, by using inet_pton() when we are building against Windows Vista and later, and fixing the existing (now for XP/Server 2003) code path to add an extra check via Winsock2's getaddrinfo() to not try to acquire a GInetAddress() when the IPv4 address is actually invalid[1]. [1]: https://bugzilla.gnome.org/show_bug.cgi?id=724609#c3
Hi, I have updated config.h.win32.in so that we can use inet_pton(), if_indextoname() and if_nametoindex() on Windows, by channging the _WIN32_WINNT value that is defined in there, if os desired. With blessings, thank you!
Created attachment 277020 [details] [review] Use inet_pton() on Windows when possible Hi, It seemed that the XP and pre-XP fallback used in g_inet_address_new_from_string() needs more investigation rather than just using getaddrinfo() there, as it broke other tests like inet-address and simple-proxy. So, this just updates the code to use inet_pton() when building against Vista or later. With blessings, thank you!
fwiw, I wouldn't mind killing off XP...
Hello Ryan, I'd second with you too, but just wanted to know from how the Windows users/devs of GLib think about not supporting XP though. My short take on this :) With blessings.
One point of data: our support for XP means that we have some very ugly hacks in the thread code in order to emulate the new (nice) mutex/cond primitives available on Vista and later... The code to fallback to this compatibility case is complex and ugly. Worse is that we have to do it at runtime, because code built on a newer version may be copied to an older machine. Removing it would be nice...
One more point: the monotonic clock stuff becomes a lot easier on Vista and later...
Yup (with those two parts!) ... :)
...and there is also bug 729858, where I tried to improve GDateTime a bit, we wouldn't have to worry so much about the language of Windows that GLib is running on in regards to GDateTime. Would be nice if someone would look at it as well. With blessings, thank you!
Comment on attachment 277020 [details] [review] Use inet_pton() on Windows when possible ignoring the larger question of whether to drop XP support, this patch seems fine
Created attachment 294387 [details] [review] ginetaddress.c: Use inet_ntop() and inet_pton() on Windows when available Hi, I thought it might be a better idea for now, since we are keeping XP support for now, that we use a dynamic run-time way of figuring out whether we have inet_ntop() and inet_pton() available to us. So, this is the patch for this-I will attach another patch for if_nametoindex() in a bit, which is in another file. With blessings, thank you!
Created attachment 294388 [details] [review] gsocket.c: Windows: Use the system's implementation of if_nametoindex() when available Hi, Like the previous patch, this uses the system's implementation of if_nametoindex(), when we have it (i.e. Vista or later). With blessings, thank you!
Created attachment 294389 [details] [review] gresolver.c: Fix IPv6 address handling on Windows Hi, I am attaching another patch for gresolver.c, as we need a check in handle_ip_address() for Windows, which is similar to the one that is used for checking the IPv4 addresses on non-Windows. Interestingly, getaddrinfo() on Windows accepts non-standard hex numbers-and-colons IPv6 addresses (vs rejecting non-standard number-and-dots IPv4 addresses), so we want to reject such IPv6 addresses that getaddrinfo() accepts, especially as if we went by the inet_pton() route for Vista and later, valid IPv6 addresses would have already been accepted in g_inet_address_new_from_string() (can't tell about the XP case though, I don't run it nowadays). As we still want to accept IPv6 addresses with a scope ID (as in the network-address test), accept the address that has them. This patch, together with attachment 294387 [details] [review], would enable the network-address test to pass on Windows, at least Vista and later. With blessings, thank you!
(In reply to comment #11) > Created an attachment (id=294387) [details] [review] > ginetaddress.c: Use inet_ntop() and inet_pton() on Windows when available This makes g_inet_address_new_from_string() / g_inet_address_to_string() pretty messy. It would be nice if you could split the Windows stuff out into separate functions. And maybe you could do all of the function-looking-up stuff from g_networking_init() rather than scattering it around multiple files? (In reply to comment #13) > Interestingly, getaddrinfo() on > Windows accepts non-standard hex numbers-and-colons IPv6 addresses What sort of non-standard addresses do you mean? I don't know of any non-standard-but-accepted IPv6 address formats.
Created attachment 294497 [details] [review] Windows: Use inet_pton(), inet_ntop() and if_nametoindex() if available Hello Dan, Here comes the patch for querying and using the functions directly from the system's Winsock2 implementation if they are available, as suggested. Hopefully this makes the situation a bit cleaner. (In reply to comment #14) > What sort of non-standard addresses do you mean? I don't know of any > non-standard-but-accepted IPv6 address formats. Like the ones below (taken from network-address.c): /* GResolver accepts this by ignoring the scope ID. This was not * intentional, but it's best to not "fix" it at this point. */ { "fe80::42%1", TRUE, TRUE, FALSE }, /* g_network_address_parse() accepts these, but they are not * (just) IP addresses. */ { "192.168.1.2:80", TRUE, FALSE, FALSE }, { "[fe80::42]", TRUE, FALSE, FALSE }, { "[fe80::42]:80", TRUE, FALSE, FALSE }, where the last 3 addresses should not have been resolved in a valid way by GResolver (but the can be parsed by g_network_address_parse()). As mentioned in gresolve.c, getaddrinfo() on Windows rejects 192.168.1.2:80, but interestingly accepts [fe80::42] and [fe80::42]:80 (where all these addresses are rejected by inet_pton() on Windows in g_inet_address_new_from_string()). The one with the scope ID is however accepted to be in line with what happens on other platforms, until the time that we want to fix this. I can't say much about the XP case since I don't normally access to it though. So, this is a bit like the situation on non-Windows that used inet_aton() to check the IPv4 addresses after it is rejected by inet_pton(), by rejecting the addresses accepted by inet_aton(). With blessings, thank you!b
(In reply to comment #15) > (In reply to comment #14) > > What sort of non-standard addresses do you mean? I don't know of any > > non-standard-but-accepted IPv6 address formats. ... > As mentioned in gresolve.c, getaddrinfo() on Windows rejects 192.168.1.2:80, > but interestingly accepts [fe80::42] and [fe80::42]:80 (where all these > addresses are rejected by inet_pton() on Windows in > g_inet_address_new_from_string()). Ah, ok, this is not the same thing as the IPv4 case then. "Non-standard numbers and dots addresses" refers to IPv4 addresses that don't consist of 4 single-byte base 10 numbers. (ie, the ones listed as "These should not be considered IP addresses by anyone." in network-address.c). In your case, the comment should be talking about rejecting addresses containing brackets and/or port numbers, not rejecting non-standard numbers-and-dots addresses. However, the patch is wrong as-is, because getaddrinfo() will accept non-IP-address strings as well, so this would reject, eg "ipv6.google.com". But anyway, there's no need to call getaddrinfo() here anyway; just check if the string has a "[" or "]" in it, and reject it if so (since no valid IP address or hostname could have those characters in it).
Created attachment 294837 [details] [review] gresolver.c: Fix IPv6 address handling on Windows [take ii] Hello Dan, Thanks for the info for me to learn new item... So, this is the patch that checks for the presence of '[' or ']'. Is the other patch for using inet_pton() and etc alright though? With blessings, thank you!
Hi, Are the latest versions of the patches ok, as it would be nice for newer Windows releases to make use of the newer networking APIs that would match with the ones used on other systems? With blessings, thank you!
(In reply to Fan, Chun-wei from comment #18) > Are the latest versions of the patches ok, as it would be nice for newer > Windows releases to make use of the newer networking APIs that would match > with the ones used on other systems? the gresolver.c one looks fine. For the inet_pton, etc, stuff, I was thinking more that you could just implement inet_pton() and inet_ntop() functions inside ginetaddress.c, where, eg, inet_pton() would call ws2funcs.pInetPton() if you had it, or else fall back to using WSAStringToAddress() if not. And then g_inet_address_new_from_string() wouldn't need any unix-vs-win32 #ifdefs; it would just always call inet_pton(). (And then likewise with inet_ntop() and g_inet_address_to_string().) And then in the future when we stop supporting versions of Windows that don't have inet_pton, we could just remove the definitions in ginetaddress.c.
Created attachment 298607 [details] [review] Windows: Use inet_pton(), inet_ntop() and if_nametoindex() if available (take ii) Hello Dan, Here comes the updated patch for the implementation of inet_pton() et al on Windows, which does the former Win32 code path on XP/Server 2003 and uses the native Winsock2 implementations of them when Winsock2 provides them, as you suggested. The inet_pton() emulation for XP/Server 2003 is not complete (also as in the original implementation) as WSAStringToAddress() would accept things like 192.168.258, which means something along the lines of a regex check or so is needed to make it work correctly, given that it comes to me that this code path is largely interim as XP support is going to be dropped pretty soon, and would probably be better be in another bug. Thanks for the notes, and thanks for the reviews. With blessings, thank you! p.s. I have pushed attachment 294837 [details] [review] as b9c8cec to master and as 4421d92 to glib-2-42 as both of these branches are affected by this.
Comment on attachment 298607 [details] [review] Windows: Use inet_pton(), inet_ntop() and if_nametoindex() if available (take ii) looks good
Review of attachment 298607 [details] [review]: Hello Dan, Thanks for the review, the patch was committed as: master: 6fe28ee glib-2-42: 83bf89c I will close this bug shortly. With blessings, thank you!
*** Bug 739656 has been marked as a duplicate of this bug. ***
This patch breaks remote-viewer (windows build) on a (at least) Windows XP machine. Before this patch a connection to a virtual machine works properly. After applying this patch, I started getting https://msdn.microsoft.com/en-us/library/ms847615.aspx as an error message for a g_pollable_output_stream_write_nonblocking().
And the patch in questions is: 6fe28eef3ce5dd7ce12c6ace080bac5d9479f50c
(In reply to Fabiano Fidêncio from comment #24) > This patch breaks remote-viewer (windows build) on a (at least) Windows XP > machine. Regardless of this specific issue — which should still be investigated — I'm not sure a fix will do you much help: Windows XP support is being phased out in both GLib and GTK.
Hi Fabiano, Can you try the patch in commit 675dbae3029ff95d94cdbd513906c98e1ad767ad and see whether things are okay for you there? As Emmanuele mentioned, as of the current state of the master branch, XP support has been phased out for GLib, so this means that I am presuming that you are using 2.44.x. Please let me know how things went. With blessings, thank you!
Hi Fabiano, By the way, since Vista+ is using proper inet_pton(), which was added to Winsock2 since NT6.x, so I don't think the issue in comment #24 should hit there. Can you let me know how things went for you there also? With blessings, thank you!
(In reply to Fan, Chun-wei from comment #27) Fan, > Hi Fabiano, > > Can you try the patch in commit 675dbae3029ff95d94cdbd513906c98e1ad767ad and > see whether things are okay for you there? As Emmanuele mentioned, as of > the current state of the master branch, XP support has been phased out for > GLib, so this means that I am presuming that you are using 2.44.x. > > Please let me know how things went. The patch does solve the problem! > > With blessings, thank you! Thanks a lot!
Hi Fabiano, Great to hear this. I will close this bug now. With blessings!
(In reply to Fan, Chun-wei from comment #28) Fan, > Hi Fabiano, > > By the way, since Vista+ is using proper inet_pton(), which was added to > Winsock2 since NT6.x, so I don't think the issue in comment #24 should hit > there. Can you let me know how things went for you there also? I've tested on Vista and Windows 7 and didn't hit the problem. Sorry for a late answer, but took some time to install those systems here :-) > > With blessings, thank you! Best Regards and thanks a lot!