dns: merge branch 'resolv-search-increase'

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/47

4 files changed, 136 insertions, 31 deletions

diff --git a/shared/nm-utils/nm-macros-internal.h b/shared/nm-utils/nm-macros-internal.h
index 6a419f788f..40e2dd0bc9 100644
--- a/shared/nm-utils/nm-macros-internal.h
+++ b/shared/nm-utils/nm-macros-internal.h
@@ -639,6 +639,9 @@ NM_G_ERROR_MSG (GError *error)
 #define NM_PROPAGATE_CONST(test_expr, ptr) (ptr)
 #endif
 
+#define NM_MAKE_STRV(...) \
+	((const char *const[]) { __VA_ARGS__, NULL })
+
 /*****************************************************************************/
 
 #define _NM_IN_SET_EVAL_1( op, _x, y)           (_x == (y))
diff --git a/src/dns/nm-dns-manager.c b/src/dns/nm-dns-manager.c
index 5ef9a6b83f..8575237742 100644
--- a/src/dns/nm-dns-manager.c
+++ b/src/dns/nm-dns-manager.c
@@ -569,9 +569,9 @@ again:
 }
 
 static char *
-create_resolv_conf (char **searches,
-                    char **nameservers,
-                    char **options)
+create_resolv_conf (const char *const*searches,
+                    const char *const*nameservers,
+                    const char *const*options)
 {
 	GString *str;
 	gsize i;
@@ -581,10 +581,39 @@ create_resolv_conf (char **searches,
 	g_string_append (str, "# Generated by NetworkManager\n");
 
 	if (searches && searches[0]) {
+		gsize search_base_idx;
+
 		g_string_append (str, "search");
+		search_base_idx = str->len;
+
 		for (i = 0; searches[i]; i++) {
+			const char *s = searches[i];
+			gsize l = strlen (s);
+
+			if (   l == 0
+			    || NM_STRCHAR_ANY (s, ch, NM_IN_SET (ch, ' ', '\t', '\n'))) {
+				/* there should be no such characters in the search entry. Also,
+				 * because glibc parser would treat them as line/word separator.
+				 *
+				 * Skip the value silently. */
+				continue;
+			}
+
+			if (search_base_idx > 0) {
+				if (str->len - search_base_idx + 1 + l > 254) {
+					/* this entry crosses the 256 character boundery. Older glibc versions
+					 * would truncate the entry at this point.
+					 *
+					 * Fill the line with spaces to cross the 256 char boundary and continue
+					 * afterwards. This way, the truncation happens between two search entries. */
+					while (str->len - search_base_idx < 257)
+						g_string_append_c (str, ' ');
+					search_base_idx = 0;
+				}
+			}
+
 			g_string_append_c (str, ' ');
-			g_string_append (str, searches[i]);
+			g_string_append_len (str, s, l);
 		}
 		g_string_append_c (str, '\n');
 	}
@@ -613,6 +642,14 @@ create_resolv_conf (char **searches,
 	return g_string_free (str, FALSE);
 }
 
+char *
+nmtst_dns_create_resolv_conf (const char *const*searches,
+                              const char *const*nameservers,
+                              const char *const*options)
+{
+	return create_resolv_conf (searches, nameservers, options);
+}
+
 static gboolean
 write_resolv_conf_contents (FILE *f,
                             const char *content,
@@ -636,9 +673,9 @@ write_resolv_conf_contents (FILE *f,
 
 static gboolean
 write_resolv_conf (FILE *f,
-                   char **searches,
-                   char **nameservers,
-                   char **options,
+                   const char *const*searches,
+                   const char *const*nameservers,
+                   const char *const*options,
                    GError **error)
 {
 	gs_free char *content = NULL;
@@ -700,7 +737,11 @@ dispatch_resolvconf (NMDnsManager *self,
 		return SR_ERROR;
 	}
 
-	success = write_resolv_conf (f, searches, nameservers, options, error);
+	success = write_resolv_conf (f,
+	                             NM_CAST_STRV_CC (searches),
+	                             NM_CAST_STRV_CC (nameservers),
+	                             NM_CAST_STRV_CC (options),
+	                             error);
 	err = pclose (f);
 	if (err < 0) {
 		errnosv = errno;
@@ -741,9 +782,9 @@ _read_link_cached (const char *path, gboolean *is_cached, char **cached)
 
 static void
 update_resolv_conf_no_stub (NMDnsManager *self,
-                            char **searches,
-                            char **nameservers,
-                            char **options)
+                            const char *const*searches,
+                            const char *const*nameservers,
+                            const char *const*options)
 {
 	gs_free char *content = NULL;
 	GError *local = NULL;
@@ -766,9 +807,9 @@ update_resolv_conf_no_stub (NMDnsManager *self,
 
 static SpawnResult
 update_resolv_conf (NMDnsManager *self,
-                    char **searches,
-                    char **nameservers,
-                    char **options,
+                    const char *const*searches,
+                    const char *const*nameservers,
+                    const char *const*options,
                     GError **error,
                     NMDnsManagerResolvConfManager rc_manager)
 {
@@ -1059,7 +1100,6 @@ _collect_resolv_conf_data (NMDnsManager *self,
                            const char **out_nis_domain)
 {
 	NMDnsManagerPrivate *priv;
-	guint i, num, len;
 	NMResolvConfData rc = {
 		.nameservers = g_ptr_array_new (),
 		.searches = g_ptr_array_new (),
@@ -1129,17 +1169,6 @@ _collect_resolv_conf_data (NMDnsManager *self,
 		}
 	}
 
-	/* Per 'man resolv.conf', the search list is limited to 6 domains
-	 * totalling 256 characters.
-	 */
-	num = MIN (rc.searches->len, 6u);
-	for (i = 0, len = 0; i < num; i++) {
-		len += strlen (rc.searches->pdata[i]) + 1; /* +1 for spaces */
-		if (len > 256)
-			break;
-	}
-	g_ptr_array_set_size (rc.searches, i);
-
 	*out_searches = _ptrarray_to_strv (rc.searches);
 	*out_options = _ptrarray_to_strv (rc.options);
 	*out_nameservers = _ptrarray_to_strv (rc.nameservers);
@@ -1431,7 +1460,10 @@ update_dns (NMDnsManager *self,
 	 * guarantee they stay alive. */
 	clear_domain_lists (self);
 
-	update_resolv_conf_no_stub (self, searches, nameservers, options);
+	update_resolv_conf_no_stub (self,
+	                            NM_CAST_STRV_CC (searches),
+	                            NM_CAST_STRV_CC (nameservers),
+	                            NM_CAST_STRV_CC (options));
 
 	/* If caching was successful, we only send 127.0.0.1 to /etc/resolv.conf
 	 * to ensure that the glibc resolver doesn't try to round-robin nameservers,
@@ -1454,7 +1486,12 @@ update_dns (NMDnsManager *self,
 		switch (priv->rc_manager) {
 		case NM_DNS_MANAGER_RESOLV_CONF_MAN_SYMLINK:
 		case NM_DNS_MANAGER_RESOLV_CONF_MAN_FILE:
-			result = update_resolv_conf (self, searches, nameservers, options, error, priv->rc_manager);
+			result = update_resolv_conf (self,
+			                             NM_CAST_STRV_CC (searches),
+			                             NM_CAST_STRV_CC (nameservers),
+			                             NM_CAST_STRV_CC (options),
+			                             error,
+			                             priv->rc_manager);
 			resolv_conf_updated = TRUE;
 			/* If we have ended with no nameservers avoid updating again resolv.conf
 			 * on stop, as some external changes may be applied to it in the meanwhile */
@@ -1479,15 +1516,26 @@ update_dns (NMDnsManager *self,
 		if (result == SR_NOTFOUND) {
 			_LOGD ("update-dns: program not available, writing to resolv.conf");
 			g_clear_error (error);
-			result = update_resolv_conf (self, searches, nameservers, options, error, NM_DNS_MANAGER_RESOLV_CONF_MAN_SYMLINK);
+			result = update_resolv_conf (self,
+			                             NM_CAST_STRV_CC (searches),
+			                             NM_CAST_STRV_CC (nameservers),
+			                             NM_CAST_STRV_CC (options),
+			                             error,
+			                             NM_DNS_MANAGER_RESOLV_CONF_MAN_SYMLINK);
 			resolv_conf_updated = TRUE;
 		}
 	}
 
 	/* Unless we've already done it, update private resolv.conf in NMRUNDIR
 	   ignoring any errors */
-	if (!resolv_conf_updated)
-		update_resolv_conf (self, searches, nameservers, options, NULL, NM_DNS_MANAGER_RESOLV_CONF_MAN_UNMANAGED);
+	if (!resolv_conf_updated) {
+		update_resolv_conf (self,
+		                    NM_CAST_STRV_CC (searches),
+		                    NM_CAST_STRV_CC (nameservers),
+		                    NM_CAST_STRV_CC (options),
+		                    NULL,
+		                    NM_DNS_MANAGER_RESOLV_CONF_MAN_UNMANAGED);
+	}
 
 	/* signal that resolv.conf was changed */
 	if (update && result == SR_SUCCESS)
diff --git a/src/dns/nm-dns-manager.h b/src/dns/nm-dns-manager.h
index ed1974a566..a3e9472e5f 100644
--- a/src/dns/nm-dns-manager.h
+++ b/src/dns/nm-dns-manager.h
@@ -129,4 +129,10 @@ typedef enum {
 
 void nm_dns_manager_stop (NMDnsManager *self);
 
+/*****************************************************************************/
+
+char *nmtst_dns_create_resolv_conf (const char *const*searches,
+                                    const char *const*nameservers,
+                                    const char *const*options);
+
 #endif /* __NETWORKMANAGER_DNS_MANAGER_H__ */
diff --git a/src/tests/test-general.c b/src/tests/test-general.c
index 51f9fedbd3..2ab5f8f735 100644
--- a/src/tests/test-general.c
+++ b/src/tests/test-general.c
@@ -29,6 +29,8 @@
 #include "NetworkManagerUtils.h"
 #include "nm-core-internal.h"
 
+#include "dns/nm-dns-manager.h"
+
 #include "nm-test-utils-core.h"
 
 /* Reference implementation for nm_utils_ip6_address_clear_host_address.
@@ -1864,6 +1866,50 @@ test_utils_file_is_in_path (void)
 
 /*****************************************************************************/
 
+#define _TEST_RC(searches, nameservers, options, expected) \
+	G_STMT_START { \
+		const char *const*const _searches = (searches); \
+		const char *const*const _nameservers = (nameservers); \
+		const char *const*const _options = (options); \
+		gs_free char *_content = NULL; \
+		\
+		_content = nmtst_dns_create_resolv_conf (_searches, _nameservers, _options); \
+		g_assert_cmpstr (_content, ==, expected); \
+	} G_STMT_END
+
+static void
+test_dns_create_resolv_conf (void)
+{
+	_TEST_RC (NM_MAKE_STRV ("a"),
+	          NULL,
+	          NULL,
+	          "# Generated by NetworkManager\n"
+	          "search a\n"
+	          "");
+
+	_TEST_RC (NM_MAKE_STRV ("a", "b.com"),
+	          NM_MAKE_STRV ("192.168.55.1", "192.168.56.1"),
+	          NM_MAKE_STRV ("opt1", "opt2"),
+	          "# Generated by NetworkManager\n"
+	          "search a b.com\n"
+	          "nameserver 192.168.55.1\n"
+	          "nameserver 192.168.56.1\n"
+	          "options opt1 opt2\n"
+	          "");
+
+	_TEST_RC (NM_MAKE_STRV ("a2x456789.b2x456789.c2x456789.d2x456789.e2x456789.f2x456789.g2x456789.h2x456789.i2x456789.j2x4567890",
+	                        "a2y456789.b2y456789.c2y456789.d2y456789.e2y456789.f2y456789.g2y456789.h2y456789.i2y456789.j2y4567890",
+	                        "a2z456789.b2z456789.c2z456789.d2z456789.e2z456789.f2z456789.g2z456789.h2z456789.i2z456789.j2z4567890"),
+	          NULL,
+	          NULL,
+	          "# Generated by NetworkManager\n"
+	          "search a2x456789.b2x456789.c2x456789.d2x456789.e2x456789.f2x456789.g2x456789.h2x456789.i2x456789.j2x4567890 a2y456789.b2y456789.c2y456789.d2y456789.e2y456789.f2y456789.g2y456789.h2y456789.i2y456789.j2y4567890                                                        a2z456789.b2z456789.c2z456789.d2z456789.e2z456789.f2z456789.g2z456789.h2z456789.i2z456789.j2z4567890\n"
+	          "");
+
+}
+
+/*****************************************************************************/
+
 NMTST_DEFINE ();
 
 int
@@ -1912,6 +1958,8 @@ main (int argc, char **argv)
 
 	g_test_add_func ("/general/test_utils_file_is_in_path", test_utils_file_is_in_path);
 
+	g_test_add_func ("/general/test_dns_create_resolv_conf", test_dns_create_resolv_conf);
+
 	return g_test_run ();
 }