From e703e9b7055cd0d7b89cf614e66405b4a740ad1f Mon Sep 17 00:00:00 2001
From: John <ebusd@ebusd.eu>
Date: Sat, 13 Jul 2024 10:09:32 +0200
Subject: [PATCH] assert names are valid identifier with little exceptions only
 (closes #1302)

---
 src/lib/ebus/data.cpp    | 50 ++++++++++++++++++++++++++++++++++------
 src/lib/ebus/data.h      | 16 +++++++++++++
 src/lib/ebus/message.cpp |  8 +++++++
 3 files changed, 67 insertions(+), 7 deletions(-)

diff --git a/src/lib/ebus/data.cpp b/src/lib/ebus/data.cpp
index 35a25805..50011c57 100644
--- a/src/lib/ebus/data.cpp
+++ b/src/lib/ebus/data.cpp
@@ -191,6 +191,34 @@ string AttributedItem::getAttribute(const string& name) const {
 }
 
 
+bool isValidIdentifierChar(char ch, bool first, bool allowFirstDigit) {
+  return ((ch >= '0' && ch <= '9') && (!first || allowFirstDigit))
+    || (ch >= 'a' && ch <= 'z')
+    || (ch >= 'A' && ch <= 'Z')
+    || ch == '_' || ch == '$'
+    // todo '.' is the only excuse for now and should be removed some day
+    || (ch == '.'  && !first);
+}
+
+bool DataField::checkIdentifier(const string& name, bool allowFirstDigit) {
+  for (size_t i = 0; i < name.size(); i++) {
+    char ch = name[i];
+    if (!isValidIdentifierChar(ch, i==0, allowFirstDigit)) {
+      return false;
+    }
+  }
+  return true;
+}
+
+void DataField::normalizeIdentifier(string& name, bool allowFirstDigit) {
+  for (size_t i = 0; i < name.size(); i++) {
+    char ch = name[i];
+    if (!isValidIdentifierChar(ch, i==0, allowFirstDigit)) {
+      name[i] = '_';
+    }
+  }
+}
+
 result_t DataField::create(bool isWriteMessage, bool isTemplate, bool isBroadcastOrMasterDestination,
     size_t maxFieldLength, const DataFieldTemplates* templates, vector< map<string, string> >* rows,
     string* errorDescription, const DataField** returnField) {
@@ -345,6 +373,9 @@ result_t DataField::create(bool isWriteMessage, bool isTemplate, bool isBroadcas
         if (!dataType) {
           result = RESULT_ERR_NOTFOUND;
           *errorDescription = "field type "+typeName+" in field "+formatInt(fieldIndex);
+        } else if (firstType && !name.empty() && !DataField::checkIdentifier(name)) {
+          *errorDescription = "field name "+name;
+          result = RESULT_ERR_INVALID_ARG;
         } else {
           SingleDataField* add = nullptr;
           result = SingleDataField::create(firstType ? name : "", row, dataType, partType, length, divisor,
@@ -370,14 +401,19 @@ result_t DataField::create(bool isWriteMessage, bool isTemplate, bool isBroadcas
         } else {
           fieldName = (firstType && lastType) ? name : "";
         }
-        if (lastType) {
-          result = templ->derive(fieldName, partType, divisor, values, &row, &fields);
+        if (!fieldName.empty() && !DataField::checkIdentifier(fieldName)) {
+          *errorDescription = "field name "+fieldName;
+          result = RESULT_ERR_INVALID_ARG;
         } else {
-          map<string, string> attrs = row;  // don't let DataField::derive() consume the row
-          result = templ->derive(fieldName, partType, divisor, values, &attrs, &fields);
-        }
-        if (result != RESULT_OK) {
-          *errorDescription = "derive field "+fieldName+" in field "+formatInt(fieldIndex);
+          if (lastType) {
+            result = templ->derive(fieldName, partType, divisor, values, &row, &fields);
+          } else {
+            map<string, string> attrs = row;  // don't let DataField::derive() consume the row
+            result = templ->derive(fieldName, partType, divisor, values, &attrs, &fields);
+          }
+          if (result != RESULT_OK) {
+            *errorDescription = "derive field "+fieldName+" in field "+formatInt(fieldIndex);
+          }
         }
       }
       if (firstType && !lastType) {
diff --git a/src/lib/ebus/data.h b/src/lib/ebus/data.h
index e922529c..f36fc058 100755
--- a/src/lib/ebus/data.h
+++ b/src/lib/ebus/data.h
@@ -215,6 +215,22 @@ class DataField : public AttributedItem {
    */
   virtual bool isList() const { return false; }
 
+  /**
+   * Check if the given name is a valid identifier.
+   * @param name the name to check (and optionally normalize).
+   * @param allowFirstDigit whether to additionally allow the name to start with a digit.
+   * @param normalize whether to replace invalid characters with an underscore.
+   * @return true if the name is valid (or was normalized), false if invalid.
+   */
+  static bool checkIdentifier(const string& name, bool allowFirstDigit = false);
+
+  /**
+   * Normalize the given name to be a valid identifier.
+   * @param name the name to check and normalize.
+   * @param allowFirstDigit whether to additionally allow the name to start with a digit.
+   */
+  static void normalizeIdentifier(string& name, bool allowFirstDigit = false);
+
   /**
    * Factory method for creating new instances.
    * @param isWriteMessage whether the field is part of a write message (default false).
diff --git a/src/lib/ebus/message.cpp b/src/lib/ebus/message.cpp
index a3cb3fe7..ab74a915 100644
--- a/src/lib/ebus/message.cpp
+++ b/src/lib/ebus/message.cpp
@@ -301,11 +301,19 @@ result_t Message::create(const string& filename, const DataFieldTemplates* templ
     *errorDescription = "circuit";
     return RESULT_ERR_MISSING_ARG;  // empty circuit
   }
+  if (!DataField::checkIdentifier(circuit, true)) {
+    *errorDescription = "circuit name "+circuit;
+    return RESULT_ERR_INVALID_ARG;  // invalid circuit name
+  }
   string name = getDefault(pluck("name", row), defaults, "name", true, true);  // name
   if (name.empty()) {
     *errorDescription = "name";
     return RESULT_ERR_MISSING_ARG;  // empty name
   }
+  if (!DataField::checkIdentifier(name)) {
+    *errorDescription = "name "+name;
+    return RESULT_ERR_INVALID_ARG;  // invalid message name
+  }
   string comment = getDefault(pluck("comment", row), defaults, "comment", true);  // [comment]
   if (!comment.empty()) {
     (*row)["comment"] = comment;