Skip to content

Replace typed range/ifMatch with generic header map on PresignedUrlDownloadRequest#7062

Draft
jencymaryjoseph wants to merge 3 commits into
feature/master/pre-signed-url-getobjectfrom
jencyjos/pre-signedurl/headers-generic-map
Draft

Replace typed range/ifMatch with generic header map on PresignedUrlDownloadRequest#7062
jencymaryjoseph wants to merge 3 commits into
feature/master/pre-signed-url-getobjectfrom
jencyjos/pre-signedurl/headers-generic-map

Conversation

@jencymaryjoseph

Copy link
Copy Markdown
Contributor

Motivation and Context

Signed header-location fields (Range, If-Match, If-None-Match, SSE-C, etc.) are signed into a presigned URL's signature but their values are not stored in the URL — only their names appear in X-Amz-SignedHeaders. The downloader must replay those exact header values or S3 returns a 403 signature mismatch.
This is the same behavior in V1, the PresignedUrlDownloadRequest required the caller to manually set any headers that were signed at URL generation time. If a header was signed but not replayed at download time, S3 rejected the request with SignatureDoesNotMatch. The SDK cannot automatically recover from this because the header values are not stored anywhere in the URL — only the header names are listed in X-Amz-SignedHeaders. The caller must know what values were signed and provide them.
v2 PresignedUrlDownloadRequest only exposed range() and ifMatch() [these were also needed to be set by SDK for the multipart downloads] with no way to supply other signed headers (SSE-C, If-None-Match, etc.).

Modifications

Added putHeader(String, String) / headers(Map<String, List>) after RequestOverrideConfiguration's header idiom in the V2 SDK. We considered extending PresignedUrlDownloadRequest from S3Request or SdkRequest so that overrideConfiguration() would be available, but this was not viable because S3Request.overrideConfiguration() exposes signer, credentials, and endpoint overrides that are incompatible with presigned URL execution (which uses a NoOpSigner and relies solely on the URL's embedded signature). Extending S3Request would pull in sdkFields(), marshalling traits, and the full request pipeline — none of which apply to a presigned URL download.

  • PresignedUrlDownloadRequest: Replaced range(String)/ifMatch(String) with putHeader(String, String), putHeader(String, List), and headers(Map<String, List>). Backed by TreeMap(String.CASE_INSENSITIVE_ORDER) using CollectionUtils.deepUnmodifiableMap.
  • PresignedUrlDownloadRequestWrapper: Carries the generic header map. sdkFields() returns empty; headers applied directly by marshaller.
  • PresignedUrlDownloadRequestMarshaller: Added addCustomHeaders() to put all headers from the map onto the HTTP request.
  • DefaultAsyncPresignedUrlExtension: Passes headers() map to wrapper.
  • PresignedUrlDownloadHelper: Package-private RANGE_HEADER/IF_MATCH_HEADER constants. Range detection and createRangedGetRequest() use the map.
  • Multipart subscribers: Log lines read from the map.
  • S3Presigner: Added "Signed headers note" Javadoc to presignGetObject().
  • GenericS3TransferManager: Updated range check to use headers().containsKey("Range").

Testing

  • PresignedUrlDownloadRequestTest — putHeader, headers(Map), case-insensitive lookup, toBuilder, EqualsVerifier.
  • PresignedUrlDownloadRequestWrapperTest — header map, case-insensitive keys, empty sdkFields, toBuilder.
  • PresignedUrlDownloadHelperTest — createRangedGetRequest uses headers map.
  • PresignedUrlDownloadRequestMarshallerTest — headers added to HTTP request.
  • DefaultAsyncPresignedUrlExtensionTest — end-to-end with putHeader.
  • PresignedUrlSignedHeadersWiremockTest — WireMock tests verifying signed headers are sent in HTTP requests and missing/wrong headers cause 403.
  • MultipartS3AsyncClientTest, AsyncPresignedUrlExtensionTest, PresignedUrlMultipartDownloaderSubscriberWiremockTest — updated to use putHeader API.
  • S3TransferManagerPresignedUrlListenerWiremockTest, S3TransferManagerPresignedUrlDownloadIntegrationTest — updated to use putHeader API.

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist

  • I have read the CONTRIBUTING document
  • Local run of mvn install succeeds
  • My code follows the code style of this project
  • My change requires a change to the Javadoc documentation
  • I have updated the Javadoc documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed
  • I have added a changelog entry. Adding a new entry must be accomplished by running the scripts/new-change script and following the instructions. Commit the new file created by the script in .changes/next-release with your changes.
  • My change is to implement 1.11 parity feature and I have updated LaunchChangelog

License

  • I confirm that this pull request can be released under the Apache 2 license

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant