From 80a6fc177dc1bf4f6235a3400a96ccf0a2ba4923 Mon Sep 17 00:00:00 2001 From: Philippe Leduc Date: Thu, 1 Oct 2026 12:21:06 +0200 Subject: [PATCH] ESI: Refuse malformed numbers - Refuse ESI numbers that do not match the schema (e.g. '12abc', '#x-1'). - Share the parsing of XML schema values with the coming ENI parser. --- lib/CMakeLists.txt | 2 + lib/include/kickcat/utils/xsd.h | 23 +++++ lib/src/ESI/Parser.cc | 116 ++++++------------------- lib/src/utils/xsd.cc | 148 ++++++++++++++++++++++++++++++++ unit/CMakeLists.txt | 1 + unit/src/utils/xsd-t.cc | 42 +++++++++ 6 files changed, 244 insertions(+), 88 deletions(-) create mode 100644 lib/include/kickcat/utils/xsd.h create mode 100644 lib/src/utils/xsd.cc create mode 100644 unit/src/utils/xsd-t.cc diff --git a/lib/CMakeLists.txt b/lib/CMakeLists.txt index e751fbf8..90396bbf 100644 --- a/lib/CMakeLists.txt +++ b/lib/CMakeLists.txt @@ -12,6 +12,8 @@ set(KICKCAT_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/src/OS/Time.cc ${CMAKE_CURRENT_SOURCE_DIR}/src/OS/Timer.cc + ${CMAKE_CURRENT_SOURCE_DIR}/src/utils/xsd.cc + ${CMAKE_CURRENT_SOURCE_DIR}/src/CoE/OD.cc ${CMAKE_CURRENT_SOURCE_DIR}/src/CoE/protocol.cc ${CMAKE_CURRENT_SOURCE_DIR}/src/CoE/mailbox/request.cc diff --git a/lib/include/kickcat/utils/xsd.h b/lib/include/kickcat/utils/xsd.h new file mode 100644 index 00000000..e135f224 --- /dev/null +++ b/lib/include/kickcat/utils/xsd.h @@ -0,0 +1,23 @@ +#ifndef KICKCAT_UTILS_XSD_H +#define KICKCAT_UTILS_XSD_H + +#include +#include +#include + +// Values of the XML schema types shared by ESI (ETG.2000) and ENI (ETG.2100) files. +// Errors throw std::invalid_argument without context: callers add the element they were reading. +namespace kickcat::xsd +{ + /// HexDecValue: [+-]?[0-9]+ or #x (0x accepted) followed by hex digits, surrounding spaces ignored. + /// Unsigned values up to 2^64 - 1 keep their bit pattern. + int64_t parseHexDec(std::string_view text); + + /// xs:boolean: 1, 0, true, false. + bool parseBoolean(std::string_view text); + + /// xs:hexBinary, in wire order; embedded whitespace (wrapped lines) ignored. + std::vector parseHexBinary(std::string_view text); +} + +#endif diff --git a/lib/src/ESI/Parser.cc b/lib/src/ESI/Parser.cc index b27aa8b9..eca82619 100644 --- a/lib/src/ESI/Parser.cc +++ b/lib/src/ESI/Parser.cc @@ -8,6 +8,7 @@ #include "kickcat/CoE/protocol.h" #include "kickcat/debug.h" #include "kickcat/ESI/Parser.h" +#include "kickcat/utils/xsd.h" using namespace tinyxml2; @@ -120,67 +121,23 @@ namespace return elem->GetText(); } - int64_t parseHexDec(std::string text, std::string const& where = {}) + int64_t parseHexDec(std::string const& text, std::string const& where = {}) { - auto fail = [&](char const* msg) + try + { + return xsd::parseHexDec(text); + } + catch (std::invalid_argument const& e) { std::string what = "ESI: "; - what += msg; - what += " '"; - what += text; - what += "'"; + what += e.what(); if (not where.empty()) { what += " in "; what += where; } throw std::invalid_argument(what); - }; - - if (text.empty()) - { - fail("empty numeric value"); - } - - int base = 10; - if (text.rfind("#x", 0) == 0) - { - if (text.size() == 2) { fail("'#x' with no hex digits"); } - text[0] = '0'; - base = 16; - } - else if (text.rfind("0x", 0) == 0 or text.rfind("0X", 0) == 0) - { - if (text.size() == 2) { fail("'0x' with no hex digits"); } - base = 16; - } - - try - { - return std::stoll(text, nullptr, base); - } - catch (std::invalid_argument const&) - { - fail("invalid numeric value"); - } - catch (std::out_of_range const&) - { - // Unsigned values in [2^63, 2^64): keep the 64-bit pattern, consumers - // (DefaultValue copy, narrowChecked) work on raw bits. - if (text[0] != '-') - { - try - { - return static_cast(std::stoull(text, nullptr, base)); - } - catch (std::exception const&) - { - // fall through to the range error - } - } - fail("numeric value exceeds 64-bit range"); } - return 0; // unreachable; fail() always throws } // Narrow an int64_t to T with bounds checking. For signed T, also accepts @@ -262,15 +219,18 @@ namespace { return false; } - if (std::strcmp(raw, "true") == 0 or std::strcmp(raw, "1") == 0) { return true; } - if (std::strcmp(raw, "false") == 0 or std::strcmp(raw, "0") == 0) { return false; } - - std::string what = "ESI: attribute '"; - what += name; - what += "' is not a valid xs:boolean (got '"; - what += raw; - what += "')"; - throw std::invalid_argument(what); + try + { + return xsd::parseBoolean(raw); + } + catch (std::invalid_argument const& e) + { + std::string what = "ESI: attribute '"; + what += name; + what += "': "; + what += e.what(); + throw std::invalid_argument(what); + } } // xs:int attribute: signed decimal, no #x hex prefix. nullopt when truly @@ -1676,38 +1636,18 @@ std::vector Parser::loadHexBinary(XMLElement* node) { return {}; } - std::string field = raw; - // Real ESI files wrap long hex payloads (e.g. ) across lines, so - // strip embedded whitespace before pairing nibbles. - field.erase(std::remove_if(field.begin(), field.end(), - [](unsigned char c){ return std::isspace(c) != 0; }), field.end()); - if (field.size() % 2 != 0) + try { - throw std::invalid_argument("ESI: hex binary <" + std::string{node->Value()} - + "> has odd length (" + std::to_string(field.size()) + " chars)"); + return xsd::parseHexBinary(raw); } - std::vector data; - data.reserve(field.size() / 2); - - for (std::size_t i = 0; i < field.size(); i += 2) + catch (std::invalid_argument const& e) { - char buf[3] = {field[i], field[i + 1], '\0'}; - char* end = nullptr; - unsigned long byte = std::strtoul(buf, &end, 16); - if (end != buf + 2) - { - std::string what = "ESI: hex binary <"; - what += node->Value(); - what += "> contains non-hex pair '"; - what += buf; - what += "' at byte "; - what += std::to_string(i / 2); - throw std::invalid_argument(what); - } - data.push_back(static_cast(byte)); + std::string what = "ESI: <"; + what += node->Value(); + what += ">: "; + what += e.what(); + throw std::invalid_argument(what); } - - return data; } diff --git a/lib/src/utils/xsd.cc b/lib/src/utils/xsd.cc new file mode 100644 index 00000000..d2cc29af --- /dev/null +++ b/lib/src/utils/xsd.cc @@ -0,0 +1,148 @@ +#include +#include +#include + +#include "kickcat/utils/xsd.h" + +namespace kickcat::xsd +{ + namespace + { + bool isSpace(char c) + { + return (c == ' ') or (c == '\t') or (c == '\n') or (c == '\r'); + } + + // -1 when c is not a digit of base + int digitValue(char c, int base) + { + int value = -1; + if ((c >= '0') and (c <= '9')) + { + value = c - '0'; + } + else if ((c >= 'a') and (c <= 'f')) + { + value = c - 'a' + 10; + } + else if ((c >= 'A') and (c <= 'F')) + { + value = c - 'A' + 10; + } + if (value >= base) + { + return -1; + } + return value; + } + + std::string_view trim(std::string_view text) + { + while (not text.empty() and isSpace(text.front())) + { + text.remove_prefix(1); + } + while (not text.empty() and isSpace(text.back())) + { + text.remove_suffix(1); + } + return text; + } + } + + int64_t parseHexDec(std::string_view raw) + { + std::string_view text = trim(raw); + std::string const quoted = "'" + std::string{text} + "'"; + + std::string_view digits = text; + int base = 10; + bool negative = false; + if ((text.rfind("#x", 0) == 0) or (text.rfind("0x", 0) == 0) or (text.rfind("0X", 0) == 0)) + { + digits = text.substr(2); + base = 16; + } + else if (not text.empty() and ((text[0] == '+') or (text[0] == '-'))) + { + negative = (text[0] == '-'); + digits = text.substr(1); + } + if (digits.empty()) + { + throw std::invalid_argument("invalid numeric value " + quoted); + } + + constexpr uint64_t MAX = std::numeric_limits::max(); + uint64_t value = 0; + for (char c : digits) + { + int digit = digitValue(c, base); + if (digit < 0) + { + throw std::invalid_argument("invalid numeric value " + quoted); + } + if (value > (MAX - static_cast(digit)) / static_cast(base)) + { + throw std::invalid_argument("numeric value " + quoted + " is out of range for 64 bits"); + } + value = value * static_cast(base) + static_cast(digit); + } + + if (not negative) + { + return static_cast(value); + } + constexpr uint64_t MIN_MAGNITUDE = uint64_t{1} << 63; + if (value > MIN_MAGNITUDE) + { + throw std::invalid_argument("numeric value " + quoted + " is out of range for 64 bits"); + } + return static_cast(0 - value); + } + + bool parseBoolean(std::string_view raw) + { + std::string_view text = trim(raw); + if ((text == "1") or (text == "true")) + { + return true; + } + if ((text == "0") or (text == "false")) + { + return false; + } + throw std::invalid_argument("invalid xs:boolean '" + std::string{text} + "'"); + } + + std::vector parseHexBinary(std::string_view text) + { + std::string field; + field.reserve(text.size()); + for (char c : text) + { + if (not isSpace(c)) + { + field.push_back(c); + } + } + if (field.size() % 2 != 0) + { + throw std::invalid_argument("hexBinary has odd length (" + std::to_string(field.size()) + " chars)"); + } + + std::vector data; + data.reserve(field.size() / 2); + for (std::size_t i = 0; i < field.size(); i += 2) + { + int high = digitValue(field[i], 16); + int low = digitValue(field[i + 1], 16); + if ((high < 0) or (low < 0)) + { + throw std::invalid_argument("hexBinary contains non-hex pair '" + field.substr(i, 2) + "' at byte " + std::to_string(i / 2)); + } + data.push_back(static_cast((high << 4) | low)); + } + return data; + } +} diff --git a/unit/CMakeLists.txt b/unit/CMakeLists.txt index 45b8d109..df3a8d38 100644 --- a/unit/CMakeLists.txt +++ b/unit/CMakeLists.txt @@ -29,6 +29,7 @@ add_executable(kickcat_unit src/adler32_sum-t.cc src/ESMStatePreOP-t.cc src/ESMStateSafeOP-t.cc src/utils/math-t.cc + src/utils/xsd-t.cc src/Units-t.cc src/Timer-t.cc src/CoE/protocol-t.cc diff --git a/unit/src/utils/xsd-t.cc b/unit/src/utils/xsd-t.cc new file mode 100644 index 00000000..25141f98 --- /dev/null +++ b/unit/src/utils/xsd-t.cc @@ -0,0 +1,42 @@ +#include +#include + +#include "kickcat/utils/xsd.h" + +using namespace kickcat; + +TEST(XSD, hexdec) +{ + EXPECT_EQ(xsd::parseHexDec("42"), 42); + EXPECT_EQ(xsd::parseHexDec("+42"), 42); + EXPECT_EQ(xsd::parseHexDec("-1"), -1); + EXPECT_EQ(xsd::parseHexDec(" #x1A "), 0x1A); + EXPECT_EQ(xsd::parseHexDec("0x1a"), 0x1A); + EXPECT_EQ(xsd::parseHexDec("#xFFFFFFFFFFFFFFFF"), -1); // ULINT keeps its bit pattern + EXPECT_EQ(xsd::parseHexDec("18446744073709551615"), -1); + + for (char const* bad : {"", "-", "#x", "12abc", "1 2", "#x-1", "#x0x12", "#x 12", "0x0x12"}) + { + EXPECT_THROW(xsd::parseHexDec(bad), std::invalid_argument) << bad; + } + EXPECT_THROW(xsd::parseHexDec("#x10000000000000000"), std::invalid_argument); +} + +TEST(XSD, boolean) +{ + EXPECT_TRUE (xsd::parseBoolean("1")); + EXPECT_TRUE (xsd::parseBoolean(" true ")); + EXPECT_FALSE(xsd::parseBoolean("0")); + EXPECT_FALSE(xsd::parseBoolean("false")); + EXPECT_THROW(xsd::parseBoolean("yes"), std::invalid_argument); + EXPECT_THROW(xsd::parseBoolean("TRUE"), std::invalid_argument); +} + +TEST(XSD, hexbinary) +{ + EXPECT_EQ(xsd::parseHexBinary("0102ff"), (std::vector{0x01, 0x02, 0xff})); + EXPECT_EQ(xsd::parseHexBinary(" 01 02\n FF "), (std::vector{0x01, 0x02, 0xff})); + EXPECT_TRUE(xsd::parseHexBinary("").empty()); + EXPECT_THROW(xsd::parseHexBinary("012"), std::invalid_argument); + EXPECT_THROW(xsd::parseHexBinary("zz"), std::invalid_argument); +}