From da6b5a2df1e6b7ca722ce46eb23f249da959df52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sun, 19 Jul 2026 17:28:27 +0200 Subject: [PATCH 1/6] AVRO-4312: [c++] Validate field names and enum symbols when parsing Record field names and enum symbols were not validated against the Avro name grammar when compiling a schema, unlike named-type simple names which are already checked by Name::check(). As a result an out-of-spec string could be emitted verbatim as a C++ identifier by the code generator. Validate field names and enum symbols during schema compilation, rejecting anything that is not a non-empty sequence of [A-Za-z0-9_]. Add negative parser tests covering spaces and identifier-injection attempts in field names and enum symbols. --- lang/c++/impl/Compiler.cc | 18 ++++++++++++++++++ lang/c++/test/SchemaTests.cc | 12 +++++++++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/lang/c++/impl/Compiler.cc b/lang/c++/impl/Compiler.cc index c0b39f9e743..6c49ad2e0e9 100644 --- a/lang/c++/impl/Compiler.cc +++ b/lang/c++/impl/Compiler.cc @@ -16,6 +16,7 @@ * limitations under the License. */ +#include #include #include #include @@ -132,6 +133,21 @@ 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 sequence of [A-Za-z0-9_], as already 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. +static void validateSimpleName(const string &name, const char *what) { + if (name.empty()) { + throw Exception("Empty {} name", what); + } + for (const char c : name) { + if (!std::isalnum(static_cast(c)) && c != '_') { + throw Exception("Invalid {} name: {}", what, name); + } + } +} + const Array &getArrayField(const Entity &e, const Object &m, const string &fieldName); @@ -309,6 +325,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,6 +444,7 @@ static NodePtr makeEnumNode(const Entity &e, if (it.type() != json::EntityType::String) { throw Exception("Enum symbol not a string: {}", it.toString()); } + validateSimpleName(it.stringValue(), "enum symbol"); symbols.add(it.stringValue()); } NodePtr node = NodePtr(new NodeEnum(asSingleAttribute(name), symbols)); diff --git a/lang/c++/test/SchemaTests.cc b/lang/c++/test/SchemaTests.cc index 2aa39d4146e..fce848b560a 100644 --- a/lang/c++/test/SchemaTests.cc +++ b/lang/c++/test/SchemaTests.cc @@ -230,7 +230,17 @@ 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"]})", + // 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"}]})" }; From 5cb9b7c625c1dd39d0b6f3a06ec68518b7877a17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Mon, 20 Jul 2026 09:36:54 +0200 Subject: [PATCH 2/6] AVRO-4312: [c++] Use locale-independent ASCII check for names Address review feedback: validateSimpleName used std::isalnum, whose result depends on the current locale and could accept characters outside the intended ASCII [A-Za-z0-9_] grammar on non-"C" locales. Check the ASCII letter/digit ranges explicitly instead. --- lang/c++/impl/Compiler.cc | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lang/c++/impl/Compiler.cc b/lang/c++/impl/Compiler.cc index 6c49ad2e0e9..f5915d32eb4 100644 --- a/lang/c++/impl/Compiler.cc +++ b/lang/c++/impl/Compiler.cc @@ -16,7 +16,6 @@ * limitations under the License. */ -#include #include #include #include @@ -136,13 +135,16 @@ string getStringField(const Entity &e, const Object &m, // Validates that a record field name or enum symbol conforms to the Avro name // grammar (a non-empty sequence of [A-Za-z0-9_], as already 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. +// being emitted verbatim as identifiers by the C++ code generator. The character +// checks are restricted to ASCII on purpose (rather than std::isalnum, which is +// locale-dependent) so the accepted grammar does not vary with the locale. static void validateSimpleName(const string &name, const char *what) { if (name.empty()) { throw Exception("Empty {} name", what); } for (const char c : name) { - if (!std::isalnum(static_cast(c)) && c != '_') { + const bool isAsciiAlnum = (c >= '0' && c <= '9') || (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z'); + if (!isAsciiAlnum && c != '_') { throw Exception("Invalid {} name: {}", what, name); } } From 60398de6c69af09e95f7157456fbda0d9beb95bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Mon, 20 Jul 2026 09:45:10 +0200 Subject: [PATCH 3/6] AVRO-4312: [c++] Add empty field name and enum symbol test cases Address review feedback: cover the non-empty constraint enforced by validateSimpleName() with explicit empty field name and empty enum symbol schemas. --- lang/c++/test/SchemaTests.cc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lang/c++/test/SchemaTests.cc b/lang/c++/test/SchemaTests.cc index fce848b560a..8c1a5ca9e1f 100644 --- a/lang/c++/test/SchemaTests.cc +++ b/lang/c++/test/SchemaTests.cc @@ -237,10 +237,14 @@ const char *basicSchemaErrors[] = { 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"}]})" + R"({"type":"record","name":"R","fields":[{"name":"x; int y","type":"long"}]})", + // Empty field name + R"({"type":"record","name":"R","fields":[{"name":"","type":"long"}]})" }; From c09291e1acf738feca4cffaeaa5229aa848aca4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Mon, 20 Jul 2026 09:50:32 +0200 Subject: [PATCH 4/6] AVRO-4312: [c++] Make Name::check name validation locale-independent Address review feedback: Name::check() (used for named-type simple names and namespaces) relied on std::isalnum, which is locale-dependent, while the new field/symbol validation uses an ASCII check. Introduce a shared-style locale-independent ASCII predicate for the name-character tests in Name::check() so all schema name elements are validated with the same locale-invariant [A-Za-z0-9_] grammar. --- lang/c++/impl/Compiler.cc | 10 +++++----- lang/c++/impl/Node.cc | 10 ++++++++-- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/lang/c++/impl/Compiler.cc b/lang/c++/impl/Compiler.cc index f5915d32eb4..04a2373ec7e 100644 --- a/lang/c++/impl/Compiler.cc +++ b/lang/c++/impl/Compiler.cc @@ -133,11 +133,11 @@ string getStringField(const Entity &e, const Object &m, } // Validates that a record field name or enum symbol conforms to the Avro name -// grammar (a non-empty sequence of [A-Za-z0-9_], as already 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 on purpose (rather than std::isalnum, which is -// locale-dependent) so the accepted grammar does not vary with the locale. +// grammar (a non-empty sequence of [A-Za-z0-9_], as also 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); diff --git a/lang/c++/impl/Node.cc b/lang/c++/impl/Node.cc index 0a5fcfc1b0b..8fbb81b1903 100644 --- a/lang/c++/impl/Node.cc +++ b/lang/c++/impl/Node.cc @@ -101,12 +101,18 @@ 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 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 { From 9446305959e57a657ae69251c5567b93ddc6088e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Mon, 20 Jul 2026 09:55:10 +0200 Subject: [PATCH 5/6] AVRO-4312: [c++] Avoid calling stringValue() twice per enum symbol Address review feedback: store the enum symbol string once instead of calling Entity::stringValue() (which performs a type check and conversion) both for validation and when adding the symbol. --- lang/c++/impl/Compiler.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lang/c++/impl/Compiler.cc b/lang/c++/impl/Compiler.cc index 04a2373ec7e..012ddecbd9c 100644 --- a/lang/c++/impl/Compiler.cc +++ b/lang/c++/impl/Compiler.cc @@ -446,8 +446,9 @@ static NodePtr makeEnumNode(const Entity &e, if (it.type() != json::EntityType::String) { throw Exception("Enum symbol not a string: {}", it.toString()); } - validateSimpleName(it.stringValue(), "enum symbol"); - 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")) { From 587fd7dfc1115f61e4a12335a9a244fc64ee3f6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Mon, 20 Jul 2026 10:06:17 +0200 Subject: [PATCH 6/6] AVRO-4312: [c++] Require names to start with a letter or underscore Address review feedback: names starting with a digit (e.g. "1abc") were accepted but would be emitted as invalid C++ identifiers by avrogencpp. Enforce the Avro rule that a simple name's first character must be [A-Za-z_] in both validateSimpleName (field names and enum symbols) and Name::check (named-type simple names), keeping the two consistent. Add negative tests for leading-digit field, enum symbol and type names. --- lang/c++/impl/Compiler.cc | 9 +++++++-- lang/c++/impl/Node.cc | 8 +++++++- lang/c++/test/SchemaTests.cc | 8 +++++++- 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/lang/c++/impl/Compiler.cc b/lang/c++/impl/Compiler.cc index 012ddecbd9c..66f16b6fb38 100644 --- a/lang/c++/impl/Compiler.cc +++ b/lang/c++/impl/Compiler.cc @@ -133,7 +133,8 @@ string getStringField(const Entity &e, const Object &m, } // Validates that a record field name or enum symbol conforms to the Avro name -// grammar (a non-empty sequence of [A-Za-z0-9_], as also enforced for named type +// 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 @@ -142,8 +143,12 @@ 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 = (c >= '0' && c <= '9') || (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z'); + const bool isAsciiAlnum = isAsciiLetter(c) || (c >= '0' && c <= '9'); if (!isAsciiAlnum && c != '_') { throw Exception("Invalid {} name: {}", what, name); } diff --git a/lang/c++/impl/Node.cc b/lang/c++/impl/Node.cc index 8fbb81b1903..b6ea4177fab 100644 --- a/lang/c++/impl/Node.cc +++ b/lang/c++/impl/Node.cc @@ -107,6 +107,10 @@ 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 !isAsciiAlnum(c) && c != '_' && c != '.' && c != '$'; } @@ -119,7 +123,9 @@ 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 8c1a5ca9e1f..e4b859d78f1 100644 --- a/lang/c++/test/SchemaTests.cc +++ b/lang/c++/test/SchemaTests.cc @@ -244,7 +244,13 @@ const char *basicSchemaErrors[] = { // 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"}]})" + 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":[]})" };