ifcfg-rh: comment out invalid lines during svWriteFile

This is especially important because we don't support
line continuation. Thus, with

    FOO='val
      bar=3'
    wrong line
      F2=b
      F3='b
    XXX=adf'
      XXX2=val2
    '
we now write

    FOO=
    #NM: FOO='val
    bar=
    #NM:   bar=3'
    #NM: wrong line
      F2=b
    F3=
    #NM:   F3='b
    XXX=
    #NM: XXX=adf'
      XXX2=val2
    #NM: '

Basically, the writer will comment out any line that is

  - not all-whitespace
  - not a '#' comment (possibly proceeded by whitespace)
  - not a valid variable assignment

This avoids that writer writes lines that are not understood by
ifcfg-rh plugin, but interferes with initscripts. E.g.

  NAME=old-name'
  rm -rf /
  '

becomes

  NAME=new-name
  #NM: rm -rf /
  #NM: '

Makefile.am

@@ -1926,7 +1926,13 @@ EXTRA_DIST += \
 	src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-trailing-spaces \
 	src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-dns-options \
 	src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wired-wake-on-lan \
-	src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wired-ipv6-only-1
+	src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wired-ipv6-only-1 \
+	src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-write-unknown-1 \
+	src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-write-unknown-1.expected \
+	src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-write-unknown-2 \
+	src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-write-unknown-2.expected \
+	src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-write-unknown-3 \
+	src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-write-unknown-3.expected
 
 # make target dependencies can't have colons in their names, which ends up
 # meaning that we can't add the alias files to EXTRA_DIST. They are instead

src/settings/plugins/ifcfg-rh/shvar.c

@@ -97,6 +97,25 @@ _shell_is_name (const char *key)
 	                       g_ascii_isalnum (ch) || ch == '_');
 }
 
+static const char *
+_shell_is_name_assignment (const char *key)
+{
+	/* whether @key is a valid identifier (name). */
+	if (!key)
+		return NULL;
+	if (   !g_ascii_isalpha (key[0])
+	    && key[0] != '_')
+		return NULL;
+	while (TRUE) {
+		const char ch = (++key)[0];
+
+		if (ch == '=')
+			return &key[1];
+		if (!g_ascii_isalnum (ch) && ch != '_')
+			return NULL;
+	}
+}
+
 /*****************************************************************************/
 
 /* like g_strescape(), except that it also escapes '\''' *sigh*.
@@ -542,6 +561,19 @@ svFileGetName (const shvarFile *s)
 	return s->fileName;
 }
 
+void
+svFileSetName (shvarFile *s, const char *fileName)
+{
+	g_free (s->fileName);
+	s->fileName = g_strdup (fileName);
+}
+
+void
+svFileSetModified (shvarFile *s)
+{
+	s->modified = TRUE;
+}
+
 /*****************************************************************************/
 
 /* Open the file <name>, returning a shvarFile on success and NULL on failure.
@@ -943,8 +975,33 @@ svWriteFile (shvarFile *s, int mode, GError **error)
 		}
 		f = fdopen (tmpfd, "w");
 		fseek (f, 0, SEEK_SET);
-		for (current = s->lineList; current; current = current->next)
-			fprintf (f, "%s\n", (const char *) current->data);
+		for (current = s->lineList; current; current = current->next) {
+			const char *line = current->data;
+			const char *str;
+			const char *value;
+
+			str = line;
+			while (g_ascii_isspace (str[0]))
+				str++;
+			if (NM_IN_SET (str[0], '\0', '#'))
+				goto write_regular;
+
+			value = _shell_is_name_assignment (str);
+			if (value) {
+				gs_free char *s_tmp = NULL;
+
+				/* we check that the assignment can be properly unescaped. */
+				if (svUnescape (value, &s_tmp))
+					goto write_regular;
+				nm_clear_g_free (&s_tmp);
+				s_tmp = g_strndup (str, value - str);
+				fprintf (f, "%s\n", s_tmp);
+			}
+			fprintf (f, "#NM: %s\n", line);
+			continue;
+write_regular:
+			fprintf (f, "%s\n", line);
+		}
 		fclose (f);
 	}
 

src/settings/plugins/ifcfg-rh/shvar.h

@@ -34,6 +34,9 @@
 typedef struct _shvarFile shvarFile;
 
 const char *svFileGetName (const shvarFile *s);
+void svFileSetName (shvarFile *s, const char *fileName);
+
+void svFileSetModified (shvarFile *s);
 
 /* Create the file <name>, return a shvarFile (never fails) */
 shvarFile *svCreateFile (const char *name);

src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-write-unknown-1

@@ -0,0 +1,8 @@
+FOO='val
+  bar=3'
+wrong line
+  F2=b
+  F3='b
+XXX=adf'
+  XXX2=val2
+'

src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-write-unknown-1.expected

@@ -0,0 +1,12 @@
+FOO=
+#NM: FOO='val
+bar=
+#NM:   bar=3'
+#NM: wrong line
+  F2=b
+F3=
+#NM:   F3='b
+XXX=
+#NM: XXX=adf'
+  XXX2=val2
+#NM: '

src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-write-unknown-2

@@ -0,0 +1,3 @@
+FOO='
+BAR=a
+'

src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-write-unknown-2.expected

@@ -0,0 +1,4 @@
+FOO=
+#NM: FOO='
+BAR=a
+#NM: '

src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-write-unknown-3

@@ -0,0 +1,3 @@
+FOO='
+BAR="
+'

src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-write-unknown-3.expected

@@ -0,0 +1,5 @@
+FOO=
+#NM: FOO='
+BAR=
+#NM: BAR="
+#NM: '

src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c

@@ -57,6 +57,8 @@
 
 #include "nm-test-utils-core.h"
 
+#define TEST_SCRATCH_DIR_TMP TEST_SCRATCH_DIR"/network-scripts/tmp"
+
 /*****************************************************************************/
 
 static NMConnection *
@@ -4048,7 +4050,7 @@ test_read_write_static_routes_legacy (void)
 	 * source tree and for 'make distcheck'.
 	 */
 	_writer_new_connection (connection,
-	                        TEST_SCRATCH_DIR "/network-scripts/tmp",
+	                        TEST_SCRATCH_DIR_TMP,
 	                        &testfile);
 
 	reread = _connection_from_file (testfile, NULL, TYPE_ETHERNET, NULL);
@@ -8993,7 +8995,36 @@ test_svUnescape (void)
 		}
 		do_svUnescape_assert (str_val->str, str_exp->str);
 	}
+}
 
+/*****************************************************************************/
+
+static void
+test_write_unknown (gconstpointer test_data)
+{
+	nmtst_auto_unlinkfile char *filename_tmp_1 = g_strdup (TEST_SCRATCH_DIR_TMP"/tmp-1");
+	const char *testfile = test_data;
+	gs_free char *testfile_expected = g_strconcat (testfile, ".expected", NULL);
+	shvarFile *sv;
+	gs_free_error GError *error = NULL;
+	gboolean success;
+	gs_free char *file_contents_out = NULL;
+	gs_free char *file_contents_exp = NULL;
+
+	sv = svOpenFile (testfile, &error);
+	nmtst_assert_success (sv, error);
+
+	svFileSetName (sv, filename_tmp_1);
+	svFileSetModified (sv);
+	success = svWriteFile (sv, 0644, &error);
+	nmtst_assert_success (success, error);
+
+	file_contents_out = nmtst_file_get_contents (filename_tmp_1);
+	file_contents_exp = nmtst_file_get_contents (testfile_expected);
+
+	g_assert_cmpstr (file_contents_out, ==, file_contents_exp);
+
+	svCloseFile (sv);
 }
 
 /*****************************************************************************/
@@ -9174,10 +9205,15 @@ int main (int argc, char **argv)
 {
 	nmtst_init_assert_logging (&argc, &argv, "INFO", "DEFAULT");
 
-	if (g_mkdir_with_parents (TEST_SCRATCH_DIR"/network-scripts/tmp", 0755) != 0)
-		g_error ("failure to create test directory \"%s\": %s", TEST_SCRATCH_DIR"/network-scripts/tmp", g_strerror (errno));
+	if (g_mkdir_with_parents (TEST_SCRATCH_DIR_TMP, 0755) != 0)
+		g_error ("failure to create test directory \"%s\": %s", TEST_SCRATCH_DIR_TMP, g_strerror (errno));
 
 	g_test_add_func (TPATH "svUnescape", test_svUnescape);
+
+	g_test_add_data_func (TPATH "write-unknown/1", TEST_IFCFG_DIR"/network-scripts/ifcfg-test-write-unknown-1", test_write_unknown);
+	g_test_add_data_func (TPATH "write-unknown/2", TEST_IFCFG_DIR"/network-scripts/ifcfg-test-write-unknown-2", test_write_unknown);
+	g_test_add_data_func (TPATH "write-unknown/3", TEST_IFCFG_DIR"/network-scripts/ifcfg-test-write-unknown-3", test_write_unknown);
+
 	g_test_add_func (TPATH "vlan-trailing-spaces", test_read_vlan_trailing_spaces);
 
 	g_test_add_func (TPATH "unmanaged", test_read_unmanaged);