[glib/wip/tingping/network-address-errors] gnetworkaddress: Fix incorrect error propagation when resolving addresses

[Patrick Griffis <pgriffis src gnome org>]

commit d53c981bfd3bfc773655c7622341b4fcc5351b16
Author: Patrick Griffis <tingping tingping se>
Date:   Thu Feb 7 08:37:32 2019 -0500

    gnetworkaddress: Fix incorrect error propagation when resolving addresses
    
    Previously this would always error if ipv6 errored after ipv4
    succeeded which was incorrect.
    
    This explicitly tests the order of erroring and also handles the
    case where ipv6 succeeds with no data by waiting on ipv4.
    
    Fixes #1644
    Fixes #1680

 gio/gnetworkaddress.c       | 23 +++++++++----
 gio/tests/meson.build       |  2 +-
 gio/tests/network-address.c | 84 ++++++++++++++++++++++++++++++++++++++++-----
 3 files changed, 94 insertions(+), 15 deletions(-)
---
diff --git a/gio/gnetworkaddress.c b/gio/gnetworkaddress.c
index 60736874e..ca3b626d0 100644
--- a/gio/gnetworkaddress.c
+++ b/gio/gnetworkaddress.c
@@ -1143,6 +1143,8 @@ got_ipv6_addresses (GObject      *source_object,
   GResolver *resolver = G_RESOLVER (source_object);
   GList *addresses;
   GError *error = NULL;
+  gboolean got_ipv4_first = FALSE;
+  gboolean got_data = FALSE;
 
   addresses = g_resolver_lookup_by_name_with_flags_finish (resolver, result, &error);
   if (!error)
@@ -1150,6 +1152,7 @@ got_ipv6_addresses (GObject      *source_object,
       /* Regardless of which responds first we add them to the enumerator
        * which does mean the timing of next_async() will potentially change
        * the results */
+      got_data = addresses != NULL;
       g_network_address_add_addresses (addr_enum->addr, g_steal_pointer (&addresses),
                                        g_resolver_get_serial (resolver));
     }
@@ -1161,18 +1164,18 @@ got_ipv6_addresses (GObject      *source_object,
     {
       g_source_destroy (addr_enum->wait_source);
       g_clear_pointer (&addr_enum->wait_source, g_source_unref);
+      got_ipv4_first = TRUE;
     }
 
-  /* If we got an error before ipv4 then let it handle it.
+  /* If we got an error or no data before ipv4 then let its response handle it.
    * If we get ipv6 response first or error second then
    * immediately complete the task.
    */
-  if (error != NULL && !addr_enum->last_error)
+  if ((error != NULL || !got_data) && !addr_enum->last_error && !got_ipv4_first)
     {
       addr_enum->last_error = g_steal_pointer (&error);
 
-      /* This shouldn't happen often but avoid never responding. */
-      addr_enum->wait_source = g_timeout_source_new_seconds (1);
+      addr_enum->wait_source = g_timeout_source_new (50);
       g_source_set_callback (addr_enum->wait_source,
                              on_address_timeout,
                              addr_enum, NULL);
@@ -1180,11 +1183,19 @@ got_ipv6_addresses (GObject      *source_object,
     }
   else if (addr_enum->queued_task != NULL)
     {
+      GError *task_error = NULL;
+
+      /* If both errored just use the ipv6 one,
+         but if ipv6 errored and ipv4 didn't we don't error */
+      if (error != NULL && addr_enum->last_error)
+          task_error = g_steal_pointer (&error);
+
       g_clear_error (&addr_enum->last_error);
       complete_queued_task (addr_enum, g_steal_pointer (&addr_enum->queued_task),
-                            g_steal_pointer (&error));
+                            task_error);
     }
-  else if (error != NULL)
+
+  if (error != NULL)
     g_clear_error (&error);
 
   g_object_unref (addr_enum);
diff --git a/gio/tests/meson.build b/gio/tests/meson.build
index 7f47f62b6..ebd04461f 100644
--- a/gio/tests/meson.build
+++ b/gio/tests/meson.build
@@ -55,7 +55,7 @@ gio_tests = {
   'memory-output-stream' : {},
   'monitor' : {},
   'mount-operation' : {},
-  'network-address' : {'extra_sources': ['mock-resolver.c'], 'suite': ['flaky']},
+  'network-address' : {'extra_sources': ['mock-resolver.c']},
   'network-monitor' : {},
   'network-monitor-race' : {},
   'permission' : {},
diff --git a/gio/tests/network-address.c b/gio/tests/network-address.c
index 4a2718e24..99913e899 100644
--- a/gio/tests/network-address.c
+++ b/gio/tests/network-address.c
@@ -710,8 +710,24 @@ test_happy_eyeballs_slow_connection_and_ipv4 (HappyEyeballsFixture *fixture,
 }
 
 static void
-test_happy_eyeballs_ipv6_error (HappyEyeballsFixture *fixture,
-                                gconstpointer         user_data)
+test_happy_eyeballs_ipv6_no_data (HappyEyeballsFixture *fixture,
+                                  gconstpointer         user_data)
+{
+  AsyncData data = { 0 };
+
+  data.loop = fixture->loop;
+  mock_resolver_set_ipv6_results (fixture->mock_resolver, NULL);
+  mock_resolver_set_ipv4_delay_ms (fixture->mock_resolver, 25);
+
+  g_socket_address_enumerator_next_async (fixture->enumerator, NULL, got_addr, &data);
+  g_main_loop_run (fixture->loop);
+
+  assert_list_matches_expected (data.addrs, fixture->input_ipv4_results);
+}
+
+static void
+test_happy_eyeballs_ipv6_error_ipv4_first (HappyEyeballsFixture *fixture,
+                                           gconstpointer         user_data)
 {
   AsyncData data = { 0 };
   GError *ipv6_error;
@@ -721,6 +737,7 @@ test_happy_eyeballs_ipv6_error (HappyEyeballsFixture *fixture,
   data.loop = fixture->loop;
   ipv6_error = g_error_new_literal (G_IO_ERROR, G_IO_ERROR_TIMED_OUT, "IPv6 Broken");
   mock_resolver_set_ipv6_error (fixture->mock_resolver, ipv6_error);
+  mock_resolver_set_ipv6_delay_ms (fixture->mock_resolver, 25);
 
   g_socket_address_enumerator_next_async (fixture->enumerator, NULL, got_addr, &data);
   g_main_loop_run (fixture->loop);
@@ -731,8 +748,52 @@ test_happy_eyeballs_ipv6_error (HappyEyeballsFixture *fixture,
 }
 
 static void
-test_happy_eyeballs_ipv4_error (HappyEyeballsFixture *fixture,
-                                gconstpointer         user_data)
+test_happy_eyeballs_ipv6_error_ipv6_first (HappyEyeballsFixture *fixture,
+                                           gconstpointer         user_data)
+{
+  AsyncData data = { 0 };
+  GError *ipv6_error;
+
+  /* If ipv6 fails we get an error. */
+
+  data.loop = fixture->loop;
+  ipv6_error = g_error_new_literal (G_IO_ERROR, G_IO_ERROR_TIMED_OUT, "IPv6 Broken");
+  mock_resolver_set_ipv6_error (fixture->mock_resolver, ipv6_error);
+  mock_resolver_set_ipv4_delay_ms (fixture->mock_resolver, 25);
+
+  g_socket_address_enumerator_next_async (fixture->enumerator, NULL, got_addr, &data);
+  g_main_loop_run (fixture->loop);
+
+  assert_list_matches_expected (data.addrs, fixture->input_ipv4_results);
+
+  g_error_free (ipv6_error);
+}
+
+static void
+test_happy_eyeballs_ipv4_error_ipv4_first (HappyEyeballsFixture *fixture,
+                                           gconstpointer         user_data)
+{
+  AsyncData data = { 0 };
+  GError *ipv4_error;
+
+  /* If ipv4 fails we still get ipv6. */
+
+  data.loop = fixture->loop;
+  ipv4_error = g_error_new_literal (G_IO_ERROR, G_IO_ERROR_TIMED_OUT, "IPv4 Broken");
+  mock_resolver_set_ipv4_error (fixture->mock_resolver, ipv4_error);
+  mock_resolver_set_ipv6_delay_ms (fixture->mock_resolver, 25);
+
+  g_socket_address_enumerator_next_async (fixture->enumerator, NULL, got_addr, &data);
+  g_main_loop_run (fixture->loop);
+
+  assert_list_matches_expected (data.addrs, fixture->input_ipv6_results);
+
+  g_error_free (ipv4_error);
+}
+
+static void
+test_happy_eyeballs_ipv4_error_ipv6_first (HappyEyeballsFixture *fixture,
+                                           gconstpointer         user_data)
 {
   AsyncData data = { 0 };
   GError *ipv4_error;
@@ -742,6 +803,7 @@ test_happy_eyeballs_ipv4_error (HappyEyeballsFixture *fixture,
   data.loop = fixture->loop;
   ipv4_error = g_error_new_literal (G_IO_ERROR, G_IO_ERROR_TIMED_OUT, "IPv4 Broken");
   mock_resolver_set_ipv4_error (fixture->mock_resolver, ipv4_error);
+  mock_resolver_set_ipv4_delay_ms (fixture->mock_resolver, 25);
 
   g_socket_address_enumerator_next_async (fixture->enumerator, NULL, got_addr, &data);
   g_main_loop_run (fixture->loop);
@@ -913,10 +975,16 @@ main (int argc, char *argv[])
               happy_eyeballs_setup, test_happy_eyeballs_very_slow_ipv6, happy_eyeballs_teardown);
   g_test_add ("/network-address/happy-eyeballs/slow-connection-and-ipv4", HappyEyeballsFixture, NULL,
               happy_eyeballs_setup, test_happy_eyeballs_slow_connection_and_ipv4, happy_eyeballs_teardown);
-  g_test_add ("/network-address/happy-eyeballs/ipv6-error", HappyEyeballsFixture, NULL,
-              happy_eyeballs_setup, test_happy_eyeballs_ipv6_error, happy_eyeballs_teardown);
-  g_test_add ("/network-address/happy-eyeballs/ipv4-error", HappyEyeballsFixture, NULL,
-              happy_eyeballs_setup, test_happy_eyeballs_ipv4_error, happy_eyeballs_teardown);
+  g_test_add ("/network-address/happy-eyeballs/ipv6-no-data", HappyEyeballsFixture, NULL,
+              happy_eyeballs_setup, test_happy_eyeballs_ipv6_no_data, happy_eyeballs_teardown);
+  g_test_add ("/network-address/happy-eyeballs/ipv6-error-ipv4-first", HappyEyeballsFixture, NULL,
+              happy_eyeballs_setup, test_happy_eyeballs_ipv6_error_ipv4_first, happy_eyeballs_teardown);
+  g_test_add ("/network-address/happy-eyeballs/ipv6-error-ipv6-first", HappyEyeballsFixture, NULL,
+              happy_eyeballs_setup, test_happy_eyeballs_ipv6_error_ipv6_first, happy_eyeballs_teardown);
+  g_test_add ("/network-address/happy-eyeballs/ipv4-error-ipv6-first", HappyEyeballsFixture, NULL,
+              happy_eyeballs_setup, test_happy_eyeballs_ipv4_error_ipv6_first, happy_eyeballs_teardown);
+  g_test_add ("/network-address/happy-eyeballs/ipv4-error-ipv4-first", HappyEyeballsFixture, NULL,
+              happy_eyeballs_setup, test_happy_eyeballs_ipv4_error_ipv4_first, happy_eyeballs_teardown);
   g_test_add ("/network-address/happy-eyeballs/both-error", HappyEyeballsFixture, NULL,
               happy_eyeballs_setup, test_happy_eyeballs_both_error, happy_eyeballs_teardown);
   g_test_add ("/network-address/happy-eyeballs/both-error-delays-1", HappyEyeballsFixture, NULL,