Copilot commented on code in PR #10670:
URL: https://github.com/apache/ozone/pull/10670#discussion_r3557509056
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -640,12 +641,26 @@ private OmKeyInfo readKeyInfo(OmKeyArgs args,
BucketLayout bucketLayout)
.filter(it -> it.getPartNumber() == partNumberParam)
.collect(Collectors.toList());
+ // A requested part number that has no blocks does not exist in this
+ // object (part numbers may be non-contiguous), so it is out of range.
+ if (currentLocations.isEmpty()) {
+ throw new OMException("Cannot read part " + partNumberParam
+ + " of key " + keyName + " because it does not exist",
+ INVALID_PART);
+ }
Review Comment:
Negative `partNumber` values currently bypass validation because the guard
uses `partNumberParam > 0`. If a client sends `partNumber=-1`, the read path
skips all part filtering/validation and returns whole-object metadata/content
instead of failing with `INVALID_PART`.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -640,12 +641,26 @@ private OmKeyInfo readKeyInfo(OmKeyArgs args,
BucketLayout bucketLayout)
.filter(it -> it.getPartNumber() == partNumberParam)
.collect(Collectors.toList());
+ // A requested part number that has no blocks does not exist in this
+ // object (part numbers may be non-contiguous), so it is out of range.
+ if (currentLocations.isEmpty()) {
+ throw new OMException("Cannot read part " + partNumberParam
+ + " of key " + keyName + " because it does not exist",
+ INVALID_PART);
+ }
+
value.updateLocationInfoList(currentLocations, true, true);
value.setDataSize(currentLocations.stream()
.mapToLong(BlockLocationInfo::getLength)
.sum());
+ } else if (partNumberParam > 1) {
+ // Non-multipart key: only part number 1 (the whole object) is valid;
+ // any higher part number is out of range.
+ throw new OMException("Cannot read part " + partNumberParam
+ + " of non-multipart key " + keyName, INVALID_PART);
Review Comment:
For non-multipart keys, the out-of-range check only rejects `partNumberParam
> 1`. If the `partNumber` is negative (eg `-1`), it will fall through as
“valid” once the outer guard is relaxed to validate negatives. Non-multipart
keys should accept only `partNumber == 1` and reject any other non-zero value.
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/ClientProtocolStub.java:
##########
@@ -120,6 +120,13 @@ public OzoneKey headS3Object(String bucketName, String
keyName)
.headObject(keyName);
}
+ @Override
+ public OzoneKey headS3Object(String bucketName, String keyName,
+ int partNumber) throws IOException {
+ return objectStoreStub.getS3Volume().getBucket(bucketName)
+ .headObject(keyName);
+ }
Review Comment:
The new `headS3Object(bucketName, keyName, partNumber)` overload ignores
`partNumber` and always returns whole-object metadata. This can hide
regressions in s3gateway tests because requests with an invalid part will
appear to succeed. Consider failing fast (or implementing part-aware behavior)
so tests don’t get false positives.
--
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]
