ChenSammi commented on code in PR #10781:
URL: https://github.com/apache/ozone/pull/10781#discussion_r4376666276
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java:
##########
@@ -180,6 +180,11 @@ public final class OzoneConsts {
public static final String OM_KEY_PREFIX = "/";
public static final String DOUBLE_SLASH_OM_KEY_PREFIX = "//";
+ /**
+ * Separates the key name from the versionId in versionedKeyTable DB keys.
+ * Not '/', since key names may contain it.
+ */
+ public static final String OM_VERSIONED_KEY_SEPARATOR = "\0";
Review Comment:
I'm not very sure that "\0" is safe or not.
According to AWS S3 doc,
https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-keys.html#object-key-guidelines,
"ASCII character ranges 00–1F hex (0–31 decimal) and 7F (127 decimal) " is
"Characters that might require special handling". "/" falls to this category
too.
And there is also a "Characters to avoid" list, which looks like a better
choice.
In the meanwhile, key name check in Ozone is controlled by
"ozone.om.keyname.character.check.enabled" which default is false.
If it's enabled, it verify the key name against following, which includes
the "Characters to avoid" list.
```
/**
* Block key name as illegal characters
*
* This regular expression is used to check if key name
* contains illegal characters when creating/renaming key.
*
* Avoid the following characters in a key name:
* {@literal "\", "{", "}", "<", ">", "^", "%", "~", "#", "|", "`", "[",
"]"}, Quotation
* marks and Non-printable ASCII characters (128–255 decimal characters).
* https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-keys.html
*/
public static final Pattern KEYNAME_ILLEGAL_CHARACTER_CHECK_REGEX =
Pattern.compile("^[^\\\\{}<>^%~#|`\\[\\]\"\\x80-\\xff]+$");
```
So basically, both "/" and "\0" are allowed whether
"ozone.om.keyname.character.check.enabled" is true or not.
Maybe we should choose some character from the "Characters to avoid" list,
make "ozone.om.keyname.character.check.enabled" default to true, at least for
OBS bucket.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
