diff --git a/lang/c++/impl/Compiler.cc b/lang/c++/impl/Compiler.cc index c0b39f9e743..66f16b6fb38 100644 --- a/lang/c++/impl/Compiler.cc +++ b/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 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")) { diff --git a/lang/c++/impl/Node.cc b/lang/c++/impl/Node.cc index 0a5fcfc1b0b..b6ea4177fab 100644 --- a/lang/c++/impl/Node.cc +++ b/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_); } diff --git a/lang/c++/test/SchemaTests.cc b/lang/c++/test/SchemaTests.cc index 2aa39d4146e..e4b859d78f1 100644 --- a/lang/c++/test/SchemaTests.cc +++ b/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":[]})" };