HDDS-15809. Validate x-amz-copy-source-range on UploadPartCopy#10711
Open
rich7420 wants to merge 1 commit into
Open
HDDS-15809. Validate x-amz-copy-source-range on UploadPartCopy#10711rich7420 wants to merge 1 commit into
rich7420 wants to merge 1 commit into
Conversation
The UploadPartCopy path read x-amz-copy-source-range but never validated it against the source object: the range was parsed with a hard-coded length of 0 and the result was never checked, so an out-of-range range was silently clamped (surfacing as InvalidRequest) and a malformed range fell through to reading the whole object (no error at all). Validate the header in ObjectEndpoint.createMultipartKey: reject a value that is not a single numeric byte range (bytes=<start>-<end>) with InvalidArgument, and reject a range whose start > end or whose end is beyond the source size with InvalidRange. Add an AWS SDK v2 integration test covering both cases.
adoroszlai
reviewed
Jul 10, 2026
adoroszlai
left a comment
adoroszlai
left a comment
Contributor
There was a problem hiding this comment.
Thanks @rich7420 for the patch.
Comment on lines
+127
to
+128
| private static final Pattern COPY_SOURCE_RANGE_PATTERN = | ||
| Pattern.compile("^bytes=(\\d+)-(\\d+)$"); |
Contributor
There was a problem hiding this comment.
Duplicate?
Comment on lines
+899
to
+909
| Matcher matcher = COPY_SOURCE_RANGE_PATTERN.matcher(range); | ||
| if (!matcher.matches()) { | ||
| throw newError(S3ErrorTable.INVALID_ARGUMENT, range); | ||
| } | ||
| long startOffset = Long.parseLong(matcher.group(1)); | ||
| long endOffset = Long.parseLong(matcher.group(2)); | ||
| long sourceSize = sourceKeyDetails.getDataSize(); | ||
| if (startOffset > endOffset || endOffset >= sourceSize) { | ||
| throw newError(S3ErrorTable.INVALID_RANGE, range); | ||
| } | ||
| rangeHeader = new RangeHeader(startOffset, endOffset, false, false); |
Contributor
There was a problem hiding this comment.
Can RangeHeaderParserUtil can be tweaked to make the range mandatory in this case?
Comment on lines
+1425
to
+1431
| UploadPartCopyRequest outOfRangeRequest = UploadPartCopyRequest.builder() | ||
| .sourceBucket(sourceBucketName) | ||
| .sourceKey(sourceKey) | ||
| .destinationBucket(destBucketName) | ||
| .destinationKey(destKey) | ||
| .uploadId(uploadId) | ||
| .partNumber(1) |
Contributor
There was a problem hiding this comment.
Can we store Builder instance and use it to build all requests, where only copySourceRange is different?
| .build(); | ||
| S3Exception outOfRange = assertThrows(S3Exception.class, () -> s3Client.uploadPartCopy(outOfRangeRequest)); | ||
| // InvalidRange maps to HTTP 416; AWS also permits 400 for this case. | ||
| assertTrue(outOfRange.statusCode() == 400 || outOfRange.statusCode() == 416, |
Contributor
There was a problem hiding this comment.
nit: please use assertThat instead of assertTrue (HDDS-9951), and use HttpStatus constants.
assertThat(outOfRange.statusCode()).isIn(SC_BAD_REQUEST, SC_REQUESTED_RANGE_NOT_SATISFIABLE);
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
UploadPartCopyread thex-amz-copy-source-rangeheader but never validated itagainst the source object. In
ObjectEndpoint.createMultipartKeythe range wasparsed with a hard-coded length of
0and the result was never checked, so:InvalidRequest, and"read the whole object", raising no error at all.
This aligns
UploadPartCopywith AWS S3 by validating the header:bytes=<start>-<end>), returnInvalidArgument(HTTP 400). This rejects values such as0-2(missingbytes=),bytes=0(no end),bytes=hello-world,bytes=0-bar,bytes=hello-,and
bytes=0-2,3-5(multiple ranges).start > endorendis beyond the source object size, returnInvalidRange(HTTP 416; AWS also permits 400). Example: a 5-byte source withbytes=0-21.end - start + 1and theexisting copy path is used.
Both
InvalidArgumentandInvalidRangealready exist inS3ErrorTable, so no newerror codes or wire changes are needed. The change is confined to the UploadPartCopy
branch;
CopyObject(without a part) and normalGetObject/PutObjectpaths areuntouched, and the shared
RangeHeaderParserUtil(used byGetObject) is notmodified.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-15809
How was this patch tested?
https://github.com/rich7420/ozone/actions/runs/29078906089