From fc8f2f99925d3e64443e776fd31bc6425b507c00 Mon Sep 17 00:00:00 2001 From: Neo Date: Sun, 12 Apr 2020 02:01:51 +0800 Subject: [PATCH 1/8] HDDS-3161. Block illegal characters when creating keys. --- .../hdds/scm/client/HddsClientUtils.java | 21 ++++++++++++++++++ .../hadoop/ozone/client/rpc/RpcClient.java | 1 + .../ozone/client/TestHddsClientUtils.java | 22 +++++++++++++++++++ .../java/org/apache/hadoop/ozone/OmUtils.java | 13 +++++++++++ .../om/request/key/OMKeyCreateRequest.java | 4 ++++ 5 files changed, 61 insertions(+) diff --git a/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/client/HddsClientUtils.java b/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/client/HddsClientUtils.java index 8a6518d103ce..04aea4f9bd37 100644 --- a/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/client/HddsClientUtils.java +++ b/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/client/HddsClientUtils.java @@ -196,6 +196,27 @@ public static void verifyResourceName(String... resourceNames) { } } + /** + * verifies that key name is a valid name. + * + * @param keyName key name to be validated + * + * @throws IllegalArgumentException + */ + public static void verifyKeyName(String keyName) { + if (keyName == null) { + throw new IllegalArgumentException("Key name is null"); + } + String regex = "^[^^{}<>^?%~#`\\[\\]\\|\\\\(\\x80-\\xff)]$"; + for (int index = 0; index < keyName.length(); index++) { + char currChar = keyName.charAt(index); + if (!(Character.toString(currChar).matches(regex))){ + throw new IllegalArgumentException("Key name has an " + + "unsupported character : " + currChar); + } + } + } + /** * Checks that object parameters passed as reference is not null. * diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java index 532a7c52623a..b2a1ca5c04d4 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java @@ -618,6 +618,7 @@ public OzoneOutputStream createKey( Map metadata) throws IOException { HddsClientUtils.verifyResourceName(volumeName, bucketName); + HddsClientUtils.verifyKeyName(keyName); HddsClientUtils.checkNotNull(keyName, type, factor); String requestId = UUID.randomUUID().toString(); diff --git a/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/TestHddsClientUtils.java b/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/TestHddsClientUtils.java index 8f8659d6e9d8..6e1e33600c37 100644 --- a/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/TestHddsClientUtils.java +++ b/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/TestHddsClientUtils.java @@ -262,4 +262,26 @@ public void testVerifyResourceName() { } } } + + @Test + public void testVerifyKeyName() { + List invalidNames = new ArrayList<>(); + invalidNames.add("#"); + invalidNames.add("ab^cd"); + invalidNames.add("test|name~"); + invalidNames.add("~hi!ozone"); + invalidNames.add("test"); + invalidNames.add("10%3=1"); + invalidNames.add("photo[0201]"); + invalidNames.add("what?"); + + for (String name : invalidNames) { + try { + HddsClientUtils.verifyKeyName(name); + fail("Did not reject invalid string [" + name + "] as a name"); + } catch (IllegalArgumentException e) { + // throwing up on an invalid name. it's working. + } + } + } } diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java index 8f12b6ea46ec..d0d5f940f71c 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java @@ -506,4 +506,17 @@ public static long getOMClientRpcTimeOut(Configuration configuration) { return OzoneConfiguration.of(configuration) .getObject(OMClientConfig.class).getRpcTimeOut(); } + + /** + * Verify key name is a valid name. + */ + public static void validateKeyName(String keyName) + throws OMException { + try { + HddsClientUtils.verifyKeyName(keyName); + } catch (IllegalArgumentException e) { + throw new OMException("Invalid key name: " + keyName, + OMException.ResultCodes.INVALID_KEY_NAME); + } + } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java index 3ec2f0370a5a..74f0e6332d44 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java @@ -26,6 +26,7 @@ import com.google.common.base.Optional; import com.google.common.base.Preconditions; +import org.apache.hadoop.ozone.OmUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -84,6 +85,9 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { KeyArgs keyArgs = createKeyRequest.getKeyArgs(); + // Verify key name + OmUtils.validateKeyName(keyArgs.getKeyName()); + // We cannot allocate block for multipart upload part when // createMultipartKey is called, as we will not know type and factor with // which initiateMultipartUpload has started for this key. When From a91e7f8186e1d34804296a9a889f273672dca1ef Mon Sep 17 00:00:00 2001 From: Neo Date: Mon, 20 Apr 2020 22:48:41 +0800 Subject: [PATCH 2/8] Moving regular expression to OzoneConsts file. Signed-off-by: Neo --- .../hadoop/hdds/scm/client/HddsClientUtils.java | 4 ++-- .../java/org/apache/hadoop/ozone/OzoneConsts.java | 13 +++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/client/HddsClientUtils.java b/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/client/HddsClientUtils.java index 04aea4f9bd37..01eebd58c3bd 100644 --- a/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/client/HddsClientUtils.java +++ b/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/client/HddsClientUtils.java @@ -207,10 +207,10 @@ public static void verifyKeyName(String keyName) { if (keyName == null) { throw new IllegalArgumentException("Key name is null"); } - String regex = "^[^^{}<>^?%~#`\\[\\]\\|\\\\(\\x80-\\xff)]$"; for (int index = 0; index < keyName.length(); index++) { char currChar = keyName.charAt(index); - if (!(Character.toString(currChar).matches(regex))){ + if (!(Character.toString(currChar) + .matches(OzoneConsts.KEYNAME_AVOID_CHARACTERS_REGEX))){ throw new IllegalArgumentException("Key name has an " + "unsupported character : " + currChar); } diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java index a88acc2a6a90..ec10f1053f51 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java @@ -324,5 +324,18 @@ private OzoneConsts() { public static final String GDPR_SECRET = "secret"; public static final String GDPR_ALGORITHM = "algorithm"; + /** + * 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: + * "\", "{", "}", "^", "<", ">", "#", "|", "%", "`", "[", "]", "~", "?" + * and Non-printable ASCII characters (128–255 decimal characters). + * https://docs.aws.amazon.com/AmazonS3/latest/dev/UsingMetadata.html + */ + public static final String KEYNAME_AVOID_CHARACTERS_REGEX = + "^[^^{}<>^?%~#`\\[\\]\\|\\\\(\\x80-\\xff)]$"; } From 78f9e99eace6affdd1bc4e6510c95902acef497d Mon Sep 17 00:00:00 2001 From: Neo Date: Mon, 20 Apr 2020 23:11:23 +0800 Subject: [PATCH 3/8] Added key name check in some requests. Signed-off-by: Neo --- .../src/main/java/org/apache/hadoop/ozone/OzoneConsts.java | 1 + .../java/org/apache/hadoop/ozone/client/rpc/RpcClient.java | 2 ++ .../hadoop/ozone/om/request/file/OMFileCreateRequest.java | 7 +++++++ .../hadoop/ozone/om/request/key/OMKeyCommitRequest.java | 7 +++++++ .../hadoop/ozone/om/request/key/OMKeyRenameRequest.java | 4 ++++ 5 files changed, 21 insertions(+) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java index ec10f1053f51..c14e76031388 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java @@ -338,4 +338,5 @@ private OzoneConsts() { public static final String KEYNAME_AVOID_CHARACTERS_REGEX = "^[^^{}<>^?%~#`\\[\\]\\|\\\\(\\x80-\\xff)]$"; + public static final String FS_FILE_COPYING_TEMP_SUFFIX= "._COPYING_"; } diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java index b2a1ca5c04d4..567bfb27e030 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java @@ -696,6 +696,7 @@ public void deleteKey( public void renameKey(String volumeName, String bucketName, String fromKeyName, String toKeyName) throws IOException { HddsClientUtils.verifyResourceName(volumeName, bucketName); + HddsClientUtils.verifyKeyName(toKeyName); HddsClientUtils.checkNotNull(fromKeyName, toKeyName); OmKeyArgs keyArgs = new OmKeyArgs.Builder() .setVolumeName(volumeName) @@ -871,6 +872,7 @@ public OzoneOutputStream createMultipartKey(String volumeName, String uploadID) throws IOException { HddsClientUtils.verifyResourceName(volumeName, bucketName); + HddsClientUtils.verifyKeyName(keyName); HddsClientUtils.checkNotNull(keyName, uploadID); Preconditions.checkArgument(partNumber > 0 && partNumber <=10000, "Part " + "number should be greater than zero and less than or equal to 10000"); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java index 2fa9ba196c45..f195548bdcf4 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java @@ -27,7 +27,10 @@ import com.google.common.base.Optional; import com.google.common.base.Preconditions; +import org.apache.commons.lang3.StringUtils; +import org.apache.hadoop.ozone.OmUtils; import org.apache.hadoop.ozone.OzoneAcl; +import org.apache.hadoop.ozone.OzoneConsts; import org.apache.hadoop.ozone.om.exceptions.OMReplayException; import org.apache.hadoop.ozone.om.response.file.OMFileCreateResponse; import org.slf4j.Logger; @@ -90,6 +93,10 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { KeyArgs keyArgs = createFileRequest.getKeyArgs(); + // Verify key name + OmUtils.validateKeyName(StringUtils.removeEnd(keyArgs.getKeyName(), + OzoneConsts.FS_FILE_COPYING_TEMP_SUFFIX)); + if (keyArgs.getKeyName().length() == 0) { // Check if this is the root of the filesystem. // Not throwing exception here, as need to throw exception after diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java index e059ca0fba81..a2bf108a31a2 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java @@ -25,6 +25,9 @@ import com.google.common.base.Optional; import com.google.common.base.Preconditions; +import org.apache.commons.lang3.StringUtils; +import org.apache.hadoop.ozone.OmUtils; +import org.apache.hadoop.ozone.OzoneConsts; import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper; import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer; import org.slf4j.Logger; @@ -83,6 +86,10 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { KeyArgs keyArgs = commitKeyRequest.getKeyArgs(); + // Verify key name + OmUtils.validateKeyName(StringUtils.removeEnd(keyArgs.getKeyName(), + OzoneConsts.FS_FILE_COPYING_TEMP_SUFFIX)); + KeyArgs.Builder newKeyArgs = keyArgs.toBuilder().setModificationTime(Time.now()); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java index ca97067d808d..8cb8f6ff42e7 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java @@ -23,6 +23,7 @@ import com.google.common.base.Optional; import com.google.common.base.Preconditions; +import org.apache.hadoop.ozone.OmUtils; import org.apache.hadoop.ozone.OzoneConsts; import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper; import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer; @@ -83,6 +84,9 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { RenameKeyRequest renameKeyRequest = getOmRequest().getRenameKeyRequest(); Preconditions.checkNotNull(renameKeyRequest); + // Verify key name + OmUtils.validateKeyName(renameKeyRequest.getToKeyName()); + // Set modification time. KeyArgs.Builder newKeyArgs = renameKeyRequest.getKeyArgs().toBuilder() .setModificationTime(Time.now()); From ab75c37044d9f5a079731b144f875576f9fd2adb Mon Sep 17 00:00:00 2001 From: Neo Date: Sat, 25 Apr 2020 01:48:51 +0800 Subject: [PATCH 4/8] pre-compiled regex patterns & check the entire string. --- .../hadoop/hdds/scm/client/HddsClientUtils.java | 11 ++++------- .../java/org/apache/hadoop/ozone/OzoneConsts.java | 6 ++++-- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/client/HddsClientUtils.java b/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/client/HddsClientUtils.java index 01eebd58c3bd..edbbc5ea734f 100644 --- a/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/client/HddsClientUtils.java +++ b/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/client/HddsClientUtils.java @@ -207,13 +207,10 @@ public static void verifyKeyName(String keyName) { if (keyName == null) { throw new IllegalArgumentException("Key name is null"); } - for (int index = 0; index < keyName.length(); index++) { - char currChar = keyName.charAt(index); - if (!(Character.toString(currChar) - .matches(OzoneConsts.KEYNAME_AVOID_CHARACTERS_REGEX))){ - throw new IllegalArgumentException("Key name has an " + - "unsupported character : " + currChar); - } + if(!OzoneConsts.KEYNAME_CHECK_ILLGEAL_CHARACTERS_REGEX + .matcher(keyName).matches()){ + throw new IllegalArgumentException("key name contains " + + "illegal characters."); } } diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java index c14e76031388..88edfd84646a 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java @@ -23,6 +23,8 @@ import org.apache.ratis.thirdparty.io.grpc.Context; import org.apache.ratis.thirdparty.io.grpc.Metadata; +import java.util.regex.Pattern; + import static org.apache.ratis.thirdparty.io.grpc.Metadata.ASCII_STRING_MARSHALLER; /** @@ -335,8 +337,8 @@ private OzoneConsts() { * and Non-printable ASCII characters (128–255 decimal characters). * https://docs.aws.amazon.com/AmazonS3/latest/dev/UsingMetadata.html */ - public static final String KEYNAME_AVOID_CHARACTERS_REGEX = - "^[^^{}<>^?%~#`\\[\\]\\|\\\\(\\x80-\\xff)]$"; + public static final Pattern KEYNAME_CHECK_ILLGEAL_CHARACTERS_REGEX = + Pattern.compile("^[^^{}<>^?%~#`\\[\\]\\|\\\\(\\x80-\\xff)]$"); public static final String FS_FILE_COPYING_TEMP_SUFFIX= "._COPYING_"; } From 4e14e818441d3e0c075f573265d0a25d1e9e9518 Mon Sep 17 00:00:00 2001 From: Neo Date: Sat, 25 Apr 2020 04:08:36 +0800 Subject: [PATCH 5/8] hange this check to optionl --- .../hadoop/hdds/scm/client/HddsClientUtils.java | 2 +- .../org/apache/hadoop/ozone/OzoneConsts.java | 2 +- .../common/src/main/resources/ozone-default.xml | 10 ++++++++++ .../hadoop/ozone/client/rpc/RpcClient.java | 17 ++++++++++++++--- .../apache/hadoop/ozone/om/OMConfigKeys.java | 5 +++++ .../om/request/file/OMFileCreateRequest.java | 10 ++++++++-- .../om/request/key/OMKeyCommitRequest.java | 10 ++++++++-- .../om/request/key/OMKeyCreateRequest.java | 9 +++++++-- .../om/request/key/OMKeyRenameRequest.java | 8 +++++++- 9 files changed, 61 insertions(+), 12 deletions(-) diff --git a/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/client/HddsClientUtils.java b/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/client/HddsClientUtils.java index edbbc5ea734f..e56484a38586 100644 --- a/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/client/HddsClientUtils.java +++ b/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/client/HddsClientUtils.java @@ -207,7 +207,7 @@ public static void verifyKeyName(String keyName) { if (keyName == null) { throw new IllegalArgumentException("Key name is null"); } - if(!OzoneConsts.KEYNAME_CHECK_ILLGEAL_CHARACTERS_REGEX + if(!OzoneConsts.KEYNAME_ILLEGAL_CHARACTER_CHECK_REGEX .matcher(keyName).matches()){ throw new IllegalArgumentException("key name contains " + "illegal characters."); diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java index 88edfd84646a..e8b42f5ea16a 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java @@ -337,7 +337,7 @@ private OzoneConsts() { * and Non-printable ASCII characters (128–255 decimal characters). * https://docs.aws.amazon.com/AmazonS3/latest/dev/UsingMetadata.html */ - public static final Pattern KEYNAME_CHECK_ILLGEAL_CHARACTERS_REGEX = + public static final Pattern KEYNAME_ILLEGAL_CHARACTER_CHECK_REGEX = Pattern.compile("^[^^{}<>^?%~#`\\[\\]\\|\\\\(\\x80-\\xff)]$"); public static final String FS_FILE_COPYING_TEMP_SUFFIX= "._COPYING_"; diff --git a/hadoop-hdds/common/src/main/resources/ozone-default.xml b/hadoop-hdds/common/src/main/resources/ozone-default.xml index 96a1128d0891..c3141e0e8d75 100644 --- a/hadoop-hdds/common/src/main/resources/ozone-default.xml +++ b/hadoop-hdds/common/src/main/resources/ozone-default.xml @@ -2581,4 +2581,14 @@ Timeout for the request submitted directly to Ratis in datanode. + + ozone.om.keyname.character.check.enabled + OZONE, OM + false + If true, then enable to check if the key name + contains illegal characters when creating/renaming key. + For the definition of illegal characters, follow the + rules in Amazon S3's object key naming guide. + + diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java index 567bfb27e030..ff62c4736b1d 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java @@ -73,6 +73,7 @@ import org.apache.hadoop.ozone.client.io.OzoneInputStream; import org.apache.hadoop.ozone.client.io.OzoneOutputStream; import org.apache.hadoop.ozone.client.protocol.ClientProtocol; +import org.apache.hadoop.ozone.om.OMConfigKeys; import org.apache.hadoop.ozone.om.ha.OMFailoverProxyProvider; import org.apache.hadoop.ozone.om.helpers.BucketEncryptionKeyInfo; import org.apache.hadoop.ozone.om.helpers.OmBucketArgs; @@ -144,6 +145,7 @@ public class RpcClient implements ClientProtocol { private final long retryInterval; private Text dtService; private final boolean topologyAwareReadEnabled; + private final boolean checkKeyNameEnabled; /** * Creates RpcClient instance with the given configuration. @@ -240,6 +242,9 @@ public RpcClient(ConfigurationSource conf, String omServiceId) topologyAwareReadEnabled = conf.getBoolean( OzoneConfigKeys.OZONE_NETWORK_TOPOLOGY_AWARE_READ_KEY, OzoneConfigKeys.OZONE_NETWORK_TOPOLOGY_AWARE_READ_DEFAULT); + checkKeyNameEnabled = conf.getBoolean( + OMConfigKeys.OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_KEY, + OMConfigKeys.OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_DEFAULT); } @Override @@ -618,7 +623,9 @@ public OzoneOutputStream createKey( Map metadata) throws IOException { HddsClientUtils.verifyResourceName(volumeName, bucketName); - HddsClientUtils.verifyKeyName(keyName); + if(checkKeyNameEnabled) { + HddsClientUtils.verifyKeyName(keyName); + } HddsClientUtils.checkNotNull(keyName, type, factor); String requestId = UUID.randomUUID().toString(); @@ -696,7 +703,9 @@ public void deleteKey( public void renameKey(String volumeName, String bucketName, String fromKeyName, String toKeyName) throws IOException { HddsClientUtils.verifyResourceName(volumeName, bucketName); - HddsClientUtils.verifyKeyName(toKeyName); + if(checkKeyNameEnabled){ + HddsClientUtils.verifyKeyName(toKeyName); + } HddsClientUtils.checkNotNull(fromKeyName, toKeyName); OmKeyArgs keyArgs = new OmKeyArgs.Builder() .setVolumeName(volumeName) @@ -872,7 +881,9 @@ public OzoneOutputStream createMultipartKey(String volumeName, String uploadID) throws IOException { HddsClientUtils.verifyResourceName(volumeName, bucketName); - HddsClientUtils.verifyKeyName(keyName); + if(checkKeyNameEnabled) { + HddsClientUtils.verifyKeyName(keyName); + } HddsClientUtils.checkNotNull(keyName, uploadID); Preconditions.checkArgument(partNumber > 0 && partNumber <=10000, "Part " + "number should be greater than zero and less than or equal to 10000"); diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java index 7af6627d05df..7a6ca5e6bff9 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java @@ -231,4 +231,9 @@ private OMConfigKeys() { // hadoop-policy.xml, "*" allows all users/groups to access. public static final String OZONE_OM_SECURITY_CLIENT_PROTOCOL_ACL = "ozone.om.security.client.protocol.acl"; + + public static final String OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_KEY = + "ozone.om.keyname.character.check.enabled"; + public static final boolean OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_DEFAULT = + false; } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java index f195548bdcf4..3bc3b536f26d 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java @@ -31,6 +31,7 @@ import org.apache.hadoop.ozone.OmUtils; import org.apache.hadoop.ozone.OzoneAcl; import org.apache.hadoop.ozone.OzoneConsts; +import org.apache.hadoop.ozone.om.OMConfigKeys; import org.apache.hadoop.ozone.om.exceptions.OMReplayException; import org.apache.hadoop.ozone.om.response.file.OMFileCreateResponse; import org.slf4j.Logger; @@ -94,8 +95,13 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { KeyArgs keyArgs = createFileRequest.getKeyArgs(); // Verify key name - OmUtils.validateKeyName(StringUtils.removeEnd(keyArgs.getKeyName(), - OzoneConsts.FS_FILE_COPYING_TEMP_SUFFIX)); + final boolean checkKeyNameEnabled = ozoneManager.getConfiguration() + .getBoolean(OMConfigKeys.OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_KEY, + OMConfigKeys.OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_DEFAULT); + if(checkKeyNameEnabled){ + OmUtils.validateKeyName(StringUtils.removeEnd(keyArgs.getKeyName(), + OzoneConsts.FS_FILE_COPYING_TEMP_SUFFIX)); + } if (keyArgs.getKeyName().length() == 0) { // Check if this is the root of the filesystem. diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java index a2bf108a31a2..53229ca2e8ae 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java @@ -28,6 +28,7 @@ import org.apache.commons.lang3.StringUtils; import org.apache.hadoop.ozone.OmUtils; import org.apache.hadoop.ozone.OzoneConsts; +import org.apache.hadoop.ozone.om.OMConfigKeys; import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper; import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer; import org.slf4j.Logger; @@ -87,8 +88,13 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { KeyArgs keyArgs = commitKeyRequest.getKeyArgs(); // Verify key name - OmUtils.validateKeyName(StringUtils.removeEnd(keyArgs.getKeyName(), - OzoneConsts.FS_FILE_COPYING_TEMP_SUFFIX)); + final boolean checkKeyNameEnabled = ozoneManager.getConfiguration() + .getBoolean(OMConfigKeys.OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_KEY, + OMConfigKeys.OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_DEFAULT); + if(checkKeyNameEnabled){ + OmUtils.validateKeyName(StringUtils.removeEnd(keyArgs.getKeyName(), + OzoneConsts.FS_FILE_COPYING_TEMP_SUFFIX)); + } KeyArgs.Builder newKeyArgs = keyArgs.toBuilder().setModificationTime(Time.now()); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java index 74f0e6332d44..e828ed678cc3 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java @@ -27,6 +27,7 @@ import com.google.common.base.Optional; import com.google.common.base.Preconditions; import org.apache.hadoop.ozone.OmUtils; +import org.apache.hadoop.ozone.om.OMConfigKeys; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -86,8 +87,12 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { KeyArgs keyArgs = createKeyRequest.getKeyArgs(); // Verify key name - OmUtils.validateKeyName(keyArgs.getKeyName()); - + final boolean checkKeyNameEnabled = ozoneManager.getConfiguration() + .getBoolean(OMConfigKeys.OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_KEY, + OMConfigKeys.OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_DEFAULT); + if(checkKeyNameEnabled){ + OmUtils.validateKeyName(keyArgs.getKeyName()); + } // We cannot allocate block for multipart upload part when // createMultipartKey is called, as we will not know type and factor with // which initiateMultipartUpload has started for this key. When diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java index 8cb8f6ff42e7..e53dfe8db821 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java @@ -25,6 +25,7 @@ import com.google.common.base.Preconditions; import org.apache.hadoop.ozone.OmUtils; import org.apache.hadoop.ozone.OzoneConsts; +import org.apache.hadoop.ozone.om.OMConfigKeys; import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper; import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer; import org.apache.hadoop.ozone.security.acl.OzoneObj; @@ -85,7 +86,12 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { Preconditions.checkNotNull(renameKeyRequest); // Verify key name - OmUtils.validateKeyName(renameKeyRequest.getToKeyName()); + final boolean checkKeyNameEnabled = ozoneManager.getConfiguration() + .getBoolean(OMConfigKeys.OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_KEY, + OMConfigKeys.OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_DEFAULT); + if(checkKeyNameEnabled){ + OmUtils.validateKeyName(renameKeyRequest.getToKeyName()); + } // Set modification time. KeyArgs.Builder newKeyArgs = renameKeyRequest.getKeyArgs().toBuilder() From 8973cd7f1090acc1f27f93ab2fb116acd0c56cb1 Mon Sep 17 00:00:00 2001 From: Neo Date: Sat, 25 Apr 2020 04:26:25 +0800 Subject: [PATCH 6/8] fixed checkstyle --- .../hadoop/ozone/om/request/file/OMFileCreateRequest.java | 4 ++-- .../hadoop/ozone/om/request/key/OMKeyCommitRequest.java | 4 ++-- .../hadoop/ozone/om/request/key/OMKeyCreateRequest.java | 4 ++-- .../hadoop/ozone/om/request/key/OMKeyRenameRequest.java | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java index 3bc3b536f26d..b4a9baa2d756 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java @@ -96,8 +96,8 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { // Verify key name final boolean checkKeyNameEnabled = ozoneManager.getConfiguration() - .getBoolean(OMConfigKeys.OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_KEY, - OMConfigKeys.OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_DEFAULT); + .getBoolean(OMConfigKeys.OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_KEY, + OMConfigKeys.OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_DEFAULT); if(checkKeyNameEnabled){ OmUtils.validateKeyName(StringUtils.removeEnd(keyArgs.getKeyName(), OzoneConsts.FS_FILE_COPYING_TEMP_SUFFIX)); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java index 53229ca2e8ae..936187581ede 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java @@ -89,8 +89,8 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { // Verify key name final boolean checkKeyNameEnabled = ozoneManager.getConfiguration() - .getBoolean(OMConfigKeys.OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_KEY, - OMConfigKeys.OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_DEFAULT); + .getBoolean(OMConfigKeys.OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_KEY, + OMConfigKeys.OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_DEFAULT); if(checkKeyNameEnabled){ OmUtils.validateKeyName(StringUtils.removeEnd(keyArgs.getKeyName(), OzoneConsts.FS_FILE_COPYING_TEMP_SUFFIX)); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java index e828ed678cc3..f40f1c88bdf7 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java @@ -88,8 +88,8 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { // Verify key name final boolean checkKeyNameEnabled = ozoneManager.getConfiguration() - .getBoolean(OMConfigKeys.OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_KEY, - OMConfigKeys.OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_DEFAULT); + .getBoolean(OMConfigKeys.OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_KEY, + OMConfigKeys.OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_DEFAULT); if(checkKeyNameEnabled){ OmUtils.validateKeyName(keyArgs.getKeyName()); } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java index e53dfe8db821..f0ab7218668a 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java @@ -87,8 +87,8 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { // Verify key name final boolean checkKeyNameEnabled = ozoneManager.getConfiguration() - .getBoolean(OMConfigKeys.OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_KEY, - OMConfigKeys.OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_DEFAULT); + .getBoolean(OMConfigKeys.OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_KEY, + OMConfigKeys.OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_DEFAULT); if(checkKeyNameEnabled){ OmUtils.validateKeyName(renameKeyRequest.getToKeyName()); } From 9ffb4d6347836766dff6130e14043ca0de33db32 Mon Sep 17 00:00:00 2001 From: Neo Date: Sat, 25 Apr 2020 05:45:53 +0800 Subject: [PATCH 7/8] fix bugs --- .../src/main/java/org/apache/hadoop/ozone/OzoneConsts.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java index e8b42f5ea16a..e125d95362db 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java @@ -338,7 +338,7 @@ private OzoneConsts() { * https://docs.aws.amazon.com/AmazonS3/latest/dev/UsingMetadata.html */ public static final Pattern KEYNAME_ILLEGAL_CHARACTER_CHECK_REGEX = - Pattern.compile("^[^^{}<>^?%~#`\\[\\]\\|\\\\(\\x80-\\xff)]$"); + Pattern.compile("^[^^{}<>^?%~#`\\[\\]\\|\\\\(\\x80-\\xff)]+$"); public static final String FS_FILE_COPYING_TEMP_SUFFIX= "._COPYING_"; } From 17e40c23392df10bbfdf5e737b417ddda82f43c0 Mon Sep 17 00:00:00 2001 From: Neo Date: Wed, 6 May 2020 00:18:16 +0800 Subject: [PATCH 8/8] update message of exception --- .../org/apache/hadoop/hdds/scm/client/HddsClientUtils.java | 3 +-- .../common/src/main/java/org/apache/hadoop/ozone/OmUtils.java | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/client/HddsClientUtils.java b/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/client/HddsClientUtils.java index e56484a38586..b294fc7ff279 100644 --- a/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/client/HddsClientUtils.java +++ b/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/client/HddsClientUtils.java @@ -209,8 +209,7 @@ public static void verifyKeyName(String keyName) { } if(!OzoneConsts.KEYNAME_ILLEGAL_CHARACTER_CHECK_REGEX .matcher(keyName).matches()){ - throw new IllegalArgumentException("key name contains " + - "illegal characters."); + throw new IllegalArgumentException("Invalid key name: " + keyName); } } diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java index d0d5f940f71c..c3907681bb78 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java @@ -515,7 +515,7 @@ public static void validateKeyName(String keyName) try { HddsClientUtils.verifyKeyName(keyName); } catch (IllegalArgumentException e) { - throw new OMException("Invalid key name: " + keyName, + throw new OMException(e.getMessage(), OMException.ResultCodes.INVALID_KEY_NAME); } }