elek commented on a change in pull request #2108:
URL: https://github.com/apache/ozone/pull/2108#discussion_r616618407



##########
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadCompleteRequest.java
##########
@@ -285,7 +285,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager 
ozoneManager,
           // But now as versioning is not supported, just following the commit
           // key approach. When versioning support comes, then we can uncomment
           // below code keyInfo.addNewVersion(locations);
-          omKeyInfo.updateLocationInfoList(partLocationInfos, true);
+          omKeyInfo.updateLocationInfoList(partLocationInfos, true, true);

Review comment:
       I had the same question / doubts and tried to understand the code (and 
it LGTM):
   
   Based on my understanding in case of MPU the keys are created during the 
`S3MultipartUploadCommit` call. This call creates an entry in the 
`MultiPartInfoTable` which includes the key location information. 
   
   During the complete phase these (already validated !) key location 
information are moved from the `MultipartInfo` entries to a final key entry.
   
   The `MultipartInfo` entries seem to be properly validated in 
`S3MultipartUploadCommitPartRequest` L:160:
   
   ```
    omKeyInfo.updateLocationInfoList(keyArgs.getKeyLocationsList().stream()
             .map(OmKeyLocationInfo::getFromProtobuf)
             .collect(Collectors.toList()), true); 
   ```
   
   This is the default version (with validation):
   
   ```
     */
     public void updateLocationInfoList(List<OmKeyLocationInfo> 
locationInfoList,
         boolean isMpu) {
       updateLocationInfoList(locationInfoList, isMpu, false);
     }
   ```
   
   --> Based on my understanding everything is perfect with this patch. As far 
as the good part infos are used, the key information is already validated. (and 
the part information is guarded by the randomness of the 
uploadId:`UUID.randomUUID().toString() + "-" + UniqueId.next()` which is known 
only by the client)




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

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to