Fix support for 0-length hex-encoded octet strings in chip-tool. (#24097)
* Fix support for 0-length hex-encoded octet strings in chip-tool.
Without this fix, these commands all fail to parse:
chip-tool unittesting write struct-attr '{"a": 1, "b": true, "c": 0, "d":"", "e": "abc", "f": 0, "g": 0, "h": 0}' 17 1
chip-tool unittesting write struct-attr '{"a": 1, "b": true, "c": 0, "d":"hex:", "e": "abc", "f": 0, "g": 0, "h": 0}' 17 1
chip-tool unittesting write octet-string "hex:" 17 1
chip-tool unittesting write-by-id 0 "hex:" 17 1
and with this fix they parse and send correct payloads.
Fixes https://github.com/project-chip/connectedhomeip/issues/24054
* Apply suggestion from review comment.
Co-authored-by: Jonathan Mégevand <77852424+jmeg-sfy@users.noreply.github.com>
* Address review comment.
Co-authored-by: Jonathan Mégevand <77852424+jmeg-sfy@users.noreply.github.com>diff --git a/examples/chip-tool/BUILD.gn b/examples/chip-tool/BUILD.gn
index d870c1d..86e003a 100644
--- a/examples/chip-tool/BUILD.gn
+++ b/examples/chip-tool/BUILD.gn
@@ -58,6 +58,7 @@
"commands/common/Commands.cpp",
"commands/common/Commands.h",
"commands/common/CredentialIssuerCommands.h",
+ "commands/common/HexConversion.h",
"commands/discover/DiscoverCommand.cpp",
"commands/discover/DiscoverCommissionablesCommand.cpp",
"commands/discover/DiscoverCommissionersCommand.cpp",
diff --git a/examples/chip-tool/commands/clusters/ComplexArgument.h b/examples/chip-tool/commands/clusters/ComplexArgument.h
index 2af80e5..e624aeb 100644
--- a/examples/chip-tool/commands/clusters/ComplexArgument.h
+++ b/examples/chip-tool/commands/clusters/ComplexArgument.h
@@ -36,6 +36,7 @@
#include <app-common/zap-generated/cluster-objects.h>
#include <app/data-model/List.h>
#include <app/data-model/Nullable.h>
+#include <commands/common/HexConversion.h>
#include <json/json.h>
#include <lib/core/Optional.h>
#include <lib/support/BytesToHex.h>
@@ -210,18 +211,22 @@
size = str.size();
}
- if (size % 2 != 0)
- {
- ChipLogError(chipTool, "Error while encoding %s as a hex string: Odd number of characters.", label);
- return CHIP_ERROR_INVALID_STRING_LENGTH;
- }
+ CHIP_ERROR err = HexToBytes(
+ chip::CharSpan(str.c_str(), size),
+ [&buffer](size_t allocSize) {
+ buffer = static_cast<uint8_t *>(chip::Platform::MemoryCalloc(allocSize, sizeof(uint8_t)));
+ return buffer;
+ },
+ &size);
- buffer = static_cast<uint8_t *>(chip::Platform::MemoryCalloc(size / 2, sizeof(uint8_t)));
- size = chip::Encoding::HexToBytes(str.c_str(), size, buffer, size / 2);
- if (size == 0)
+ if (err != CHIP_NO_ERROR)
{
- ChipLogError(chipTool, "Error while encoding %s as a hex string.", label);
- return CHIP_ERROR_INTERNAL;
+ if (buffer != nullptr)
+ {
+ chip::Platform::MemoryFree(buffer);
+ }
+
+ return err;
}
}
diff --git a/examples/chip-tool/commands/clusters/CustomArgument.h b/examples/chip-tool/commands/clusters/CustomArgument.h
index 63fe7d7..ab6ce40 100644
--- a/examples/chip-tool/commands/clusters/CustomArgument.h
+++ b/examples/chip-tool/commands/clusters/CustomArgument.h
@@ -19,6 +19,7 @@
#pragma once
#include <app-common/zap-generated/cluster-objects.h>
+#include <commands/common/HexConversion.h>
#include <lib/support/BytesToHex.h>
#include <lib/support/CHIPMemString.h>
#include <lib/support/SafeInt.h>
@@ -140,14 +141,18 @@
static CHIP_ERROR PutOctetString(chip::TLV::TLVWriter * writer, chip::TLV::Tag tag, Json::Value & value)
{
- size_t size = strlen(value.asCString());
- VerifyOrReturnError(size % 2 == 0, CHIP_ERROR_INVALID_STRING_LENGTH);
-
+ const char * hexData = value.asCString() + kPayloadHexPrefixLen;
+ size_t hexDataLen = strlen(hexData);
chip::Platform::ScopedMemoryBuffer<uint8_t> buffer;
- VerifyOrReturnError(buffer.Calloc(size / 2), CHIP_ERROR_NO_MEMORY);
- size_t octetCount = chip::Encoding::HexToBytes(value.asCString() + kPayloadHexPrefixLen, size - kPayloadHexPrefixLen,
- buffer.Get(), (size - kPayloadHexPrefixLen) / 2);
- VerifyOrReturnError(octetCount != 0, CHIP_ERROR_NO_MEMORY);
+
+ size_t octetCount;
+ ReturnErrorOnFailure(HexToBytes(
+ chip::CharSpan(hexData, hexDataLen),
+ [&buffer](size_t allocSize) {
+ buffer.Calloc(allocSize);
+ return buffer.Get();
+ },
+ &octetCount));
return chip::app::DataModel::Encode(*writer, tag, chip::ByteSpan(buffer.Get(), octetCount));
}
diff --git a/examples/chip-tool/commands/common/Command.cpp b/examples/chip-tool/commands/common/Command.cpp
index c4161b6..cd39293 100644
--- a/examples/chip-tool/commands/common/Command.cpp
+++ b/examples/chip-tool/commands/common/Command.cpp
@@ -18,6 +18,7 @@
#include "Command.h"
#include "CustomStringPrefix.h"
+#include "HexConversion.h"
#include "platform/PlatformManager.h"
#include <functional>
@@ -377,14 +378,16 @@
// representation is always longer than the octet string it encodes,
// so we have enough space in argValue for the decoded version.
chip::Platform::ScopedMemoryBuffer<uint8_t> buffer;
- if (!buffer.Calloc(argLen)) // Bigger than needed, but it's fine.
- {
- return false;
- }
- size_t octetCount =
- chip::Encoding::HexToBytes(argValue + kHexStringPrefixLen, argLen - kHexStringPrefixLen, buffer.Get(), argLen);
- if (octetCount == 0)
+ size_t octetCount;
+ CHIP_ERROR err = HexToBytes(
+ chip::CharSpan(argValue + kHexStringPrefixLen, argLen - kHexStringPrefixLen),
+ [&buffer](size_t allocSize) {
+ buffer.Calloc(allocSize);
+ return buffer.Get();
+ },
+ &octetCount);
+ if (err != CHIP_NO_ERROR)
{
return false;
}
diff --git a/examples/chip-tool/commands/common/HexConversion.h b/examples/chip-tool/commands/common/HexConversion.h
new file mode 100644
index 0000000..b315681
--- /dev/null
+++ b/examples/chip-tool/commands/common/HexConversion.h
@@ -0,0 +1,65 @@
+/*
+ * Copyright (c) 2022 Project CHIP Authors
+ * All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include <lib/core/CHIPError.h>
+#include <lib/support/BytesToHex.h>
+#include <lib/support/Span.h>
+#include <lib/support/logging/CHIPLogging.h>
+
+/**
+ * Utility for converting a hex string to bytes, with the right error checking
+ * and allocation size computation.
+ *
+ * Takes a functor to allocate the buffer to use for the hex bytes. The functor
+ * is expected to return uint8_t *. The caller is responsible for cleaning up
+ * this buffer as needed.
+ *
+ * On success, *octetCount is filled with the number of octets placed in the
+ * buffer. On failure, the value of *octetCount is undefined.
+ */
+template <typename F>
+CHIP_ERROR HexToBytes(chip::CharSpan hex, F bufferAllocator, size_t * octetCount)
+{
+ *octetCount = 0;
+
+ if (hex.size() % 2 != 0)
+ {
+ ChipLogError(chipTool, "Error while encoding '%.*s' as an octet string: Odd number of characters.",
+ static_cast<int>(hex.size()), hex.data());
+ return CHIP_ERROR_INVALID_STRING_LENGTH;
+ }
+
+ const size_t bufferSize = hex.size() / 2;
+ uint8_t * buffer = bufferAllocator(bufferSize);
+ if (buffer == nullptr && bufferSize != 0)
+ {
+ ChipLogError(chipTool, "Failed to allocate buffer of size: %llu", static_cast<unsigned long long>(bufferSize));
+ return CHIP_ERROR_NO_MEMORY;
+ }
+
+ size_t byteCount = chip::Encoding::HexToBytes(hex.data(), hex.size(), buffer, bufferSize);
+ if (byteCount == 0 && hex.size() != 0)
+ {
+ ChipLogError(chipTool, "Error while encoding '%.*s' as an octet string.", static_cast<int>(hex.size()), hex.data());
+ return CHIP_ERROR_INTERNAL;
+ }
+
+ *octetCount = byteCount;
+ return CHIP_NO_ERROR;
+}
diff --git a/examples/darwin-framework-tool/BUILD.gn b/examples/darwin-framework-tool/BUILD.gn
index a677fc6..3e59a3c 100644
--- a/examples/darwin-framework-tool/BUILD.gn
+++ b/examples/darwin-framework-tool/BUILD.gn
@@ -124,6 +124,7 @@
"${chip_root}/examples/chip-tool/commands/common/Command.h",
"${chip_root}/examples/chip-tool/commands/common/Commands.cpp",
"${chip_root}/examples/chip-tool/commands/common/Commands.h",
+ "${chip_root}/examples/chip-tool/commands/common/HexConversion.h",
"${chip_root}/zzz_generated/chip-tool/zap-generated/cluster/ComplexArgumentParser.cpp",
"commands/clusters/ClusterCommandBridge.h",
"commands/clusters/ModelCommandBridge.mm",
diff --git a/examples/tv-casting-app/tv-casting-common/BUILD.gn b/examples/tv-casting-app/tv-casting-common/BUILD.gn
index 37572da..4d797a9 100644
--- a/examples/tv-casting-app/tv-casting-common/BUILD.gn
+++ b/examples/tv-casting-app/tv-casting-common/BUILD.gn
@@ -43,6 +43,7 @@
"${chip_root}/examples/chip-tool/commands/common/Commands.cpp",
"${chip_root}/examples/chip-tool/commands/common/Commands.h",
"${chip_root}/examples/chip-tool/commands/common/CredentialIssuerCommands.h",
+ "${chip_root}/examples/chip-tool/commands/common/HexConversion.h",
"${chip_root}/src/controller/ExamplePersistentStorage.cpp",
"${chip_root}/src/controller/ExamplePersistentStorage.h",
"${chip_root}/zzz_generated/chip-tool/zap-generated/cluster/ComplexArgumentParser.cpp",
