AVRO-4312: [c++] Validate field names and enum symbols when parsing

lang/c++/impl/Compiler.cc

@@ -132,6 +132,29 @@ string getStringField(const Entity &e, const Object &m,
     return it->second.stringValue();
 }
 
+// Validates that a record field name or enum symbol conforms to the Avro name
+// grammar: a non-empty string whose first character is [A-Za-z_] and whose
+// remaining characters are [A-Za-z0-9_] (the same rule enforced for named type
+// simple names by Name::check()). This prevents out-of-spec strings from being
+// emitted verbatim as identifiers by the C++ code generator. The character checks
+// are restricted to ASCII (rather than the locale-dependent std::isalnum), which
+// is consistent with the locale-independent checks used by Name::check().
+static void validateSimpleName(const string &name, const char *what) {
+    if (name.empty()) {
+        throw Exception("Empty {} name", what);
+    }
+    const auto isAsciiLetter = [](char c) { return (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z'); };
+    if (!isAsciiLetter(name[0]) && name[0] != '_') {
+        throw Exception("Invalid {} name: {}", what, name);
+    }
+    for (const char c : name) {
+        const bool isAsciiAlnum = isAsciiLetter(c) || (c >= '0' && c <= '9');
+        if (!isAsciiAlnum && c != '_') {
+            throw Exception("Invalid {} name: {}", what, name);
+        }
+    }
+}
+
 const Array &getArrayField(const Entity &e, const Object &m,
                            const string &fieldName);
 
@@ -309,6 +332,7 @@ static void getCustomAttributes(const Object &m, CustomAttributes &customAttribu
 static Field makeField(const Entity &e, SymbolTable &st, const string &ns) {
     const Object &m = e.objectValue();
     string n = getStringField(e, m, "name");
+    validateSimpleName(n, "field");
     vector<string> aliases;
     string aliasesName = "aliases";
     if (containsField(m, aliasesName)) {
@@ -427,7 +451,9 @@ static NodePtr makeEnumNode(const Entity &e,
         if (it.type() != json::EntityType::String) {
             throw Exception("Enum symbol not a string: {}", it.toString());
         }
-        symbols.add(it.stringValue());
+        const string &symbol = it.stringValue();
+        validateSimpleName(symbol, "enum symbol");
+        symbols.add(symbol);
     }
     NodePtr node = NodePtr(new NodeEnum(asSingleAttribute(name), symbols));
     if (containsField(m, "doc")) {

lang/c++/impl/Node.cc

@@ -101,19 +101,31 @@ bool Name::operator<(const Name &n) const {
     return (ns_ < n.ns_) || (!(n.ns_ < ns_) && (simpleName_ < n.simpleName_));
 }
 
+// Locale-independent ASCII alphanumeric test. Using std::isalnum here would make
+// the accepted name grammar depend on the current locale.
+static bool isAsciiAlnum(char c) {
+    return (c >= '0' && c <= '9') || (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z');
+}
+
+static bool isAsciiLetter(char c) {
+    return (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z');
+}
+
 static bool invalidChar1(char c) {
-    return !isalnum(c) && c != '_' && c != '.' && c != '$';
+    return !isAsciiAlnum(c) && c != '_' && c != '.' && c != '$';
 }
 
 static bool invalidChar2(char c) {
-    return !isalnum(c) && c != '_';
+    return !isAsciiAlnum(c) && c != '_';
 }
 
 void Name::check() const {
     if (!ns_.empty() && (ns_[0] == '.' || ns_[ns_.size() - 1] == '.' || std::find_if(ns_.begin(), ns_.end(), invalidChar1) != ns_.end())) {
         throw Exception("Invalid namespace: " + ns_);
     }
-    if (simpleName_.empty()
+    // A simple name must be non-empty, start with [A-Za-z_], and otherwise
+    // contain only [A-Za-z0-9_].
+    if (simpleName_.empty() || !(isAsciiLetter(simpleName_[0]) || simpleName_[0] == '_')
         || std::find_if(simpleName_.begin(), simpleName_.end(), invalidChar2) != simpleName_.end()) {
         throw Exception("Invalid name: " + simpleName_);
     }

lang/c++/test/SchemaTests.cc

@@ -230,7 +230,27 @@ const char *basicSchemaErrors[] = {
     // default double - null
     R"({ "name":"test", "type": "record", "fields": [ {"name": "double","type": "double","default" : null }]})",
     // default double - string
-    R"({ "name":"test", "type": "record", "fields": [ {"name": "double","type": "double","default" : "string" }]})"
+    R"({ "name":"test", "type": "record", "fields": [ {"name": "double","type": "double","default" : "string" }]})",
+
+    // Names outside the Avro name grammar
+    // Enum symbol containing a space
+    R"({"type": "enum", "name": "Status", "symbols" : ["Ok", "Not Ok"]})",
+    // Enum symbol attempting identifier injection
+    R"({"type": "enum", "name": "Status", "symbols" : ["Ok", "A, B_c = 5"]})",
+    // Empty enum symbol
+    R"({"type": "enum", "name": "Status", "symbols" : ["Ok", ""]})",
+    // Field name containing a space
+    R"({"type":"record","name":"R","fields":[{"name":"in valid","type":"long"}]})",
+    // Field name attempting identifier/code injection
+    R"({"type":"record","name":"R","fields":[{"name":"x; int y","type":"long"}]})",
+    // Empty field name
+    R"({"type":"record","name":"R","fields":[{"name":"","type":"long"}]})",
+    // Field name starting with a digit
+    R"({"type":"record","name":"R","fields":[{"name":"1abc","type":"long"}]})",
+    // Enum symbol starting with a digit
+    R"({"type": "enum", "name": "Status", "symbols" : ["Ok", "1abc"]})",
+    // Type name starting with a digit
+    R"({"type":"record","name":"1Bad","fields":[]})"
 
 };