environment: disallow recursive env var assignments

...like FOO=$FOO:bar to avoid environment variables growing on
reconfigure.

docs/labwc-config.5.scd

@@ -50,9 +50,14 @@ that directory contains either the "environment" file or at least one
 
 Note: environment files are treated differently by Openbox, which will simply
 source the file as a valid shell script before running the window manager. Files
-are instead parsed directly by labwc, although any environment variables
-referenced as $VARIABLE or ${VARIABLE} will be substituted and the tilde (~)
-will be expanded as the user's home directory.
+are instead parsed directly by labwc so that environment variables can be
+re-loaded on --reconfigure.
+
+Any environment variables referenced as $VARIABLE or ${VARIABLE} will be
+substituted and the tilde (~) will be expanded as the user's home directory.
+
+Note: Recursive variable assignments (for example FOO=$FOO:bar) is disallowed to
+prevent environment variables from growing on each --reconfigure.
 
 The *autostart* file is executed as a shell script after labwc has read its
 configuration and set variables defined in the environment file. Additionally,

include/config/is-recursive.h

@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef LABWC_IS_RECURSIVE_H
+#define LABWC_IS_RECURSIVE_H
+#include <stdbool.h>
+
+/**
+ * is_recursive() - Check if env var assignment key=value is recursive
+ * @key: environment variable
+ * @value: the value to be assigned to the environment variable
+ *
+ * Note: Can be used to detect assignments like FOO=$FOO:bar which may not be
+ * good to call multiple times.
+ */
+bool is_recursive(const char *key, const char *value);
+
+#endif /* LABWC_IS_RECURSIVE_H */

src/config/is-recursive.c

@@ -0,0 +1,58 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#define _POSIX_C_SOURCE 200809L
+#include <ctype.h>
+#include <stdio.h>
+#include <string.h>
+#include "common/buf.h"
+#include "common/string-helpers.h"
+#include "config/is-recursive.h"
+
+static bool
+is_env_var_identifier_char(char p)
+{
+	return isalnum(p) || p == '_';
+}
+
+bool
+is_recursive(const char *key, const char *value)
+{
+	if (string_null_or_empty(key) || string_null_or_empty(value)) {
+		return false;
+	}
+
+	bool ret = false;
+	struct buf needle = BUF_INIT;
+
+	/* Given key=FOO lets start with a needle set to ${FOO} */
+	buf_add_fmt(&needle, "${%s}", key);
+	if (strstr(value, needle.data)) {
+		ret = true;
+		goto out;
+	}
+
+	/*
+	 * Let's try searching for $FOO. That's a bit harder because $FOO exists
+	 * in $FOO (obviously) but not in $FOOO
+	 */
+	buf_clear(&needle);
+	buf_add_fmt(&needle, "$%s", key);
+	const char *p = value;
+	for (;;) {
+		/* Go through all matches of $FOO */
+		p = strstr(p, needle.data);
+		if (!p) {
+			break;
+		}
+		if (is_env_var_identifier_char(p[needle.len])) {
+			/* We are in a long identifier like $FOOOOO */
+			p += needle.len;
+			continue;
+		}
+		ret = true;
+		break;
+	}
+
+out:
+	buf_reset(&needle);
+	return ret;
+}

src/config/meson.build

@@ -1,4 +1,5 @@
 labwc_sources += files(
+  'is-recursive.c',
   'rcxml.c',
   'keybind.c',
   'session.c',

src/config/session.c

@@ -18,6 +18,7 @@
 #include "common/parse-bool.h"
 #include "common/spawn.h"
 #include "common/string-helpers.h"
+#include "config/is-recursive.h"
 #include "config/session.h"
 #include "labwc.h"
 
@@ -52,9 +53,22 @@ process_line(char *line)
 
 	struct buf value = BUF_INIT;
 	buf_add(&value, string_strip(++p));
+
+	/*
+	 * We do not allow recursive assignments (e.g. FOO=$FOO:bar) because
+	 * they would just keep growing on --reconfigure
+	 */
+	if (is_recursive(key, value.data)) {
+		wlr_log(WLR_ERROR, "recursive assignment not allowed (%s=%s)",
+			key, value.data);
+		goto err;
+	}
+
 	buf_expand_shell_variables(&value);
 	buf_expand_tilde(&value);
 	setenv(key, value.data, 1);
+
+err:
 	buf_reset(&value);
 }
 

t/meson.build

@@ -11,6 +11,7 @@ test_lib = static_library(
 
 tests = [
   'buf-simple',
+  'recursion',
 ]
 
 foreach t : tests

t/recursion.c

@@ -0,0 +1,36 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#define _POSIX_C_SOURCE 200809L
+#include <setjmp.h>
+#include <stdarg.h>
+#include <stddef.h>
+#include <stdio.h>
+#include <string.h>
+#include <cmocka.h>
+#include "common/buf.h"
+#include "common/mem.h"
+#include "../src/config/is-recursive.c"
+
+static void
+test_recursion(void **state)
+{
+	(void)state;
+
+	assert_false(is_recursive("FOO", "bar"));
+	assert_false(is_recursive("FOO", "FOO"));
+	assert_false(is_recursive("FOO", "$FOOOOOO"));
+
+	assert_true(is_recursive("FOO", "$FOO:bar"));
+	assert_true(is_recursive("FOO", "O$FOO"));
+	assert_true(is_recursive("FOO", "$FOOO$FOO"));
+	assert_true(is_recursive("FOO", "$FOOO$FOOOO$BAR$FOO"));
+	assert_true(is_recursive("FOO", "$FOOO$FOOOO$BAR$FOO:bar"));
+}
+
+int main(int argc, char **argv)
+{
+	const struct CMUnitTest tests[] = {
+		cmocka_unit_test(test_recursion),
+	};
+
+	return cmocka_run_group_tests(tests, NULL, NULL);
+}