Skip to content

Commit

Permalink
toml: fix silent failure on meta-key insertion
Browse files Browse the repository at this point in the history
  • Loading branch information
Bujuhu committed Nov 28, 2022
1 parent d63805f commit ac0ff88
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 6 deletions.
86 changes: 86 additions & 0 deletions doc/decisions/0_drafts/readonly_keynames.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
# Readonly Keynames

## Problem

It might be a good idea to prevent changes to the keyname after creation.
Currently, the name of a key is automatically made read-only, when it is currently part of or at some point in the past was part of at least one `KeySet`.
There are discussions about making key names writable again, when the key is removed from all `KeySet`s.

There may also be situations where changing the name of a key after its creation is required.
In some situations, it may be wise to reuse a key instead of deleting it and creating a new one.

## Constraints

1. `keyAddName` et al. still have to work up to a certain point
2.
3.

## Assumptions

1.
2.
3.

## Considered Alternatives

### Separate API for keynames

Use the proposed `ElektraBuffer` struct to create a separate API for keynames independent of the `Key` API.

### Read-only keynames

The key name should be permanently read-only after creation.

TBD: How would we dynamically create keynames with this approach?

### Re-entrant lock for the key name

Since we assume a single threaded context, this can be implemented as a simple counter.

```c
struct _Key {
// [...] other stuff
uint16_t nameLock; // if zero, name is writable, otherwise name is readonly
};

void keyLockName (Key * k) {
k->nameLock++;
}

void keyUnlockName (Key * k) {
if (k->nameLock == 0) return;
k->nameLock--;
}

void ksAppendKey (KeySet * ks, Key * k) {
keyLockName (k);
// [...]
}

void ksRemove (KeySet * ks, elektraCursor cursor) {
keyUnlockName (ks->array[cursor]);
// [...]
}
```
### Alternative C
## Decision
## Rationale
## Implications
-
-
-
## Related Decisions
- []()
- []()
- []()
## Notes
- [Issue 2202](https://issues.libelektra.org/2202) talks about how unexpected it is that keys will be readonly once in a keyset, but they don't get unlocked when removing them from a keyset.
8 changes: 7 additions & 1 deletion doc/news/_preparation_next_release.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,12 @@ The following text lists news about the [plugins](https://www.libelektra.org/plu
- Fix redirect logic to not cause loops _(@stefnotch)_
- <<TODO>>

### toml

- Fix bug, where meta-keys that cannot be inserted don't report an error _(@Bujuhu)_
- <<TODO>>
- <<TODO>>

### uname

- Add error handling if uname call fails _(Richard Stöckl @Eiskasten)_
Expand Down Expand Up @@ -355,7 +361,7 @@ This section keeps you up-to-date with the multi-language support provided by El
- <<TODO>>
- Many small fixes to adapt to documentation guidelines and new decision process. _(Markus Raab)_
- <<TODO>>
- <<TODO>>
- Add decision for [read-only keynames](../decisions/0_drafts/readonly_keynames.md) _(Maximilian Irlinger @atmaxinger)_
- <<TODO>>
- <<TODO>>
- <<TODO>>
Expand Down
25 changes: 20 additions & 5 deletions src/plugins/toml/testmod_toml.c
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ static const char * prefix = NULL;
if (lastKey != NULL) keySetMeta (lastKey, "binary", NULL); \
}

static bool writeFile (const char * filename, KeySet * ksWrite, int pluginStatus);
static void testRoundtrip (const char * filePath);
static void testRead (void);
static void testReadRoot (void);
Expand Down Expand Up @@ -176,6 +177,7 @@ static void testWriteReadComments (void);
static void testWriteReadCommentsArray (void);
static void testWriteReadOrderTableNonTable (void);
static void testWriteReadNull (void);
static void testWriteReadBogusMetaMustError(void);
// static void testWriteReadBase64(void);
static Key * addKey (KeySet * ks, const char * name, const char * value, size_t size, const char * orig, const char * type,
const char * array, const char * tomltype, int order);
Expand Down Expand Up @@ -341,6 +343,7 @@ static void testWriteRead (const char * _prefix)
testWriteReadInlineTableInArray ();
testWriteReadArrayInlineTableAlternating ();
testWriteReadOrderTableNonTable ();
testWriteReadBogusMetaMustError();
prefix = NULL;
}

Expand Down Expand Up @@ -457,6 +460,18 @@ static void testWriteReadNull (void)
TEST_WR_FOOT;
}

static void testWriteReadBogusMetaMustError (void)
{
TEST_WR_HEAD; \
WRITE_KEY ("asdf");
SET_META("asdf", "asdf");
writeFile("test_write_read.toml", writeKs, ELEKTRA_PLUGIN_STATUS_ERROR);
keyDel (lastKey);
ksDel (expectedKs); \
ksDel (writeKs); \
printf ("End Test: %s\n\n", __func__); \
}

/*static void testWriteReadBase64 (void)
{
TEST_WR_HEAD;
Expand Down Expand Up @@ -1423,7 +1438,7 @@ static KeySet * readFile (const char * filename)
return ks;
}

static bool writeFile (const char * filename, KeySet * ksWrite)
static bool writeFile (const char * filename, KeySet * ksWrite, int pluginStatus)
{
bool success = true;
ELEKTRA_LOG_DEBUG ("Writing '%s'\n", filename);
Expand All @@ -1433,8 +1448,8 @@ static bool writeFile (const char * filename, KeySet * ksWrite)
PLUGIN_OPEN ("toml");

int setStatus = plugin->kdbSet (plugin, ksWrite, parentKey);
succeed_if (setStatus == ELEKTRA_PLUGIN_STATUS_SUCCESS, "Could not write keys");
if (setStatus != ELEKTRA_PLUGIN_STATUS_SUCCESS)
succeed_if (setStatus == pluginStatus, "Writing keys did not return with expected Status");
if (setStatus != pluginStatus)
{
output_error (parentKey);
success = false;
Expand All @@ -1448,7 +1463,7 @@ static void testWriteReadCompare (KeySet * ksWrite, KeySet * expected)
{
const char * filename = "test_write_read.toml";

if (writeFile (filename, ksWrite))
if (writeFile (filename, ksWrite, ELEKTRA_PLUGIN_STATUS_SUCCESS))
{
{
KeySet * ksRead = readFile (filename);
Expand All @@ -1469,7 +1484,7 @@ static bool roundtripFile (const char * filenameIn, const char * filenameOut)
{
return false;
}
bool success = writeFile (filenameOut, ksRead);
bool success = writeFile (filenameOut, ksRead, ELEKTRA_PLUGIN_STATUS_SUCCESS);
ksDel (ksRead);
return success;
}
Expand Down
9 changes: 9 additions & 0 deletions src/plugins/toml/write.c
Original file line number Diff line number Diff line change
Expand Up @@ -858,6 +858,15 @@ static int collectComments (CommentList ** comments, Key * key, Writer * writer)
subDepth++;
pos += elektraStrLen (pos);
}
//Accept valid metakeys, throw error for everything else
} else if(elektraStrCmp(pos, "order") != 0 &&
elektraStrCmp(pos, "type") != 0 &&
elektraStrCmp(pos, "tomltype") != 0 &&
elektraStrCmp(pos, "origvalue") != 0 &&
elektraStrCmp(pos, "binary") != 0 &&
elektraStrCmp(pos, "array") != 0) {
ELEKTRA_SET_RESOURCE_ERRORF(key, "The Metakey %s is not supported by TOML", keyString(meta));
return -1;
}
}
*comments = commentRoot;
Expand Down

0 comments on commit ac0ff88

Please sign in to comment.