You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@avro.apache.org by "iemejia (via GitHub)" <gi...@apache.org> on 2026/07/20 07:20:24 UTC
[PR] AVRO-4312: [csharp] Validate names against the Avro name grammar [avro]
iemejia opened a new pull request, #3890:
URL: https://github.com/apache/avro/pull/3890
## What is the purpose of the change
Record/fixed/enum names, record field names and protocol message names were
accepted verbatim when parsing, unlike enum symbols which were already
validated. Because the code generator splices some of these values directly
into generated C# source (for example protocol message names into a `case`
label and field names into `Get`/`Put` switch bodies), an out-of-spec name
produced malformed or unexpected generated code.
This change adds a shared, Unicode-aware name validator (first character a
letter or `_`, remaining characters letters, digits or `_`) and applies it to
schema names, field names and message names during parsing.
Notes:
- The rule is Unicode-aware to preserve the existing support for non-ASCII
names (e.g. the `TestRecordFieldNames` case).
- Namespaces are intentionally not validated, because the code generator's
namespace-mapping feature rewrites them to C#-specific values (such as
`@return` for reserved words) and re-parses the schema.
Part of AVRO-4312.
## Verifying this change
This change added tests and can be verified as follows:
- Added negative cases to `Schema/SchemaTests.cs` (invalid record name, invalid
field name, and a field name attempting identifier injection) and
`Protocol/ProtocolTest.cs` (`TestInvalidMessageName`).
- Verified locally: the full `Avro.test` suite (1526 tests) passes, including
the AvroGen code-generation tests.
## Documentation
- Does this pull request introduce a new feature? no
- If yes, how is the feature documented? not applicable
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-4312: [csharp] Validate names against the Avro name grammar [avro]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3890:
URL: https://github.com/apache/avro/pull/3890#discussion_r3612679748
##########
lang/csharp/src/apache/main/Schema/SchemaName.cs:
##########
@@ -25,6 +27,76 @@ namespace Avro
/// </summary>
public class SchemaName
{
+ /// <summary>
+ /// Validates that a simple (unqualified) name conforms to the Avro name
+ /// grammar: a non-empty string whose first character is a letter or '_'
+ /// and whose remaining characters are letters, digits or '_'. Letters and
+ /// digits are recognized in a Unicode-aware way, matching the default
+ /// behavior of the Java SDK (and the C# SDK's existing support for
+ /// non-ASCII field names). Throws <see cref="SchemaParseException"/> when
+ /// the name is invalid.
+ /// </summary>
+ /// <param name="name">the simple name to validate</param>
+ /// <param name="what">description of the kind of name, used in error messages</param>
+ internal static void ValidateName(string name, string what)
+ {
+ if (string.IsNullOrEmpty(name)
+ || !(char.IsLetter(name[0]) || name[0] == '_'))
+ {
+ throw new SchemaParseException($"Invalid {what} name: {Quote(name)}");
+ }
+
+ for (int i = 1; i < name.Length; i++)
+ {
+ char c = name[i];
+ if (!(char.IsLetterOrDigit(c) || c == '_'))
+ {
+ throw new SchemaParseException($"Invalid {what} name: {Quote(name)}");
+ }
+ }
+ }
Review Comment:
`ValidateName` uses `char.IsLetter(name[0])` / `char.IsLetterOrDigit(c)`, which operate on UTF-16 code units. That will reject valid supplementary-plane letters/digits (represented by surrogate pairs), despite the method’s comment stating Unicode-aware behavior. Consider iterating by Unicode scalar value using the `char.IsLetter(string,int)` / `char.IsLetterOrDigit(string,int)` overloads and advancing past surrogate pairs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-4312: [csharp] Validate names against the Avro name grammar [avro]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3890:
URL: https://github.com/apache/avro/pull/3890#discussion_r3612645363
##########
lang/csharp/src/apache/main/Schema/Field.cs:
##########
@@ -153,6 +153,8 @@ internal Field(Schema schema, string name, IList<string> aliases, int pos, strin
throw new ArgumentNullException(nameof(name), "name cannot be null.");
}
+ SchemaName.ValidateName(name, "field");
Review Comment:
Field aliases are still accepted verbatim. Since aliases participate in schema resolution (and are also names per the Avro grammar), they should be validated with the same rules as the field name; otherwise invalid aliases can slip through even though field names are now rejected.
##########
lang/csharp/src/apache/main/Schema/SchemaName.cs:
##########
@@ -25,6 +25,35 @@ namespace Avro
/// </summary>
public class SchemaName
{
+ /// <summary>
+ /// Validates that a simple (unqualified) name conforms to the Avro name
+ /// grammar: a non-empty string whose first character is a letter or '_'
+ /// and whose remaining characters are letters, digits or '_'. Letters and
+ /// digits are recognized in a Unicode-aware way, matching the default
+ /// behavior of the Java SDK (and the C# SDK's existing support for
+ /// non-ASCII field names). Throws <see cref="SchemaParseException"/> when
+ /// the name is invalid.
+ /// </summary>
+ /// <param name="name">the simple name to validate</param>
+ /// <param name="what">description of the kind of name, used in error messages</param>
+ internal static void ValidateName(string name, string what)
+ {
+ if (string.IsNullOrEmpty(name)
+ || !(char.IsLetter(name[0]) || name[0] == '_'))
+ {
+ throw new SchemaParseException($"Invalid {what} name: {name}");
+ }
+
+ for (int i = 1; i < name.Length; i++)
+ {
+ char c = name[i];
+ if (!(char.IsLetterOrDigit(c) || c == '_'))
+ {
+ throw new SchemaParseException($"Invalid {what} name: {name}");
+ }
+ }
+ }
Review Comment:
The exception message interpolates the invalid name verbatim. If the input contains newlines/control characters, this can produce multi-line/ambiguous errors (and potentially enable log-forging). Consider escaping \r/\n (and quoting the value) for consistency with other parse errors (e.g., JsonHelper strips newlines).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-4314: [csharp] Validate names against the Avro name grammar [avro]
Posted by "iemejia (via GitHub)" <gi...@apache.org>.
iemejia commented on PR #3890:
URL: https://github.com/apache/avro/pull/3890#issuecomment-5042256803
Superseded by #3895. This PR was automatically closed when its head branch was renamed from `AVRO-4312-csharp-name-validation` to `AVRO-4314-csharp-name-validation` (the work is now tracked under AVRO-4314, independent of the C++ ticket AVRO-4312). Please continue the review in #3895.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-4314: [csharp] Validate names against the Avro name grammar [avro]
Posted by "iemejia (via GitHub)" <gi...@apache.org>.
iemejia closed pull request #3890: AVRO-4314: [csharp] Validate names against the Avro name grammar
URL: https://github.com/apache/avro/pull/3890
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org