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


##########
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, and 
Windows filesystem
+   * semantics.
+   */
+  private static final String FORBIDDEN_CHARS = "/\\:*?\"<>|#";

Review Comment:
   It's correct that this PR is broader than the original ML thread. It now 
defines general REST-layer entity-name semantics, and also bans additional 
characters beyond slash/empty/leading/trailing whitespace.
   
   I think this defense-in-depth restriction is justified, especially because 
names flow into storage paths and downstream provider code.
   
   The changelog entry is the right place for the compatibility warning. Given 
that compatibility warning and the narrow implementation scope, I think we can 
move on with this change.



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