Skip to content

Fixes for S3 uploading bugs. #1984

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
daviesrob merged 3 commits into from
Feb 5, 2026
+81 −52
Merged

Fixes for S3 uploading bugs. #1984

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fbcc174
Fixes two bugs in uploading.
whitwham Jan 22, 2026
a1241e9
Removed errant debug message.
whitwham Feb 2, 2026
64a37a0
A few changes on review.
whitwham Feb 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 81 additions & 52 deletions hfile_s3.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,8 @@ static int redirect_endpoint(hFILE_s3 *fp, kstring_t *header) {
}
}

if (hts_verbose >= HTS_LOG_INFO) fprintf(stderr, "hfile_s3: redirect_endpoint: return %d\n", ret);

return ret;
}

Expand Down Expand Up @@ -1120,9 +1122,9 @@ static int set_region(s3_auth_data *ad, kstring_t *region) {
// Some common code

#define S3_MOVED_PERMANENTLY 301
#define S3_TEMPORARY_REDIRECT 307
#define S3_BAD_REQUEST 400


static struct {
kstring_t useragent;
CURLSH *share;
Expand Down Expand Up @@ -1178,7 +1180,7 @@ static char *stristr(char *haystack, char *needle) {
char *h = haystack;
char *n = needle;

while (toupper(*h) == toupper(*n)) {
while (toupper_c(*h) == toupper_c(*n)) {
h++, n++;
if (!*h || !*n) break;
}
Expand Down Expand Up @@ -1319,6 +1321,15 @@ static struct curl_slist *set_html_headers(hFILE_s3 *fp, kstring_t *auth, kstrin
struct curl_slist *headers = NULL;
int err = 0;

/* The next two lines have the effect of preventing curl from
adding these headers. If they exist it can lead to conflicts
in the signature calculations (not present in all S3 systems).
*/
err = add_header(&headers, "Content-Type:");
err |= add_header(&headers, "Expect:");

if (err) goto error;

if (auth->l)
if ((err = add_header(&headers, auth->s)))
goto error;
Expand Down Expand Up @@ -1407,11 +1418,13 @@ uploads and abandon the upload process.
static int abort_upload(hFILE_s3 *fp) {
kstring_t url = KS_INITIALIZE;
kstring_t canonical_query_string = KS_INITIALIZE;
int ret = -1;
int ret = -1, save_errno;
struct curl_slist *headers = NULL;
char http_request[] = "DELETE";
CURLcode err;

save_errno = errno; // keep the errno that caused the need to abort

clear_authorisation_values(fp);

if (ksprintf(&canonical_query_string, "uploadId=%s", fp->upload_id.s) < 0) {
Expand Down Expand Up @@ -1459,6 +1472,7 @@ static int abort_upload(hFILE_s3 *fp) {
fp->aborted = 1;
cleanup(fp);

errno = save_errno;
return ret;
}

Expand Down Expand Up @@ -1497,6 +1511,7 @@ static int complete_upload(hFILE_s3 *fp, kstring_t *resp) {
curl_easy_reset(fp->curl);

err = curl_easy_setopt(fp->curl, CURLOPT_POST, 1L);

err |= curl_easy_setopt(fp->curl, CURLOPT_POSTFIELDS, fp->completion_message.s);
err |= curl_easy_setopt(fp->curl, CURLOPT_POSTFIELDSIZE, (long) fp->completion_message.l);
err |= curl_easy_setopt(fp->curl, CURLOPT_WRITEFUNCTION, response_callback);
Expand Down Expand Up @@ -1670,6 +1685,7 @@ static int s3_write_close(hFILE *fpv) {
kstring_t response = {0, 0, NULL};
int ret = 0;
CURLcode cret;
long response_code;

if (!fp->aborted) {

Expand All @@ -1679,7 +1695,6 @@ static int s3_write_close(hFILE *fpv) {
ret = upload_part(fp, &response);

if (!ret) {
long response_code;
kstring_t etag = {0, 0, NULL};

cret = curl_easy_getinfo(fp->curl, CURLINFO_RESPONSE_CODE, &response_code);
Expand Down Expand Up @@ -1715,6 +1730,17 @@ static int s3_write_close(hFILE *fpv) {
if (!ret) {
if (strstr(response.s, "CompleteMultipartUploadResult") == NULL) {
ret = -1;
cret = curl_easy_getinfo(fp->curl, CURLINFO_RESPONSE_CODE, &response_code);

if (cret == CURLE_OK) {
if (hts_verbose >= HTS_LOG_INFO) {
if (report_s3_error(&response, response_code)) {
fprintf(stderr, "hfile_s3: warning, unable to report full S3 error status.\n");
}
}

errno = http_status_errno(response_code);
}
}
}
} else {
Expand Down Expand Up @@ -1746,6 +1772,8 @@ static int handle_bad_request(hFILE_s3 *fp, kstring_t *resp) {

ks_free(&region);

if (hts_verbose >= HTS_LOG_INFO) fprintf(stderr, "hfile_s3: handle_bad_request: return %d\n", ret);

return ret;
}

Expand Down Expand Up @@ -1852,14 +1880,13 @@ static int get_part(hFILE_s3 *fp, kstring_t *resp) {
struct curl_slist *headers = NULL;
int ret = -1;
char http_request[] = "GET";
char canonical_query_string = 0;
CURLcode err;

ks_clear(&fp->buffer); // reset storage buffer
clear_authorisation_values(fp);

if (fp->au->is_v4) {
if (v4_authorisation(fp, http_request, NULL, &canonical_query_string, 0) != 0) {
if (v4_authorisation(fp, http_request, NULL, "", 0) != 0) {
goto out;
}

Expand Down Expand Up @@ -2077,7 +2104,7 @@ static hFILE *s3_write_open(const char *url, s3_auth_data *auth) {
hFILE_s3 *fp;
kstring_t response = {0, 0, NULL};
kstring_t header = {0, 0, NULL};
int ret, has_user_query = 0;
int has_user_query = 0;
char *query_start;
const char *env;
CURLcode cret;
Expand Down Expand Up @@ -2125,30 +2152,35 @@ static hFILE *s3_write_open(const char *url, s3_auth_data *auth) {
has_user_query = 1;;
}

ret = initialise_upload(fp, &header, &response, has_user_query);
if (initialise_upload(fp, &header, &response, has_user_query))
goto error;

cret = curl_easy_getinfo(fp->curl, CURLINFO_RESPONSE_CODE, &response_code);

if (ret == 0) {
if (cret == CURLE_OK) {
if (response_code == S3_MOVED_PERMANENTLY) {
if (redirect_endpoint(fp, &header) == 0) {
ks_clear(&response);
ks_clear(&header);
if (cret == CURLE_OK) {
if (response_code == S3_MOVED_PERMANENTLY || response_code == S3_TEMPORARY_REDIRECT) {
if (redirect_endpoint(fp, &header) == 0) {
ks_clear(&response);
ks_clear(&header);

ret = initialise_upload(fp, &header, &response, has_user_query);
}
} else if (response_code == S3_BAD_REQUEST) {
if (handle_bad_request(fp, &response) == 0) {
ks_clear(&response);
ks_clear(&header);
if (initialise_upload(fp, &header, &response, has_user_query))
goto error;
}
} else if (response_code == S3_BAD_REQUEST) {
if (handle_bad_request(fp, &response) == 0) {
ks_clear(&response);
ks_clear(&header);

ret = initialise_upload(fp, &header, &response, has_user_query);
}
if (initialise_upload(fp, &header, &response, has_user_query))
goto error;
}
} else {
// unable to get a response code from curl
ret = -1;
}

// reget the response code (may not have changed)
cret = curl_easy_getinfo(fp->curl, CURLINFO_RESPONSE_CODE, &response_code);
} else {
// unable to get a response code from curl
goto error;
}

if (response_code >= 300) {
Expand All @@ -2164,11 +2196,9 @@ static hFILE *s3_write_open(const char *url, s3_auth_data *auth) {
errno = http_status_errno(response_code);
}

ret = -1;
goto error;
}

if (ret) goto error;

if (get_upload_id(fp, &response)) goto error;

// start the completion message (a formatted list of parts)
Expand Down Expand Up @@ -2203,7 +2233,6 @@ static hFILE *s3_read_open(const char *url, s3_auth_data *auth) {
const char *env;
kstring_t response = {0, 0, NULL};
kstring_t file_range = {0, 0, NULL};
int ret;
CURLcode cret;
long response_code = 0;

Expand Down Expand Up @@ -2240,31 +2269,33 @@ static hFILE *s3_read_open(const char *url, s3_auth_data *auth) {

kputs(url, &fp->url);

ret = initialise_download(fp, &response);
cret = curl_easy_getinfo(fp->curl, CURLINFO_RESPONSE_CODE, &response_code);
if (initialise_download(fp, &response))
goto error;

if (ret == 0) {
if (cret == CURLE_OK) {
if (response_code == S3_MOVED_PERMANENTLY) {
ks_clear(&response);
cret = curl_easy_getinfo(fp->curl, CURLINFO_RESPONSE_CODE, &response_code);

if (redirect_endpoint(fp, &response) == 0) {
ret = initialise_download(fp, &response);
}
} else if (response_code == S3_BAD_REQUEST) {
ks_clear(&response);
if (cret == CURLE_OK) {
if (response_code == S3_MOVED_PERMANENTLY || response_code == S3_TEMPORARY_REDIRECT) {
ks_clear(&response);

if (handle_bad_request(fp, &fp->buffer) == 0) {
ret = initialise_download(fp, &response);
}
if (redirect_endpoint(fp, &response) == 0) {
if (initialise_download(fp, &response))
goto error;
}
} else if (response_code == S3_BAD_REQUEST) {
ks_clear(&response);

// reget the response code (may not have changed)
cret = curl_easy_getinfo(fp->curl, CURLINFO_RESPONSE_CODE, &response_code);
} else {
// unable to get a response code from curl
ret = -1;
if (handle_bad_request(fp, &fp->buffer) == 0) {
if (initialise_download(fp, &response))
goto error;
}
}

// reget the response code (may not have changed)
cret = curl_easy_getinfo(fp->curl, CURLINFO_RESPONSE_CODE, &response_code);
} else {
// unable to get a response code from curl
goto error;
}

if (response_code >= 300) {
Expand All @@ -2280,11 +2311,9 @@ static hFILE *s3_read_open(const char *url, s3_auth_data *auth) {
errno = http_status_errno(response_code);
}

ret = -1;
goto error;
}

if (ret) goto error;

if (get_entry(response.s, "content-range: bytes ", "\n", &file_range) == EOF) {
fprintf(stderr, "hfile_s3: warning: failed to read file size.\n");
fp->file_size = -1;
Expand Down Expand Up @@ -2373,7 +2402,7 @@ static hFILE *hopen_s3(const char *url, const char *mode)
{
hFILE *fp;

if (getenv("HTS_S3_V2") == NULL) { // Force the v2 signature code
if (getenv("HTS_S3_V2") == NULL) {
fp = s3_open_v4(url, mode, NULL);
} else {
fp = s3_open_v2(url, mode, NULL);
Expand Down