[glib/pgriffis/wip/resolver-https] Review fixups

[Patrick Griffis <pgriffis src gnome org>]

commit f0c933d1baf070d775f7e59808ee943bd58feae1
Author: Patrick Griffis <pgriffis igalia com>
Date:   Wed Dec 15 10:30:42 2021 -0600

    Review fixups

 gio/gioenums.h          |  8 ++---
 gio/gthreadedresolver.c | 78 +++++++++++++++++++++++++++++--------------------
 gio/tests/gresolver.c   |  3 ++
 3 files changed, 54 insertions(+), 35 deletions(-)
---
diff --git a/gio/gioenums.h b/gio/gioenums.h
index 2a44cdc17..4e097624e 100644
--- a/gio/gioenums.h
+++ b/gio/gioenums.h
@@ -759,13 +759,13 @@ typedef enum {
  * `(qsa{sv})`, representing the priority, target host, and params of the domain.
  * The keys of the params dictionary are:
  *  - `alpn`: array of strings of protocol names
- *  - `no-default-alpn`: an empty string
- *  - `port`: uint16
+ *  - `no-default-alpn`: an empty string if present
+ *  - `port`: uint16 (0-65535)
  *  - `ipv4hint`: array of strings of addresses
  *  - `ipv6hint`: array of strings of addresses
- *  - `ech`: string of base64 data containing an ECHConfigList defined 
[here](https://datatracker.ietf.org/doc/draft-ietf-tls-esni/)
+ *  - `ech`: byte array of data containing an ECHConfigList defined 
[here](https://datatracker.ietf.org/doc/draft-ietf-tls-esni/)
  *  - `mandatory`: array of strings matching these keys
- *  - `keyN`: for unknown keys, N is the key number, the value is a bytestring of unparsed data
+ *  - `keyN`: for unknown keys, N is the numeric value of the key type, the value is a byte array of 
unparsed data
  * See the [RFC](https://datatracker.ietf.org/doc/draft-ietf-dnsop-svcb-https/) for more information.
  *
  * Since: 2.34
diff --git a/gio/gthreadedresolver.c b/gio/gthreadedresolver.c
index acc10f0a4..4082c460b 100644
--- a/gio/gthreadedresolver.c
+++ b/gio/gthreadedresolver.c
@@ -673,7 +673,9 @@ svcb_key_to_string (SVCBKey key)
 }
 
 static gchar *
-get_uncompressed_domain (guchar *src, guchar *end, guchar **out)
+get_uncompressed_domain (guchar        *src,
+                         const guchar  *end,
+                         guchar       **out)
 {
   GString *target = g_string_new (NULL);
 
@@ -704,7 +706,8 @@ get_uncompressed_domain (guchar *src, guchar *end, guchar **out)
 }
 
 static GVariant *
-get_svcb_ipv4_hint_value (const guchar *src, const guchar *end, guint16 length)
+get_svcb_ipv4_hint_value (const guchar *src,
+                          guint16       length)
 {
   GVariantBuilder builder;
 
@@ -722,7 +725,8 @@ get_svcb_ipv4_hint_value (const guchar *src, const guchar *end, guint16 length)
 }
 
 static GVariant *
-get_svcb_ipv6_hint_value (const guchar *src, const guchar *end, guint16 length)
+get_svcb_ipv6_hint_value (const guchar *src,
+                          guint16       length)
 {
   GVariantBuilder builder;
 
@@ -742,41 +746,42 @@ get_svcb_ipv6_hint_value (const guchar *src, const guchar *end, guint16 length)
 }
 
 static GVariant *
-get_svcb_alpn_value (const guchar *src, const guchar *end, guint16 length)
+get_svcb_alpn_value (const guchar *src,
+                     guint16       length)
 {
   GVariantBuilder builder;
   GString *alpn_id = NULL;
-  guchar alpn_id_remaning = 0;
+  guchar alpn_id_remaining = 0;
 
   /* Format defined in Section 6.1 */
   g_variant_builder_init (&builder, G_VARIANT_TYPE_STRING_ARRAY);
 
   /* length prefixed strings */
-  for (; src <= end && length; src++, length--)
+  for (; length > 0; src++, length--)
     {
       guchar c = *src;
 
-      if (alpn_id && !alpn_id_remaning)
+      if (alpn_id != NULL && alpn_id_remaining == 0)
         {
           g_variant_builder_add_value (&builder,
                                        g_variant_new_take_string (g_string_free (g_steal_pointer (&alpn_id), 
FALSE)));
           /* This drops through as the current char is the new length */
         }
 
-      if (!alpn_id)
+      if (alpn_id == NULL)
         {
           /* First byte is length */
-          alpn_id_remaning = c;
-          alpn_id = g_string_sized_new (c + 1);
+          alpn_id_remaining = c;
+          alpn_id = g_string_sized_new (((gsize)c) + 1);
           continue;
         }
 
       g_string_append_c (alpn_id, c);
-      alpn_id_remaning--;
+      alpn_id_remaining--;
     }
 
   /* Trailing value */
-  if (alpn_id)
+  if (alpn_id != NULL)
     {
       g_variant_builder_add_value (&builder,
                                    g_variant_new_take_string (g_string_free (g_steal_pointer (&alpn_id), 
FALSE)));
@@ -786,7 +791,8 @@ get_svcb_alpn_value (const guchar *src, const guchar *end, guint16 length)
 }
 
 static GVariant *
-get_svcb_mandatory_value (const guchar *src, const guchar *end, guint16 length)
+get_svcb_mandatory_value (const guchar *src,
+                          guint16       length)
 {
   GVariantBuilder builder;
 
@@ -808,58 +814,64 @@ get_svcb_mandatory_value (const guchar *src, const guchar *end, guint16 length)
 }
 
 static gboolean
-get_svcb_value (SVCBKey key, guint16 value_length, const guchar *src, const guchar *end, GVariant **value)
+get_svcb_value (SVCBKey        key,
+                guint16        value_length,
+                const guchar  *src,
+                GVariant     **value)
 {
   switch (key)
     {
     case SVCB_KEY_MANDATORY:
-      *value = get_svcb_mandatory_value (src, end, value_length);
+      *value = get_svcb_mandatory_value (src, value_length);
       return TRUE;
     case SVCB_KEY_ALPN:
-      *value = get_svcb_alpn_value (src, end, value_length);
+      *value = get_svcb_alpn_value (src, value_length);
       return TRUE;
     case SVCB_KEY_PORT:
       {
         guint16 port;
         GETSHORT (port, src);
+        /* 0-65535 is valid */
         *value = g_variant_new_uint16 (port);
         return TRUE;
       }
     case SVCB_KEY_ECH:
       {
         guint8 length;
-        gchar *base64_str;
+        GBytes *bytes;
         GETSHORT (length, src);
 
-        if (length > (gsize) (end - src))
+        if (length > (value_length - 1))
           return FALSE;
 
-        base64_str = g_base64_encode (src, length);
-        *value = g_variant_new_take_string (base64_str);
+        bytes = g_bytes_new (src, length);
+        *value = g_variant_new_from_bytes (G_VARIANT_TYPE_BYTESTRING, bytes, FALSE);
+        g_bytes_unref (bytes);
         return TRUE;
       }
     case SVCB_KEY_IPV4HINT:
-      *value = get_svcb_ipv4_hint_value (src, end, value_length);
+      *value = get_svcb_ipv4_hint_value (src, value_length);
       return TRUE;
     case SVCB_KEY_IPV6HINT:
-      *value = get_svcb_ipv6_hint_value (src, end, value_length);
+      *value = get_svcb_ipv6_hint_value (src, value_length);
       return TRUE;
     case SVCB_KEY_NO_DEFAULT_ALPN:
-      G_GNUC_FALLTHROUGH;
+      *value = g_variant_new_string ("");
+      return TRUE;
     default:
       {
-        gchar *string = g_strndup ((char *)src, value_length);
-        *value = g_variant_new_bytestring (string);
-        g_free (string);
+        GBytes *bytes = g_bytes_new (src, value_length);
+        *value = g_variant_new_from_bytes (G_VARIANT_TYPE_BYTESTRING, bytes, FALSE);
+        g_bytes_unref (bytes);
         return TRUE;
       }
     }
 }
 
-static GVariant *parse_res_https (guchar  *answer,
-                 guchar  *end,
-                 guchar **p,
-                 GError **error)
+static GVariant *parse_res_https (const guchar  *answer,
+                                  guchar        *end,
+                                  guchar       **p,
+                                  GError       **error)
 {
   GVariant *variant;
   gchar *target;
@@ -900,7 +912,7 @@ static GVariant *parse_res_https (guchar  *answer,
 
           key_str = svcb_key_to_string (key);
 
-          if (!get_svcb_value (key, value_length, *p, end, &value))
+          if (!get_svcb_value (key, value_length, *p, &value))
             goto invalid_input;
 
           g_variant_builder_add (&params_builder, "{sv}", key_str, value);
@@ -1041,11 +1053,15 @@ g_resolver_records_from_res_query (const gchar      *rrname,
 
       if (record != NULL)
         records = g_list_prepend (records, record);
+
+      if (parsing_error)
+        break;
     }
 
   if (parsing_error)
     {
       g_propagate_prefixed_error (error, parsing_error, _("Failed to parse DNS response for “%s”: "), 
rrname);
+      g_list_free_full (records, (GDestroyNotify)g_variant_unref);
       return NULL;
     }
   else if (!records)
diff --git a/gio/tests/gresolver.c b/gio/tests/gresolver.c
index 45ed4a5e0..3ba7dbad2 100644
--- a/gio/tests/gresolver.c
+++ b/gio/tests/gresolver.c
@@ -242,6 +242,7 @@ test_https_invalid_1 (TestData      *fixture,
   g_assert_error (error, G_RESOLVER_ERROR, G_RESOLVER_ERROR_INTERNAL);
   g_assert_null (records);
 
+  g_error_free (error);
   g_string_free (https_answer, TRUE);
 #endif
 }
@@ -275,6 +276,7 @@ test_https_invalid_2 (TestData      *fixture,
   g_assert_error (error, G_RESOLVER_ERROR, G_RESOLVER_ERROR_INTERNAL);
   g_assert_null (records);
 
+  g_error_free (error);
   g_string_free (https_answer, TRUE);
 #endif
 }
@@ -306,6 +308,7 @@ test_https_invalid_3 (TestData      *fixture,
   g_assert_error (error, G_RESOLVER_ERROR, G_RESOLVER_ERROR_INTERNAL);
   g_assert_null (records);
 
+  g_error_free (error);
   g_string_free (https_answer, TRUE);
 #endif
 }