Update key name decision

Fixes 1:1 mapping by removing collision between keys like / and /%

doc/keynames/README.md

@@ -397,6 +397,9 @@ An escaped key name is considered an invalid key name, if any of the following a
   (This mainly applies to root keys.)
 - It contains a namespace separator `:`, but the substring before the first `:` is not one of: `meta`, `spec`, `proc`, `dir`, `user`, `system` and `default`.
 - It contains an illegal escape sequence (see below).
+- It is the string `/%` or consists of a namespace followed by the namespace separator `:` followed by `/%`.
+  In other words, the first escaped part is translated into an empty unescaped part.
+  The unescaped names for these keys would collide with the root keys `/`, `user:/`, etc.
 
 > _Note:_ The C-API does not allow you to construct a `Key` with an invalid key name; for example `keyNew` (and `keyVNew`) will return `NULL`.
 

doc/keynames/keynames.py

@@ -176,6 +176,13 @@ def canonicalize(name: str, prefix: str = "", verbose: bool = False) -> str:
             f"Allowed values for <NAMESPACE>: {NAMESPACES}"
         )
 
+    # Check if Key Name starts correctly after Namespace
+    if fullname == "/%":
+        raise KeyNameException(
+            f"Key must NOT be '<NAMESPACE>:/%' or just '/%' (collides with root key). " +
+            f"Allowed values for <NAMESPACE>: {NAMESPACES}"
+        )
+
     # Check if for dangling escapes
     if sum(1 for _ in takewhile(lambda x: x == "\\", reversed(fullname))) % 2 != 0:
         raise KeyNameException(

src/bindings/cpp/include/kdbvalue.hpp

@@ -448,7 +448,9 @@ class Value : public ValueObserver, public ValueSubject, public Wrapped
 		assert (m_spec.getName ()[0] == '/' && "spec keys are not yet supported");
 		m_context.attachByName (m_spec.getName (), *this);
 		Command::Func fun = [this] () -> Command::Pair {
-			this->unsafeUpdateKeyUsingContext (m_context.evaluate (m_spec.getName ()));
+			auto evaluatedName = m_context.evaluate (m_spec.getName ());
+			evaluatedName = evaluatedName == "/%" ? "/" : evaluatedName;
+			this->unsafeUpdateKeyUsingContext (evaluatedName);
 			this->unsafeSyncCache (); // set m_cache
 			return std::make_pair ("", m_key.getName ());
 		};

src/libs/elektra/keyname.c

@@ -513,6 +513,20 @@ bool elektraKeyNameValidate (const char * name, bool isComplete)
 			ELEKTRA_LOG_DEBUG ("Illegal name start; expected (namespace +) slash: %s", name);
 			return 0;
 		}
+
+		if (*(name + 1) == '%' && *(name + 2) == '\0')
+		{
+			ELEKTRA_LOG_DEBUG ("Illegal escaped part; first part cannot be empty (collides with root key): %s", name);
+			return 0;
+		}
+	}
+	else
+	{
+		if (*name == '%' && *(name + 1) == '\0')
+		{
+			ELEKTRA_LOG_DEBUG ("Illegal escaped part; first part cannot be empty (collides with root key): %s", name);
+			return 0;
+		}
 	}
 
 	const char * cur = name;

tests/abi/testabi_key.c

@@ -2366,6 +2366,15 @@ static void test_keyEscape (void)
 	keyDel (k);
 }
 
+static void test_keyAdd_test (Key * k, const char * escaped, const char * unescaped)
+{
+	char buffer[500];
+	succeed_if_fmt (keyAddName (k, escaped) != -1, "keyAddName returned an error for '%s'", escaped);
+	succeed_if_same_string (keyBaseName (k), unescaped);
+	succeed_if (keyGetBaseName (k, buffer, 499) != -1, "keyGetBaseName returned an error");
+	succeed_if_same_string (buffer, unescaped);
+}
+
 static void test_keyAdd (void)
 {
 	printf ("test keyAdd\n");
@@ -2391,22 +2400,25 @@ static void test_keyAdd (void)
 	succeed_if (keyAddName (k, "invalid\\") < 0, "added invalid name");
 	succeed_if (keyAddName (k, "valid") == sizeof ("user:/valid"), "added valid name");
 
-#undef TEST_NOESCAPE_PART
-#define TEST_NOESCAPE_PART(A, S)                                                                                                           \
-	do                                                                                                                                 \
-	{                                                                                                                                  \
-		succeed_if (keyAddName (k, A) != -1, "keyAddName returned an error");                                                      \
-		succeed_if_same_string (keyBaseName (k), S);                                                                               \
-		succeed_if (keyGetBaseName (k, buffer, 499) != -1, "keyGetBaseName returned an error");                                    \
-		succeed_if_same_string (buffer, S);                                                                                        \
-	} while (0)
-	char buffer[500];
-
 	for (int i = 0; i < NUMBER_OF_NAMESPACES; ++i)
 	{
 		keySetName (k, namespaces[i]);
 
-#include <data_noescape.c>
+		test_keyAdd_test (k, "a", "a");
+		test_keyAdd_test (k, "$", "$");
+		test_keyAdd_test (k, "€", "€");
+		test_keyAdd_test (k, "\x01", "\x01");
+		test_keyAdd_test (k, "\xFF", "\xFF");
+		test_keyAdd_test (k, "\xFF\xFF\xFF\xFF", "\xFF\xFF\xFF\xFF");
+		test_keyAdd_test (k, "\xFF\xFF/\xFF\xFF", "\xFF\xFF");
+		test_keyAdd_test (k, "test", "test");
+		test_keyAdd_test (k, "test/name", "name");
+		test_keyAdd_test (k, "a/b/c/d/e/f/g/h/j/k/l/m/n/o/p/q/r/s/t/u/v/w/x/y/z", "z");
+		test_keyAdd_test (k, "a\\/b\\/c\\/d\\/e\\/f\\/g\\/h\\/j\\/k\\/l\\/m\\/n\\/o\\/p\\/q\\/r\\/s\\/t\\/u\\/v\\/w\\/x\\/y\\/z",
+				  "a/b/c/d/e/f/g/h/j/k/l/m/n/o/p/q/r/s/t/u/v/w/x/y/z");
+		test_keyAdd_test (k, "\\\\%", "\\%");
+		test_keyAdd_test (k, "a/test", "test");
+		test_keyAdd_test (k, "a\\/test", "a/test");
 	}
 
 	keyDel (k);

tests/ctest/test_keyname.c

@@ -43,7 +43,6 @@ static void test_validate (void)
 	TEST_VALIDATE_OK ("/ab", NULL);
 	TEST_VALIDATE_OK ("/abc", NULL);
 
-	TEST_VALIDATE_OK ("/%", NULL);
 	TEST_VALIDATE_OK ("/\\%", NULL);
 	TEST_VALIDATE_OK ("/\\\\%", NULL);
 	TEST_VALIDATE_OK ("/\\\\\\\\%", NULL);
@@ -238,7 +237,6 @@ static void test_validate (void)
 
 	TEST_VALIDATE_OK ("", "/abc");
 	TEST_VALIDATE_OK ("/", "/abc");
-	TEST_VALIDATE_OK ("%", "/abc");
 
 	TEST_VALIDATE_OK ("/abc/def/ghi", "/abc");
 	TEST_VALIDATE_OK ("/abc/def/ghi/", "/abc");
@@ -343,6 +341,11 @@ static void test_validate (void)
 	TEST_VALIDATE_OK ("..///../../..////../../../..//user", "/much/more/level/1/2/3");
 	TEST_VALIDATE_OK ("../../....///../../..////../../../..//user", "/much/more/level/1/2/3");
 
+	TEST_VALIDATE_OK ("/%a", NULL);
+	TEST_VALIDATE_OK ("user:/%a", NULL);
+	TEST_VALIDATE_OK ("%a", "/");
+	TEST_VALIDATE_OK ("%a", "user:/");
+
 	succeed_if (!elektraKeyNameValidate (NULL, true), "(NULL) SHOULD NOT BE a valid complete key name");
 
 	TEST_VALIDATE_ERROR ("", NULL);
@@ -398,6 +401,11 @@ static void test_validate (void)
 
 	TEST_VALIDATE_ERROR ("/\\#0/\\#1", NULL);
 	TEST_VALIDATE_ERROR ("/\\#0/..", "/");
+
+	TEST_VALIDATE_ERROR ("/%", NULL);
+	TEST_VALIDATE_ERROR ("user:/%", NULL);
+	TEST_VALIDATE_ERROR ("%", "/");
+	TEST_VALIDATE_ERROR ("%", "user:/");
 }
 
 #undef TEST_VALIDATE_OK
@@ -545,11 +553,9 @@ static void test_canonicalize (void)
 
 	TEST_CANONICALIZE_OK ("", "/", "/", 3, 3);
 	TEST_CANONICALIZE_OK ("/", "/", "/", 3, 3);
-	TEST_CANONICALIZE_OK ("%", "/", "/%", 3, 3); // FIXME: 1:1 mapping -> make illegal
 
 	TEST_CANONICALIZE_OK ("", "user:/", "user:/", 3, 3);
 	TEST_CANONICALIZE_OK ("/", "user:/", "user:/", 3, 3);
-	TEST_CANONICALIZE_OK ("%", "user:/", "user:/%", 3, 3); // FIXME: 1:1 mapping -> make illegal
 
 	TEST_CANONICALIZE_OK ("/abc/def/ghi", "/abc", "/abc/abc/def/ghi", 6, 18);
 	TEST_CANONICALIZE_OK ("/abc/def/ghi/", "/abc", "/abc/abc/def/ghi", 6, 18);
@@ -574,12 +580,10 @@ static void test_canonicalize (void)
 	TEST_CANONICALIZE_OK ("abc/..", "user:/", "user:/", 3, 3);
 	TEST_CANONICALIZE_OK ("user:/abc/..", NULL, "user:/", 0, 3);
 
-	TEST_CANONICALIZE_OK ("/%", NULL, "/%", 0, 3); // FIXME: 1:1 mapping -> make illegal
 	TEST_CANONICALIZE_OK ("/\\%", NULL, "/\\%", 0, 4);
 	TEST_CANONICALIZE_OK ("/\\\\%", NULL, "/\\\\%", 0, 5);
 	TEST_CANONICALIZE_OK ("/\\\\\\\\%", NULL, "/\\\\\\\\%", 0, 6);
 
-	TEST_CANONICALIZE_OK ("user:/%", NULL, "user:/%", 0, 3); // FIXME: 1:1 mapping -> make illegal
 	TEST_CANONICALIZE_OK ("user:/\\%", NULL, "user:/\\%", 0, 4);
 	TEST_CANONICALIZE_OK ("user:/\\\\%", NULL, "user:/\\\\%", 0, 5);
 	TEST_CANONICALIZE_OK ("user:/\\\\\\\\%", NULL, "user:/\\\\\\\\%", 0, 6);