You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by st...@apache.org on 2018/08/30 13:34:35 UTC

hadoop git commit: HADOOP-15667. FileSystemMultipartUploader should verify that UploadHandle has non-0 length. Contributed by Ewan Higgs

Repository: hadoop
Updated Branches:
  refs/heads/trunk 781437c21 -> 2e6c1109d


HADOOP-15667. FileSystemMultipartUploader should verify that UploadHandle has non-0 length.
Contributed by Ewan Higgs


Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/2e6c1109
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/2e6c1109
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/2e6c1109

Branch: refs/heads/trunk
Commit: 2e6c1109dcdeedb59a3345047e9201271c9a0b27
Parents: 781437c
Author: Steve Loughran <st...@apache.org>
Authored: Thu Aug 30 14:33:16 2018 +0100
Committer: Steve Loughran <st...@apache.org>
Committed: Thu Aug 30 14:33:16 2018 +0100

----------------------------------------------------------------------
 .../hadoop/fs/FileSystemMultipartUploader.java  |  6 ++-
 .../org/apache/hadoop/fs/MultipartUploader.java | 11 +++++
 .../AbstractContractMultipartUploaderTest.java  | 43 ++++++++++++++++++++
 .../hadoop/fs/s3a/S3AMultipartUploader.java     | 10 ++---
 4 files changed, 61 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hadoop/blob/2e6c1109/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystemMultipartUploader.java
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystemMultipartUploader.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystemMultipartUploader.java
index a700a9f..f13b50b 100644
--- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystemMultipartUploader.java
+++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystemMultipartUploader.java
@@ -68,6 +68,7 @@ public class FileSystemMultipartUploader extends MultipartUploader {
       throws IOException {
 
     byte[] uploadIdByteArray = uploadId.toByteArray();
+    checkUploadId(uploadIdByteArray);
     Path collectorPath = new Path(new String(uploadIdByteArray, 0,
         uploadIdByteArray.length, Charsets.UTF_8));
     Path partPath =
@@ -101,6 +102,8 @@ public class FileSystemMultipartUploader extends MultipartUploader {
       List<Pair<Integer, PartHandle>> handles, UploadHandle multipartUploadId)
       throws IOException {
 
+    checkUploadId(multipartUploadId.toByteArray());
+
     if (handles.isEmpty()) {
       throw new IOException("Empty upload");
     }
@@ -133,8 +136,7 @@ public class FileSystemMultipartUploader extends MultipartUploader {
   @Override
   public void abort(Path filePath, UploadHandle uploadId) throws IOException {
     byte[] uploadIdByteArray = uploadId.toByteArray();
-    Preconditions.checkArgument(uploadIdByteArray.length != 0,
-        "UploadId is empty");
+    checkUploadId(uploadIdByteArray);
     Path collectorPath = new Path(new String(uploadIdByteArray, 0,
         uploadIdByteArray.length, Charsets.UTF_8));
 

http://git-wip-us.apache.org/repos/asf/hadoop/blob/2e6c1109/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/MultipartUploader.java
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/MultipartUploader.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/MultipartUploader.java
index 47fd9f2..76f58d3 100644
--- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/MultipartUploader.java
+++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/MultipartUploader.java
@@ -21,6 +21,7 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.util.List;
 
+import com.google.common.base.Preconditions;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -91,4 +92,14 @@ public abstract class MultipartUploader {
   public abstract void abort(Path filePath, UploadHandle multipartUploadId)
       throws IOException;
 
+  /**
+   * Utility method to validate uploadIDs
+   * @param uploadId
+   * @throws IllegalArgumentException
+   */
+  protected void checkUploadId(byte[] uploadId)
+      throws IllegalArgumentException {
+    Preconditions.checkArgument(uploadId.length > 0,
+        "Empty UploadId is not valid");
+  }
 }

http://git-wip-us.apache.org/repos/asf/hadoop/blob/2e6c1109/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractMultipartUploaderTest.java
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractMultipartUploaderTest.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractMultipartUploaderTest.java
index c0e1600..85a6861 100644
--- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractMultipartUploaderTest.java
+++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractMultipartUploaderTest.java
@@ -297,4 +297,47 @@ public abstract class AbstractContractMultipartUploaderTest extends
         () -> mpu.complete(dest, new ArrayList<>(), handle));
     mpu.abort(dest, handle);
   }
+
+  /**
+   * When we pass empty uploadID, putPart throws IllegalArgumentException.
+   * @throws Exception
+   */
+  @Test
+  public void testPutPartEmptyUploadID() throws Exception {
+    describe("Expect IllegalArgumentException when putPart uploadID is empty");
+    FileSystem fs = getFileSystem();
+    Path dest = path("testCompleteEmptyUpload");
+    MultipartUploader mpu = MultipartUploaderFactory.get(fs, null);
+    mpu.initialize(dest);
+    UploadHandle emptyHandle =
+        BBUploadHandle.from(ByteBuffer.wrap(new byte[0]));
+    byte[] payload = generatePayload(1);
+    InputStream is = new ByteArrayInputStream(payload);
+    intercept(IllegalArgumentException.class,
+        () -> mpu.putPart(dest, is, 1, emptyHandle, payload.length));
+  }
+
+  /**
+   * When we pass empty uploadID, complete throws IllegalArgumentException.
+   * @throws Exception
+   */
+  @Test
+  public void testCompleteEmptyUploadID() throws Exception {
+    describe("Expect IllegalArgumentException when complete uploadID is empty");
+    FileSystem fs = getFileSystem();
+    Path dest = path("testCompleteEmptyUpload");
+    MultipartUploader mpu = MultipartUploaderFactory.get(fs, null);
+    UploadHandle realHandle = mpu.initialize(dest);
+    UploadHandle emptyHandle =
+        BBUploadHandle.from(ByteBuffer.wrap(new byte[0]));
+    List<Pair<Integer, PartHandle>> partHandles = new ArrayList<>();
+    byte[] payload = generatePayload(1);
+    InputStream is = new ByteArrayInputStream(payload);
+    PartHandle partHandle = mpu.putPart(dest, is, 1, realHandle,
+        payload.length);
+    partHandles.add(Pair.of(1, partHandle));
+
+    intercept(IllegalArgumentException.class,
+        () -> mpu.complete(dest, partHandles, emptyHandle));
+  }
 }

http://git-wip-us.apache.org/repos/asf/hadoop/blob/2e6c1109/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AMultipartUploader.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AMultipartUploader.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AMultipartUploader.java
index 6a1df54..4a6cb8c 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AMultipartUploader.java
+++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AMultipartUploader.java
@@ -84,9 +84,10 @@ public class S3AMultipartUploader extends MultipartUploader {
   public PartHandle putPart(Path filePath, InputStream inputStream,
       int partNumber, UploadHandle uploadId, long lengthInBytes)
       throws IOException {
-    final WriteOperationHelper writeHelper = s3a.getWriteOperationHelper();
-    String key = s3a.pathToKey(filePath);
     byte[] uploadIdBytes = uploadId.toByteArray();
+    checkUploadId(uploadIdBytes);
+    String key = s3a.pathToKey(filePath);
+    final WriteOperationHelper writeHelper = s3a.getWriteOperationHelper();
     String uploadIdString = new String(uploadIdBytes, 0, uploadIdBytes.length,
         Charsets.UTF_8);
     UploadPartRequest request = writeHelper.newUploadPartRequest(key,
@@ -155,11 +156,6 @@ public class S3AMultipartUploader extends MultipartUploader {
     }
   }
 
-  private void checkUploadId(byte[] uploadId) throws IllegalArgumentException {
-    Preconditions.checkArgument(uploadId.length > 0,
-        "Empty UploadId is not valid");
-  }
-
   /**
    * Build the payload for marshalling.
    * @param eTag upload etag


---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org