all: make simple action's "sdata" string

For kernel, the sdata is a NUL terminated string. Very much like
file names or interface names or basically everything, kernel
is agnostic to any encoding. Of course, that is just lazy and leaves
user space in an impossible situation. User space rightly wants to
treat these fields as strings, show them in a UI, and read them
from command line options (like, the terminal should display
characters according to a certain encoding, not just show �).
But once a text is converted to a bunch of bytes, it only makes
sense if you know the text encoding that is used. But as kernel does
not restrict the encoding in any way (except requiring NUL termination,
and in case of file names special treatment of '.' and '/' -- ASCII
encoded), user space cannot reliably convert the blob back into a string.

Still, overall we don't want to treat such strings as blobs. In most
cases, users are sensible enough to just set them to ASCII or at most
UTF-8 encoding. So, treating them as UTF-8 strings is a good choice.

Change the attribute type of "sdata" from bytestring to string for
that, but make a special exception: this string is not taken literally,
instead it is a backslash encoded UTF-8 string.

That means, in most of the cases, the user can just treat it as
a regular string. Except that he must take special care of backslash.
But it still gives the user the possibility to encode every binary
value in this string using backslash escaping.

The escaped value can be created with nm_utils_str_utf8safe_escape()
or g_strescape() and converted back via nm_utils_str_utf8safe_unescape()
or g_strcompress().

Also, the attributes are not verified. Before accessing them in
tc_commit(), check that the attribute has the expected type.

4 files changed, 17 insertions, 10 deletions

diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c
index 74f8920740..51165a687b 100644
--- a/libnm-core/nm-utils.c
+++ b/libnm-core/nm-utils.c
@@ -2317,7 +2317,7 @@ nm_utils_tc_qdisc_from_str (const char *str, GError **error)
 /*****************************************************************************/
 
 static const NMVariantAttributeSpec * const tc_action_simple_attribute_spec[] = {
-	TC_ATTR_SPEC_PTR ("sdata",   G_VARIANT_TYPE_BYTESTRING,  FALSE, FALSE, 0   ),
+	TC_ATTR_SPEC_PTR ("sdata",   G_VARIANT_TYPE_STRING,      FALSE, FALSE, 0   ),
 	NULL,
 };
 
diff --git a/libnm-core/tests/test-setting.c b/libnm-core/tests/test-setting.c
index b935760f92..fbfaf9363b 100644
--- a/libnm-core/tests/test-setting.c
+++ b/libnm-core/tests/test-setting.c
@@ -1337,7 +1337,7 @@ test_tc_config_action (void)
 	nm_tc_action_unref (action1);
 	action1 = nm_tc_action_new ("simple", &error);
 	nmtst_assert_success (action1, error);
-	nm_tc_action_set_attribute (action1, "sdata", g_variant_new_bytestring ("Hello"));
+	nm_tc_action_set_attribute (action1, "sdata", g_variant_new_string ("Hello"));
 
 	g_assert (!nm_tc_action_equal (action1, action2));
 
@@ -1365,7 +1365,7 @@ test_tc_config_action (void)
 	nmtst_assert_success (action1, error);
 
 	g_assert_cmpstr (nm_tc_action_get_kind (action1), ==, "simple");
-	g_assert_cmpstr (g_variant_get_bytestring (nm_tc_action_get_attribute (action1, "sdata")), ==, "Hello");
+	g_assert_cmpstr (g_variant_get_string (nm_tc_action_get_attribute (action1, "sdata"), NULL), ==, "Hello");
 
 	nm_tc_action_unref (action1);
 	nm_tc_action_unref (action2);
@@ -1393,7 +1393,7 @@ test_tc_config_tfilter (void)
 
 	action1 = nm_tc_action_new ("simple", &error);
 	nmtst_assert_success (action1, error);
-	nm_tc_action_set_attribute (action1, "sdata", g_variant_new_bytestring ("Hello"));
+	nm_tc_action_set_attribute (action1, "sdata", g_variant_new_string ("Hello"));
 	nm_tc_tfilter_set_action (tfilter1, action1);
 	nm_tc_action_unref (action1);
 
@@ -1506,7 +1506,7 @@ test_tc_config_dbus (void)
 	nmtst_assert_success (tfilter2, error);
 	action = nm_tc_action_new ("simple", &error);
 	nmtst_assert_success (action, error);
-	nm_tc_action_set_attribute (action, "sdata", g_variant_new_bytestring ("Hello"));
+	nm_tc_action_set_attribute (action, "sdata", g_variant_new_string ("Hello"));
 	nm_tc_tfilter_set_action (tfilter2, action);
 	nm_tc_action_unref (action);
 	nm_setting_tc_config_add_tfilter (NM_SETTING_TC_CONFIG (s_tc), tfilter2);
diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index 7caf439347..1cc136634f 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -5381,10 +5381,18 @@ tc_commit (NMDevice *self)
 
 			action = nm_tc_tfilter_get_action (s_tfilter);
 			if (action) {
+				GVariant *attr;
+
 				tfilter->action.kind = nm_tc_action_get_kind (action);
-				if (strcmp (tfilter->action.kind, "simple") == 0) {
+				if (   nm_streq (tfilter->action.kind, "simple")
+				    && (attr = nm_tc_action_get_attribute (action, "sdata"))
+				    && g_variant_is_of_type (attr, G_VARIANT_TYPE_STRING)) {
+					gs_free char *sdata = NULL;
+
+					/* we don't check the length of sdata. Just truncate if it's too long. */
 					g_strlcpy (tfilter->action.simple.sdata,
-					           g_variant_get_bytestring (nm_tc_action_get_attribute (action, "sdata")),
+					           nm_utils_str_utf8safe_unescape (g_variant_get_string (attr, NULL),
+					                                           &sdata),
 					           sizeof (tfilter->action.simple.sdata));
 				}
 			}
diff --git a/src/settings/plugins/keyfile/tests/test-keyfile.c b/src/settings/plugins/keyfile/tests/test-keyfile.c
index f27efddd62..1161fe2b10 100644
--- a/src/settings/plugins/keyfile/tests/test-keyfile.c
+++ b/src/settings/plugins/keyfile/tests/test-keyfile.c
@@ -2679,8 +2679,7 @@ test_read_tc_config (void)
 	action2 = nm_tc_tfilter_get_action (tfilter2);
 	g_assert (action2);
 	g_assert (g_strcmp0 (nm_tc_action_get_kind (action2), "simple") == 0);
-	g_assert (g_strcmp0 (g_variant_get_bytestring (nm_tc_action_get_attribute (action2, "sdata")),
-	                     "Hello") == 0);
+	g_assert_cmpstr (g_variant_get_string (nm_tc_action_get_attribute (action2, "sdata"), NULL), ==, "Hello");
 }
 
 static void
@@ -2726,7 +2725,7 @@ test_write_tc_config (void)
 	nmtst_assert_success (tfilter2, error);
 	action = nm_tc_action_new ("simple", &error);
 	nmtst_assert_success (action, error);
-	nm_tc_action_set_attribute (action, "sdata", g_variant_new_bytestring ("Hello"));
+	nm_tc_action_set_attribute (action, "sdata", g_variant_new_string ("Hello"));
 	nm_tc_tfilter_set_action (tfilter2, action);
 	nm_tc_action_unref (action);
 	nm_setting_tc_config_add_tfilter (NM_SETTING_TC_CONFIG (s_tc), tfilter2);