Re: [PR] AVRO-4312: [csharp] Validate names against the Avro name grammar [avro]

["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