Fix chip-tool PersistentStorageDelegate failing on some keys (#20239)

* Fix chip-tool PersistentStorageDelegate failing on some keys

- chip-tool's PersistentStorageDelegate failed the audit:

```
[1656721506.201383][688277:688277] CHIP:ATM: ../../../examples/chip-tool/third_party/connectedhomeip/src/lib/support/PersistentStorageAudit.cpp:248: assertion failed: "err == CHIP_NO_ERROR"
[1656721506.201409][688277:688277] CHIP:ATM: ../../../examples/chip-tool/third_party/connectedhomeip/src/lib/support/PersistentStorageAudit.cpp:249: assertion failed: "size == strlen(kBase64SymbolValues)"
[1656721506.201413][688277:688277] CHIP:ATM: ../../../examples/chip-tool/third_party/connectedhomeip/src/lib/support/PersistentStorageAudit.cpp:250: assertion failed: "0 == memcmp(&buf[0], kBase64SymbolValues, strlen(kBase64SymbolValues))"
[1656721506.201437][688277:688277] CHIP:ATM: ../../../examples/chip-tool/third_party/connectedhomeip/src/lib/support/PersistentStorageAudit.cpp:255: assertion failed: "err == CHIP_NO_ERROR"
```

- The audit also crashed on SyncSetKeyValue with nullptr argument

- To fail the audit, had to run the audit on a version that forcibly loaded what
  was stored again.
- Root cause was base64 padding has `=` which confuses INI parser into
  giving out keys that are too small

Issue #20188

This PR:
- Implements `SyncDoesKeyExist` natively
- Fixes handling of `nullptr` arguments and zero-size keys in the storage impl
- Uses `\x00` C-style hex escaping for any characters that could fool the INI parser,
  which retains human readability of keys in the INI files, but fixes the bugs
  found by the audit
- Removes unnecessary newliens in storage audit logging

Testing done:
- Unit tests still pass
- Cert tests still pass
- Storage audit passed

```
[1656722163.492413][696066:696066] CHIP:ATM: ==== PersistentStorageDelegate API audit: SUCCESS ====
```

* Restyled by clang-format

* Remove a stale temporary added during testing

Co-authored-by: Restyled.io <commits@restyled.io>

examples/chip-tool/config/PersistentStorage.cpp

@@ -50,6 +50,29 @@ std::string GetFilename(const char * name)
 
 namespace {
 
+std::string EscapeKey(const std::string & key)
+{
+ std::string escapedKey;
+ escapedKey.reserve(key.size());
+
+ for (char c : key)
+ {
+ // Replace spaces, non-printable chars and `=` with hex-escaped (C-style) characters.
+ if ((c <= 0x20) || (c == '=') || (c >= 0x7F))
+ {
+ char escaped[5] = { 0 };
+ snprintf(escaped, sizeof(escaped), "\\x%02x", (static_cast<unsigned>(c) & 0xff));
+ escapedKey += escaped;
+ }
+ else
+ {
+ escapedKey += c;
+ }
+ }
+
+ return escapedKey;
+}
+
 std::string StringToBase64(const std::string & value)
 {
  std::unique_ptr<char[]> buffer(new char[BASE64_ENCODED_LEN(value.length())]);
@@ -106,10 +129,11 @@ CHIP_ERROR PersistentStorage::SyncGetKeyValue(const char * key, void * value, ui
  ReturnErrorCodeIf(((value == nullptr) && (size != 0)), CHIP_ERROR_INVALID_ARGUMENT);
 
  auto section = mConfig.sections[kDefaultSectionName];
- auto it = section.find(key);
- ReturnErrorCodeIf(it == section.end(), CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND);
 
- ReturnErrorCodeIf(!inipp::extract(section[key], iniValue), CHIP_ERROR_INVALID_ARGUMENT);
+ ReturnErrorCodeIf(!SyncDoesKeyExist(key), CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND);
+
+ std::string escapedKey = EscapeKey(key);
+ ReturnErrorCodeIf(!inipp::extract(section[escapedKey], iniValue), CHIP_ERROR_INVALID_ARGUMENT);
 
  iniValue = Base64ToString(iniValue);
 
@@ -126,8 +150,19 @@ CHIP_ERROR PersistentStorage::SyncGetKeyValue(const char * key, void * value, ui
 
 CHIP_ERROR PersistentStorage::SyncSetKeyValue(const char * key, const void * value, uint16_t size)
 {
+ ReturnErrorCodeIf((value == nullptr) && (size != 0), CHIP_ERROR_INVALID_ARGUMENT);
+
  auto section = mConfig.sections[kDefaultSectionName];
- section[key] = StringToBase64(std::string(static_cast<const char *>(value), size));
+
+ std::string escapedKey = EscapeKey(key);
+ if (value == nullptr)
+ {
+ section[escapedKey] = "";
+ }
+ else
+ {
+ section[escapedKey] = StringToBase64(std::string(static_cast<const char *>(value), size));
+ }
 
  mConfig.sections[kDefaultSectionName] = section;
  return CommitConfig(mName);
@@ -136,15 +171,24 @@ CHIP_ERROR PersistentStorage::SyncSetKeyValue(const char * key, const void * val
 CHIP_ERROR PersistentStorage::SyncDeleteKeyValue(const char * key)
 {
  auto section = mConfig.sections[kDefaultSectionName];
- auto it = section.find(key);
- ReturnErrorCodeIf(it == section.end(), CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND);
 
- section.erase(key);
+ ReturnErrorCodeIf(!SyncDoesKeyExist(key), CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND);
+
+ std::string escapedKey = EscapeKey(key);
+ section.erase(escapedKey);
 
  mConfig.sections[kDefaultSectionName] = section;
  return CommitConfig(mName);
 }
 
+bool PersistentStorage::SyncDoesKeyExist(const char * key)
+{
+ std::string escapedKey = EscapeKey(key);
+ auto section = mConfig.sections[kDefaultSectionName];
+ auto it = section.find(escapedKey);
+ return (it != section.end());
+}
+
 CHIP_ERROR PersistentStorage::SyncClearAll()
 {
  ChipLogProgress(chipTool, "Clearing %s storage", kDefaultSectionName);

examples/chip-tool/config/PersistentStorage.h

@@ -32,6 +32,7 @@ class PersistentStorage : public chip::PersistentStorageDelegate
  CHIP_ERROR SyncGetKeyValue(const char * key, void * buffer, uint16_t & size) override;
  CHIP_ERROR SyncSetKeyValue(const char * key, const void * value, uint16_t size) override;
  CHIP_ERROR SyncDeleteKeyValue(const char * key) override;
+ bool SyncDoesKeyExist(const char * key) override;
 
  uint16_t GetListenPort();
  chip::Logging::LogCategory GetLoggingLevel();

src/lib/support/PersistentStorageAudit.cpp

@@ -34,7 +34,7 @@
  \
  if (!(inCondition)) \
  { \
- ChipLogError(Automation, "%s:%u: assertion failed: \"%s\"\n", __FILE__, __LINE__, #inCondition); \
+ ChipLogError(Automation, "%s:%u: assertion failed: \"%s\"", __FILE__, __LINE__, #inCondition);  \
  (inSuite)->failedAssertions += 1; \
  (inSuite)->flagError = true; \
  } \