Skip to content

Improve hfile_s3.c error handling #2036

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
whitwham merged 1 commit into from
Jun 17, 2026
Merged

Improve hfile_s3.c error handling #2036

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
161 changes: 147 additions & 14 deletions hfile_s3.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -984,7 +984,7 @@ static int update_time(s3_auth_data *ad, time_t now) {
}

if (strftime(ad->date_short, 9, "%Y%m%d", tm) != 8) {
return -1;;
return -1;
}

ad->date_html.l = 0;
Expand Down Expand Up @@ -1242,6 +1242,9 @@ static int report_s3_error(kstring_t *body, long resp_code) {

static int http_status_errno(int status)
{
if (hts_verbose >= HTS_LOG_INFO)
fprintf(stderr, "hfile_s3: setting errno from HTTP status code %d\n", status);

if (status >= 500)
switch (status) {
case 501: return ENOSYS;
Expand All @@ -1266,6 +1269,88 @@ static int http_status_errno(int status)
}


static int easy_errno(CURL *curl, CURLcode err)
{
long lval;

if (hts_verbose >= HTS_LOG_INFO)
fprintf(stderr, "hfile_s3: setting errno from libcurl error code %d (%s)\n",
(int) err, curl_easy_strerror(err));

switch (err) {
case CURLE_OK:
return 0;

case CURLE_UNSUPPORTED_PROTOCOL:
case CURLE_URL_MALFORMAT:
return EINVAL;

#if LIBCURL_VERSION_NUM >= 0x071505
case CURLE_NOT_BUILT_IN:
return ENOSYS;
#endif

case CURLE_COULDNT_RESOLVE_PROXY:
case CURLE_COULDNT_RESOLVE_HOST:
case CURLE_FTP_CANT_GET_HOST:
return EDESTADDRREQ; // Lookup failure

case CURLE_COULDNT_CONNECT:
case CURLE_SEND_ERROR:
case CURLE_RECV_ERROR:
if (curl_easy_getinfo(curl, CURLINFO_OS_ERRNO, &lval) == CURLE_OK)
return lval;
else
return ECONNABORTED;

case CURLE_REMOTE_ACCESS_DENIED:
case CURLE_LOGIN_DENIED:
case CURLE_TFTP_PERM:
return EACCES;

case CURLE_PARTIAL_FILE:
return EPIPE;

case CURLE_HTTP_RETURNED_ERROR:
if (curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &lval) == CURLE_OK)
return http_status_errno(lval);
else
return EIO;

case CURLE_OUT_OF_MEMORY:
return ENOMEM;

case CURLE_OPERATION_TIMEDOUT:
return ETIMEDOUT;

case CURLE_RANGE_ERROR:
return ESPIPE;

case CURLE_SSL_CONNECT_ERROR:
return ECONNABORTED;

case CURLE_FILE_COULDNT_READ_FILE:
case CURLE_TFTP_NOTFOUND:
return ENOENT;

case CURLE_TOO_MANY_REDIRECTS:
return ELOOP;

case CURLE_FILESIZE_EXCEEDED:
return EFBIG;

case CURLE_REMOTE_DISK_FULL:
return ENOSPC;

case CURLE_REMOTE_FILE_EXISTS:
return EEXIST;

default:
return EIO;
}
}


static void initialise_local(hFILE_s3 *fp) {
ks_initialize(&fp->buffer);
ks_initialize(&fp->url);
Expand Down Expand Up @@ -1319,6 +1404,7 @@ static int add_header(struct curl_slist **head, char *value) {
static struct curl_slist *set_html_headers(hFILE_s3 *fp, kstring_t *auth, kstring_t *date,
kstring_t *content, kstring_t *token, kstring_t *range) {
struct curl_slist *headers = NULL;
CURLcode cret;
int err = 0;

/* The next two lines have the effect of preventing curl from
Expand Down Expand Up @@ -1349,7 +1435,12 @@ static struct curl_slist *set_html_headers(hFILE_s3 *fp, kstring_t *auth, kstrin
if ((err = add_header(&headers, token->s)))
goto error;

curl_easy_setopt(fp->curl, CURLOPT_HTTPHEADER, headers);
cret = curl_easy_setopt(fp->curl, CURLOPT_HTTPHEADER, headers);
if (cret != CURLE_OK) {
err = 1;
errno = easy_errno(fp->curl, cret);
goto error;
}

error:

Expand Down Expand Up @@ -1450,8 +1541,10 @@ static int abort_upload(hFILE_s3 *fp) {
err |= curl_easy_setopt(fp->curl, CURLOPT_URL, url.s);
err |= curl_easy_setopt(fp->curl, CURLOPT_VERBOSE, fp->verbose);

if (err != CURLE_OK)
if (err != CURLE_OK) {
errno = EINVAL;
goto out;
}

headers = set_html_headers(fp, &fp->authorisation, &fp->date, &fp->content, &fp->token, NULL);

Expand All @@ -1462,6 +1555,8 @@ static int abort_upload(hFILE_s3 *fp) {

if (fp->ret == CURLE_OK) {
ret = 0;
} else {
errno = easy_errno(fp->curl, fp->ret);
}

out:
Expand Down Expand Up @@ -1520,8 +1615,10 @@ static int complete_upload(hFILE_s3 *fp, kstring_t *resp) {
err |= curl_easy_setopt(fp->curl, CURLOPT_USERAGENT, curl.useragent.s);
err |= curl_easy_setopt(fp->curl, CURLOPT_VERBOSE, fp->verbose);

if (err != CURLE_OK)
if (err != CURLE_OK) {
errno = EINVAL;
goto out;
}

headers = set_html_headers(fp, &fp->authorisation, &fp->date, &fp->content, &fp->token, NULL);

Expand All @@ -1532,6 +1629,8 @@ static int complete_upload(hFILE_s3 *fp, kstring_t *resp) {

if (fp->ret == CURLE_OK) {
ret = 0;
} else {
errno = easy_errno(fp->curl, fp->ret);
}

out:
Expand Down Expand Up @@ -1600,8 +1699,10 @@ static int upload_part(hFILE_s3 *fp, kstring_t *resp) {
err |= curl_easy_setopt(fp->curl, CURLOPT_USERAGENT, curl.useragent.s);
err |= curl_easy_setopt(fp->curl, CURLOPT_VERBOSE, fp->verbose);

if (err != CURLE_OK)
if (err != CURLE_OK) {
errno = EINVAL;
goto out;
}

headers = set_html_headers(fp, &fp->authorisation, &fp->date, &fp->content, &fp->token, NULL);

Expand All @@ -1612,6 +1713,8 @@ static int upload_part(hFILE_s3 *fp, kstring_t *resp) {

if (fp->ret == CURLE_OK) {
ret = 0;
} else {
errno = easy_errno(fp->curl, fp->ret);
}

out:
Expand Down Expand Up @@ -1645,7 +1748,10 @@ static ssize_t s3_write(hFILE *fpv, const void *bufferv, size_t nbytes) {

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

if (cret != CURLE_OK || response_code > 200) {
if (cret != CURLE_OK) {

@jmarshall jmarshall Jun 16, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the unlikely event curl_easy_getinfo() fails, response_code will usually not have been written to. So separating these cases avoids reading an uninitialised variable in http_status_errno(response_code) below.

errno = easy_errno(fp->curl, cret);
ret = -1;
} else if (response_code > 200) {
errno = http_status_errno(response_code);
ret = -1;
} else {
Expand Down Expand Up @@ -1699,7 +1805,10 @@ static int s3_write_close(hFILE *fpv) {

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

if (cret != CURLE_OK || response_code > 200) {
if (cret != CURLE_OK) {
errno = easy_errno(fp->curl, cret);
ret = -1;
} else if (response_code > 200) {
errno = http_status_errno(response_code);
ret = -1;
} else {
Expand Down Expand Up @@ -1740,6 +1849,8 @@ static int s3_write_close(hFILE *fpv) {
}

errno = http_status_errno(response_code);
} else {
errno = easy_errno(fp->curl, cret);
}
}
}
Expand Down Expand Up @@ -1813,8 +1924,10 @@ static int initialise_upload(hFILE_s3 *fp, kstring_t *head, kstring_t *resp, int
err |= curl_easy_setopt(fp->curl, CURLOPT_USERAGENT, curl.useragent.s);
err |= curl_easy_setopt(fp->curl, CURLOPT_VERBOSE, fp->verbose);

if (err != CURLE_OK)
if (err != CURLE_OK) {
errno = EINVAL;
goto out;
}

headers = set_html_headers(fp, &fp->authorisation, &fp->date, &fp->content, &fp->token, NULL);

Expand All @@ -1825,6 +1938,8 @@ static int initialise_upload(hFILE_s3 *fp, kstring_t *head, kstring_t *resp, int

if (fp->ret == CURLE_OK) {
ret = 0;
} else {
errno = easy_errno(fp->curl, fp->ret);
}

out:
Expand Down Expand Up @@ -1925,8 +2040,10 @@ static int get_part(hFILE_s3 *fp, kstring_t *resp) {
err |= curl_easy_setopt(fp->curl, CURLOPT_HEADERDATA, (void *)resp);
}

if (err != CURLE_OK)
if (err != CURLE_OK) {
errno = EINVAL;
goto out;
}

headers = set_html_headers(fp, &fp->authorisation, &fp->date, &fp->content, &fp->token, &fp->range);

Expand All @@ -1937,6 +2054,8 @@ static int get_part(hFILE_s3 *fp, kstring_t *resp) {

if (fp->ret == CURLE_OK) {
ret = 0;
} else {
errno = easy_errno(fp->curl, fp->ret);
}

out:
Expand Down Expand Up @@ -1988,7 +2107,10 @@ static ssize_t s3_read(hFILE *fpv, void *bufferv, size_t nbytes) {
long response_code;
CURLcode cret = curl_easy_getinfo(fp->curl, CURLINFO_RESPONSE_CODE, &response_code);

if (cret != CURLE_OK || response_code > 300) {
if (cret != CURLE_OK) {
errno = easy_errno(fp->curl, cret);
ret = -1;
} else if (response_code > 300) {
errno = http_status_errno(response_code);
ret = -1;
}
Expand Down Expand Up @@ -2109,7 +2231,7 @@ static hFILE *s3_write_open(const char *url, s3_auth_data *auth) {
const char *env;
CURLcode cret;
long response_code;

int save_errno;

fp = (hFILE_s3 *)hfile_init(sizeof(hFILE_s3), "w", 0);

Expand Down Expand Up @@ -2149,7 +2271,7 @@ static hFILE *s3_write_open(const char *url, s3_auth_data *auth) {
kputs(url, &fp->url);

if ((query_start = strchr(fp->url.s, '?'))) {
has_user_query = 1;;
has_user_query = 1;
}

if (initialise_upload(fp, &header, &response, has_user_query))
Expand Down Expand Up @@ -2180,6 +2302,7 @@ static hFILE *s3_write_open(const char *url, s3_auth_data *auth) {
cret = curl_easy_getinfo(fp->curl, CURLINFO_RESPONSE_CODE, &response_code);
} else {
// unable to get a response code from curl
errno = easy_errno(fp->curl, cret);
goto error;
}

Expand All @@ -2194,6 +2317,8 @@ static hFILE *s3_write_open(const char *url, s3_auth_data *auth) {
}

errno = http_status_errno(response_code);
} else {
errno = easy_errno(fp->curl, cret);
}

goto error;
Expand All @@ -2219,11 +2344,13 @@ static hFILE *s3_write_open(const char *url, s3_auth_data *auth) {
return &fp->base;

error:
save_errno = errno;
ks_free(&response);
ks_free(&header);
cleanup_local(fp);
free_authorisation_values(fp);
hfile_destroy((hFILE *)fp);
errno = save_errno;
return NULL;
}

Expand All @@ -2235,6 +2362,7 @@ static hFILE *s3_read_open(const char *url, s3_auth_data *auth) {
kstring_t file_range = {0, 0, NULL};
CURLcode cret;
long response_code = 0;
int save_errno;

fp = (hFILE_s3 *)hfile_init(sizeof(hFILE_s3), "r", 0);

Expand Down Expand Up @@ -2295,6 +2423,7 @@ static hFILE *s3_read_open(const char *url, s3_auth_data *auth) {
cret = curl_easy_getinfo(fp->curl, CURLINFO_RESPONSE_CODE, &response_code);
} else {
// unable to get a response code from curl
errno = easy_errno(fp->curl, cret);
goto error;
}

Expand All @@ -2309,6 +2438,8 @@ static hFILE *s3_read_open(const char *url, s3_auth_data *auth) {
}

errno = http_status_errno(response_code);
} else {
errno = easy_errno(fp->curl, cret);
}

goto error;
Expand All @@ -2335,13 +2466,14 @@ static hFILE *s3_read_open(const char *url, s3_auth_data *auth) {
ks_free(&file_range);
return &fp->base;


error:
save_errno = errno;
ks_free(&response);
ks_free(&file_range);
cleanup_local(fp);
free_authorisation_values(fp);
hfile_destroy((hFILE *)fp);
errno = save_errno;
return NULL;
}

Expand Down Expand Up @@ -2461,7 +2593,8 @@ int PLUGIN_GLOBAL(hfile_plugin_init,_s3)(struct hFILE_plugin *self) {
err = curl_global_init(CURL_GLOBAL_ALL);

if (err != CURLE_OK) {
// look at putting in an errno here
// Set a suitably catastrophic error code
errno = ENETDOWN;
Comment on lines +2596 to +2597

@jmarshall jmarshall Jun 16, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curl_global_init() always returns CURLE_FAILED_INIT on error, so simply translate that never-happens into a suitably catastrophic errno value.

return -1;
}

Expand Down
Loading