From 5839dd4abf3a8453d563cf907bb30b0f4a2708c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Klemens=20B=C3=B6swirth?= Date: Fri, 20 Nov 2020 17:05:54 +0100 Subject: [PATCH] Update key name decision Fixes 1:1 mapping by removing collision between keys like / and /% --- doc/keynames/README.md | 3 +++ doc/keynames/keynames.py | 7 ++++++ src/bindings/cpp/include/kdbvalue.hpp | 4 ++- src/libs/elektra/keyname.c | 14 +++++++++++ tests/abi/testabi_key.c | 36 ++++++++++++++++++--------- tests/ctest/test_keyname.c | 16 +++++++----- tests/data/data_noescape.c | 35 -------------------------- 7 files changed, 61 insertions(+), 54 deletions(-) delete mode 100644 tests/data/data_noescape.c diff --git a/doc/keynames/README.md b/doc/keynames/README.md index ea5cd31d7ee..500559d8e89 100644 --- a/doc/keynames/README.md +++ b/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`. diff --git a/doc/keynames/keynames.py b/doc/keynames/keynames.py index b78ecadd08c..0b4234232bf 100755 --- a/doc/keynames/keynames.py +++ b/doc/keynames/keynames.py @@ -176,6 +176,13 @@ def canonicalize(name: str, prefix: str = "", verbose: bool = False) -> str: f"Allowed values for : {NAMESPACES}" ) + # Check if Key Name starts correctly after Namespace + if fullname == "/%": + raise KeyNameException( + f"Key must NOT be ':/%' or just '/%' (collides with root key). " + + f"Allowed values for : {NAMESPACES}" + ) + # Check if for dangling escapes if sum(1 for _ in takewhile(lambda x: x == "\\", reversed(fullname))) % 2 != 0: raise KeyNameException( diff --git a/src/bindings/cpp/include/kdbvalue.hpp b/src/bindings/cpp/include/kdbvalue.hpp index b9c574de2f8..12031486840 100644 --- a/src/bindings/cpp/include/kdbvalue.hpp +++ b/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 ()); }; diff --git a/src/libs/elektra/keyname.c b/src/libs/elektra/keyname.c index 06de9e3c354..e1fbaf004b1 100644 --- a/src/libs/elektra/keyname.c +++ b/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; diff --git a/tests/abi/testabi_key.c b/tests/abi/testabi_key.c index 5716fddc04b..04c4f31c7dc 100644 --- a/tests/abi/testabi_key.c +++ b/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 + 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); diff --git a/tests/ctest/test_keyname.c b/tests/ctest/test_keyname.c index 2a1db4c487b..762cce9af34 100644 --- a/tests/ctest/test_keyname.c +++ b/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); diff --git a/tests/data/data_noescape.c b/tests/data/data_noescape.c deleted file mode 100644 index bc35aa63740..00000000000 --- a/tests/data/data_noescape.c +++ /dev/null @@ -1,35 +0,0 @@ -/** - * @file - * - * @brief - * - * @copyright BSD License (see LICENSE.md or https://www.libelektra.org) - */ - -// clang-format off - -TEST_NOESCAPE_PART("a", "a"); -TEST_NOESCAPE_PART("$", "$"); -TEST_NOESCAPE_PART("€", "€"); -TEST_NOESCAPE_PART("\x01", "\x01"); -TEST_NOESCAPE_PART("\xFF", "\xFF"); -TEST_NOESCAPE_PART("\xFF\xFF\xFF\xFF", "\xFF\xFF\xFF\xFF"); -TEST_NOESCAPE_PART("\xFF\xFF/\xFF\xFF", "\xFF\xFF"); -TEST_NOESCAPE_PART("test", "test"); -TEST_NOESCAPE_PART("test/name", "name"); -TEST_NOESCAPE_PART("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_ESCAPE_PART("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_NOESCAPE_PART("%", ""); -TEST_NOESCAPE_PART("\\\\%", "\\%"); - -// TEST_NOESCAPE_PART("a\\\\\\", "a\\\\\\"); -// TEST_NOESCAPE_PART("\\", "\\"); -// TEST_NOESCAPE_PART("\\\\", "\\\\"); -// TEST_NOESCAPE_PART("\\\\\\\\", "\\\\\\\\"); -// TEST_NOESCAPE_PART("\\\\\\\\\\\\\\\\", "\\\\\\\\\\\\\\\\"); -// -TEST_NOESCAPE_PART("a/test", "test"); -TEST_NOESCAPE_PART("a\\/test", "a/test");