ESI: Refuse malformed numbers

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

lib/include/kickcat/utils/xsd.h

@@ -0,0 +1,23 @@
+#ifndef KICKCAT_UTILS_XSD_H
+#define KICKCAT_UTILS_XSD_H
+
+#include <cstdint>
+#include <string_view>
+#include <vector>
+
+// 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<uint8_t> parseHexBinary(std::string_view text);
+}
+
+#endif

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<int64_t>(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<uint8_t> Parser::loadHexBinary(XMLElement* node)
     {
         return {};
     }
-    std::string field = raw;
-    // Real ESI files wrap long hex payloads (e.g. <ConfigData>) 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<uint8_t> 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<uint8_t>(byte));
+        std::string what = "ESI: <";
+        what += node->Value();
+        what += ">: ";
+        what += e.what();
+        throw std::invalid_argument(what);
     }
-
-    return data;
 }
 
 

lib/src/utils/xsd.cc

@@ -0,0 +1,148 @@
+#include <limits>
+#include <stdexcept>
+#include <string>
+
+#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<uint64_t>::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<uint64_t>(digit)) / static_cast<uint64_t>(base))
+            {
+                throw std::invalid_argument("numeric value " + quoted + " is out of range for 64 bits");
+            }
+            value = value * static_cast<uint64_t>(base) + static_cast<uint64_t>(digit);
+        }
+
+        if (not negative)
+        {
+            return static_cast<int64_t>(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<int64_t>(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<uint8_t> 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<uint8_t> 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<uint8_t>((high << 4) | low));
+        }
+        return data;
+    }
+}

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

unit/src/utils/xsd-t.cc

@@ -0,0 +1,42 @@
+#include <gtest/gtest.h>
+#include <stdexcept>
+
+#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<uint8_t>{0x01, 0x02, 0xff}));
+    EXPECT_EQ(xsd::parseHexBinary(" 01 02\n FF "), (std::vector<uint8_t>{0x01, 0x02, 0xff}));
+    EXPECT_TRUE(xsd::parseHexBinary("").empty());
+    EXPECT_THROW(xsd::parseHexBinary("012"), std::invalid_argument);
+    EXPECT_THROW(xsd::parseHexBinary("zz"),  std::invalid_argument);
+}