Allowing S3 virtual paths and S3 aliases as data source #1756
Conversation
This file contains 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
Currently only s3 pathing works with a redirect; With this fix s3 virtual gets a virtual redirect url; additionally, performs an additional redirect if necessary.
daviesrob
reviewed
Mar 4, 2024
hfile_s3.c
Outdated
| @@ -1298,6 +1311,20 @@ static hFILE *s3_open_v4(const char *s3url, const char *mode, va_list *argsp) { | |||
|
|
|||
| if (fp == NULL) goto error; | |||
|
|
|||
| if (http_response >= 300 && http_response < 400) { | |||
There was a problem hiding this comment.
I think doing this for all 300 codes might be a bit over-tolerant. It would be safer to check for the specific codes we expect for this sort of redirection.
Also, you need an hclose_abruptly(fp); here to avoid leakage of the old connection.
There was a problem hiding this comment.
Hi. Thank you for the review!
I specified in the latest commit the http_response to just check for 307. Also, I (hopefully) fixed the leakage.
|
Thanks, I can confirm that this works in my tests. |
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.
Related issue: samtools/samtools#1984
The two main changes are:
s3_open_v4an additionalhopenis called if a redirect (status code between 300 and 399) is detected.redirect_endpoint_callbackthe url is parsed and a new one constructed respecting the virtual path style in Amazon S3 links.Some minor changes:
s3_auth_datato allow the use inredirect_endpoint_callback{0, 0, NULL}byKS_INITIALIZEfor various variables.