getPartNumber

linkedin · Jan 15, 2025 · 8936836 · 8936836
1 parent 0df6043
commit 8936836
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 14 deletions.
diff --git a/ambry-frontend/src/main/java/com/github/ambry/frontend/s3/S3Constants.java b/ambry-frontend/src/main/java/com/github/ambry/frontend/s3/S3Constants.java
@@ -22,7 +22,7 @@ public class S3Constants {
   // Error Messages
   public static final String ERR_INVALID_MULTIPART_UPLOAD = "Invalid multipart upload.";
   public static final String ERR_INVALID_PART_NUMBER =
-      "Invalid part number: %d. " + String.format("Part number must be an integer between %d and %d.", MIN_PART_NUM, MAX_PART_NUM);
+      "Invalid part number: %s. " + String.format("Part number must be an integer between %d and %d.", MIN_PART_NUM, MAX_PART_NUM);
   public static final String ERR_DUPLICATE_PART_NUMBER = "Duplicate part number found: %d.";
   public static final String ERR_DUPLICATE_ETAG = "Duplicate eTag found: %s.";
   public static final String ERR_EMPTY_REQUEST_BODY = "Xml request body cannot be empty.";

diff --git a/...frontend/src/main/java/com/github/ambry/frontend/s3/S3MultipartCompleteUploadHandler.java b/...frontend/src/main/java/com/github/ambry/frontend/s3/S3MultipartCompleteUploadHandler.java
@@ -430,15 +430,7 @@ List<Part> validatePartsOrThrow(CompleteMultipartUpload request) throws RestServ
     Set<Integer> partNumbers = new HashSet<>();
     Set<String> etags = new HashSet<>();
     for (Part part : parts) {
-      int partNumber;
-      try {
-        partNumber = part.getPartNumber();
-      } catch (NumberFormatException e) {
-        String error = String.format(S3Constants.ERR_INVALID_PART_NUMBER, part.getPartNumber());
-        throw new RestServiceException(error, RestServiceErrorCode.BadRequest);
-      } catch (Throwable e) {
-        throw new RestServiceException(S3Constants.ERR_INVALID_MULTIPART_UPLOAD, RestServiceErrorCode.BadRequest);
-      }
+      int partNumber = getPartNumber(part);
       String etag = part.geteTag();
       if (partNumber < S3Constants.MIN_PART_NUM || partNumber > S3Constants.MAX_PART_NUM) {
         String error = String.format(S3Constants.ERR_INVALID_PART_NUMBER, partNumber);
@@ -456,13 +448,26 @@ List<Part> validatePartsOrThrow(CompleteMultipartUpload request) throws RestServ
     return parts;
   }
 
-  List<Part> getParts(CompleteMultipartUpload request) throws RestServiceException {
-    List<Part> parts;
+  int getPartNumber(Part part) throws RestServiceException {
+    int partNumber;
     try {
-      parts = Arrays.asList(request.getPart());
+      partNumber = part.getPartNumber();
+    } catch (NumberFormatException e) {
+      // cannot use getPartNumber() here as it would cause another exception
+      String error = String.format(S3Constants.ERR_INVALID_PART_NUMBER, part);
+      throw new RestServiceException(error, RestServiceErrorCode.BadRequest);
     } catch (Throwable e) {
+      // any other exception is invalid
       throw new RestServiceException(S3Constants.ERR_INVALID_MULTIPART_UPLOAD, RestServiceErrorCode.BadRequest);
     }
+    return partNumber;
+  }
+
+  List<Part> getParts(CompleteMultipartUpload request) throws RestServiceException {
+    if (request.getPart() == null) {
+      throw new RestServiceException(S3Constants.ERR_EMPTY_REQUEST_BODY, RestServiceErrorCode.BadRequest);
+    }
+    List<Part> parts = Arrays.asList(request.getPart());
     if (parts.isEmpty()) {
       throw new RestServiceException(S3Constants.ERR_EMPTY_REQUEST_BODY, RestServiceErrorCode.BadRequest);
     }

diff --git a/ambry-frontend/src/test/java/com/github/ambry/frontend/S3MultipartUploadTest.java b/ambry-frontend/src/test/java/com/github/ambry/frontend/S3MultipartUploadTest.java
@@ -294,7 +294,7 @@ public void testExceedMaxParts() throws Exception {
   @Test
   public void testEmptyPartList() throws Exception {
     Part[] parts = {};
-    String expectedMessage = S3Constants.ERR_INVALID_MULTIPART_UPLOAD;
+    String expectedMessage = S3Constants.ERR_EMPTY_REQUEST_BODY;
     testMultipartUploadWithInvalidParts(parts, expectedMessage);
   }