adutra commented on code in PR #4479:
URL: https://github.com/apache/polaris/pull/4479#discussion_r3290727784


##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/validation/EntityNameValidator.java:
##########
@@ -27,25 +27,45 @@
  *
  * <ul>
  *   <li>is not null or empty;
- *   <li>does not contain a forward slash ({@code /});
+ *   <li>is not {@code .} or {@code ..};
+ *   <li>does not contain ISO control characters (U+0000–U+001F or 
U+007F–U+009F);
+ *   <li>does not contain any of: {@code / \ : * ? " < > | # + `};
  *   <li>does not start or end with whitespace.
  * </ul>
  */
 public final class EntityNameValidator {
 
   private EntityNameValidator() {}
 
+  /**
+   * Characters forbidden in entity names beyond control characters and 
leading/trailing whitespace.
+   * Covers characters rejected or strongly discouraged by S3, GCS, Azure, 
Windows filesystem
+   * semantics, URL encoding, and shell/template/SQL quoting.
+   */
+  private static final String FORBIDDEN_CHARS = "/\\:*?\"<>|#+`";
+
   /** Validates a single entity name (table, view, namespace level, ...). */
   public static void validateName(String name) {
     if (name == null || name.isEmpty()) {
       throw new IllegalArgumentException("Entity name must not be empty");
     }
-    if (name.indexOf('/') >= 0) {
-      throw new IllegalArgumentException("Entity name must not contain '/': " 
+ name);
+    if (name.equals(".") || name.equals("..")) {
+      throw new IllegalArgumentException("Entity name must not be '.' or 
'..'");
     }
-    if (!name.equals(name.strip())) {
-      throw new IllegalArgumentException(
-          "Entity name must not have leading or trailing whitespace: " + name);
+    for (int i = 0; i < name.length(); i++) {
+      char c = name.charAt(i);
+      if (Character.isISOControl(c)) {
+        throw new IllegalArgumentException(
+            String.format(
+                "Entity name must not contain control characters (U+%04X): 
%s", (int) c, name));

Review Comment:
   I did as suggested, but I'm quite sure that the REST layer already does that.



-- 
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]

Reply via email to