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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
