Merge lp:~vicamo/upstart/xenial-escape-systemd-strings into lp:~ubuntu-core-dev/upstart/xenial

[You-Sheng Yang]

upstart-local-bridge: escape strings for systemd

Android property names and values may contain invalid characters for systemd and breaks the mechanism here.

[Łukasz Zemczak (sil2100)]

In overall the proposed change looks fine an seems to work reasonably well. The same could have been realized without the new hexchar() function, but having it makes things potentially slightly faster and simplified.
That being said, I do not know the upstart code-base that well, so maybe it would be nice if some other maintainer took a quick look as well.

One stylistic recommendation: in the inline comments, the if, else if and else parts could use { } even in the single-line conditional commands. It's much cleaner this way and follows the upstart code style better.

review: Approve (not an upstart maintainer)

[lp:~vicamo/upstart/xenial-escape-systemd-strings]

1633. By You-Sheng Yang on 2016-10-05

Address stylistic review comments.

[Dimitri John Ledkov (xnox)]

Yakkety: https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/2047

Xenial: https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/2048

=== modified file 'extra/upstart-local-bridge.c'
--- extra/upstart-local-bridge.c	2015-01-16 23:55:17 +0000
+++ extra/upstart-local-bridge.c	2016-10-05 09:27:10 +0000
@@ -912,6 +912,47 @@
 
 }
 
+static char
+hexchar (char c)
+{
+	static const char table[16] = "0123456789abcdef";
+
+	return table[c & 0xf];
+}
+
+static char*
+escape_string (const char *f)
+{
+	char *r = NULL;
+	char *t = NULL;
+
+	nih_assert (f);
+
+	r = t = NIH_MUST (nih_alloc (NULL, strlen(f) * 4 + 1));
+
+#define VALID_CHARS \
+		"0123456789" \
+		"abcdefghijklmnopqrstuvwxyz" \
+		"ABCDEFGHIJKLMNOPQRSTUVWXYX" \
+		":_."
+	for (; *f; f++) {
+		if (*f == '/') {
+			*(t++) = '-';
+		} else if (!strchr(VALID_CHARS, *f)) {
+			*(t++) = '\\';
+			*(t++) = 'x';
+			*(t++) = hexchar(*f >> 4);
+			*(t++) = hexchar(*f);
+		} else {
+			*(t++) = *f;
+		}
+	}
+
+	*t = '\0';
+
+	return r;
+}
+
 static void
 systemd_launch_instance (ClientConnection  *client,
 	    const char        *pair,
@@ -919,6 +960,8 @@
 {
 	nih_local char     *safe_pair = NULL;
 	nih_local char    **key_value = NULL;
+	nih_local char     *escaped_key = NULL;
+	nih_local char     *escaped_value = NULL;
 	nih_local char     *group_name = NULL;
 	nih_local char     *unit_name = NULL;
 	nih_local char     *job_name = NULL;
@@ -933,14 +976,18 @@
 	/* Get key val from the key=val pair */
 	key_value = NIH_MUST (nih_str_split (NULL, safe_pair, "=", TRUE));
 
+	/* Escape key/value for systemd */
+	escaped_key = NIH_MUST (escape_string (key_value[0]));
+	escaped_value = NIH_MUST (escape_string (key_value[1] ? key_value[1] : "(null)"));
+
 	/* Construct systemd event@key=*.target group name */
-	group_name = NIH_MUST (nih_sprintf (NULL, "%s@%s=*.target",
-					    event_name, key_value[0]));
+	group_name = NIH_MUST (nih_sprintf (NULL, "%s@%s\\x3d*.target",
+					    event_name, escaped_key));
 
 	/* Construct systemd event@key=value.target unit name */
 	unit_name = NIH_MUST (nih_sprintf (NULL, "%s@%s\\x3d%s.target",
-					   event_name, key_value[0],
-					   key_value[1]));
+					   event_name, escaped_key,
+					   escaped_value));
 
 	/* Stop group */
 	int pid = -1;