Created attachment 245804 [details] [review]
gtest: add function for testing for WINE

We have some testcases that are failing due to what appears to be
limitations in WINE.  Introduce a function called g_test_is_under_wine()
in order to determine if we are running under WINE so that we can
disable some testcases.

Created attachment 245805 [details] [review]
inet-address: disable some testcases under WINE

WINE appears not to parse IPv6 addresses containing '::' properly, so
disable some tests of this functionality.

Comment on attachment 245805 [details] [review]
inet-address: disable some testcases under WINE

>WINE appears not to parse IPv6 addresses containing '::' properly, so
>disable some tests of this functionality.

Not true. In fact, WINE's own test suite contains exactly the same test that is failing here... The bug must be in glib.

(Though I would swear this test used to pass...)

Comment on attachment 245804 [details] [review]
gtest: add function for testing for WINE

Although the specific case in this bug was wrong, there certainly are things that fail under WINE, but not Windows. So this might be useful. Although I feel like it should be versioned, since hopefully the things that don't work in WINE now will be fixed in later versions. Figuring out the WINE version would obviously be tricky though since there's not even a real API for determining if we're under WINE...

>+      g_once_init_leave (&result_plus_1, result + 1);
>+    }
>+
>+  return result_plus_1 - 1;

Ugh. Just use a separate "inited" variable.

Created attachment 245883 [details] [review]
ginetaddress: fix addr/string conversions on windows

When parsing an address, we need to re-set "len" between IPv4 and
IPv6, since WSAStringToAddress() might set it to sizeof(struct sin_addr)
when trying to parse the string as IPv4, even if it fails. Also, we
need to make sure to not pass strings to WSAStringToAddress() that it
will accept but that we don't want it to.

When stringifying an address, we need to clear the sockaddr before
filling it in, so we don't accidentally end up with an unwanted
scope_id or the like.

Review of attachment 245883 [details] [review]:

This causes the testcase to pass for me.

Your comment about WINE suggests that it may be possible to get it to accept an invalid string as a inet6 address via our API, which seems a bit worrying...

Created attachment 246111 [details] [review]
tests: add a few more invalid IPv6 address tests

Created attachment 246112 [details] [review]
tests: add a few more invalid IPv6 address tests

An off-by-1 made the last iteration of this patch less useful than it should have been.

Anyway... these tests pass for both wine and Linux native, so I guess I'm happy... the comment still seems confusing to me, though -- if the Linux native functions are rejecting a "[0:1:2:3:4:5:6:7]" type string then why does wine have to do extra checks?

It's just that WSAStringToAddress() doesn't have exactly the right semantics for g_inet_address_new_from_string(); we only want to accept IP addresses, but it also accepts IP address + port combos (eg, 127.0.0.1:80 or [fe80::1]:22). Alternatively, we could just call g_host_is_ip_address() on @string before passing it to WSAStringToAddress()...

The WINE-specific bit is that when checking for the IPv6+port form, it looks for the "]:", and if it finds it, it skips over the first character of the address without actually checking that it's "[", so you could put anything there and it would still accept it.

Review of attachment 245883 [details] [review]:

Approved, but maybe update the comment to be as clear as your comments on bugzilla are? (ie: the part about wine's behaviour with ']:').

pushed, along with your added tests, plus two more tests of the
specific kinds of strings that WSAStringToAddress() accepts that we
want to reject.

Attachment 245883 [details] pushed as 59ed934 - ginetaddress: fix addr/string conversions on windows
Attachment 246112 [details] pushed as 74a0340 - tests: add a few more invalid IPv6 address tests

The inet address test is consistently failing in the gnome-ostree CI system:

"GLib-GIO:ERROR:../../../gio/tests/inet-address.c:70:test_parse: assertion failed: (addr != NULL)" }

Dan: looks like you meant "== NULL" in this case?

+  addr = g_inet_address_new_from_string ("204.152.189.116:80");
+  g_assert (addr != NULL);

Matthias backed out the commit for now...

argh. I typed "make" and then ran inet-address again, and it passed, and so I committed it. Of course, "make" doesn't re-build test programs any more, so I was still running the old version. :-/

-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/709.