From 93c4cd2b255f0b4a912e44b48da13f50f4ce2093 Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Tue, 14 Jan 2025 17:47:49 -0800 Subject: [PATCH 01/12] Minor reorg of ros3 initialization code * Better location for HTTP verb allocation * Shove all auth setup into the same if statement * strcpy() instead of memcpy() to copy strings * Other minor renaming and comment cleanup --- src/H5FDros3.c | 48 +++++++++++++++-------------------- src/H5FDs3comms.c | 64 ++++++++++++++++++++++------------------------- 2 files changed, 50 insertions(+), 62 deletions(-) diff --git a/src/H5FDros3.c b/src/H5FDros3.c index 2c0e9c389f3..0c378383650 100644 --- a/src/H5FDros3.c +++ b/src/H5FDros3.c @@ -696,14 +696,10 @@ static H5FD_t * H5FD__ros3_open(const char *url, unsigned flags, hid_t fapl_id, haddr_t maxaddr) { H5FD_ros3_t *file = NULL; - struct tm *now = NULL; - char iso8601now[ISO8601_SIZE]; unsigned char signing_key[SHA256_DIGEST_LENGTH]; s3r_t *handle = NULL; const H5FD_ros3_fapl_t *fa = NULL; H5P_genplist_t *plist = NULL; - htri_t token_exists; - char *token; H5FD_t *ret_value = NULL; FUNC_ENTER_PACKAGE @@ -733,47 +729,43 @@ H5FD__ros3_open(const char *url, unsigned flags, hid_t fapl_id, haddr_t maxaddr) if (NULL == (fa = (const H5FD_ros3_fapl_t *)H5P_peek_driver_info(plist))) HGOTO_ERROR(H5E_VFL, H5E_CANTGET, NULL, "could not get ros3 VFL driver info"); - /* Session/security token */ - if ((token_exists = H5P_exist_plist(plist, ROS3_TOKEN_PROP_NAME)) < 0) - HGOTO_ERROR(H5E_VFL, H5E_CANTGET, NULL, "failed check for property token in plist"); - if (token_exists) { - if (H5P_get(plist, ROS3_TOKEN_PROP_NAME, &token) < 0) - HGOTO_ERROR(H5E_VFL, H5E_CANTGET, NULL, "unable to get token value"); - } - /* Open file; procedure depends on whether or not the fapl instructs to * authenticate requests or not. */ if (fa->authenticate == true) { + htri_t token_exists; /* Does the token exist in the fapl? */ + char *fapl_token = NULL; /* Token from fapl */ + const char *token = NULL; /* const pointer passed to s3r_open */ + char iso8601_now[ISO8601_SIZE]; /* ISO 8601 string */ + /* Compute signing key (part of AWS/S3 REST API). Can be re-used by * user/key for 7 days after creation. - * - * TODO: Find way to reuse/share? */ - now = gmnow(); - assert(now != NULL); - if (ISO8601NOW(iso8601now, now) != (ISO8601_SIZE - 1)) + if (ISO8601NOW(iso8601_now, gmnow()) != (ISO8601_SIZE - 1)) HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, NULL, "problem while writing iso8601 timestamp"); if (H5FD_s3comms_make_aws_signing_key(signing_key, (const char *)fa->secret_key, - (const char *)fa->aws_region, (const char *)iso8601now) < 0) + (const char *)fa->aws_region, (const char *)iso8601_now) < 0) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, NULL, "problem while computing signing key"); - if (token_exists) - handle = H5FD_s3comms_s3r_open(url, (const char *)fa->aws_region, (const char *)fa->secret_id, - (const unsigned char *)signing_key, (const char *)token); + /* Get the token, if it exists */ + if ((token_exists = H5P_exist_plist(plist, ROS3_TOKEN_PROP_NAME)) < 0) + HGOTO_ERROR(H5E_VFL, H5E_CANTGET, NULL, "failed check for property token in plist"); + if (token_exists) { + if (H5P_get(plist, ROS3_TOKEN_PROP_NAME, &fapl_token) < 0) + HGOTO_ERROR(H5E_VFL, H5E_CANTGET, NULL, "unable to get token value"); + token = fapl_token; + } else - handle = H5FD_s3comms_s3r_open(url, (const char *)fa->aws_region, (const char *)fa->secret_id, - (const unsigned char *)signing_key, ""); + token = ""; + + handle = H5FD_s3comms_s3r_open(url, (const char *)fa->aws_region, (const char *)fa->secret_id, + (const unsigned char *)signing_key, token); } else handle = H5FD_s3comms_s3r_open(url, NULL, NULL, NULL, NULL); if (handle == NULL) - /* If we want to check CURL's say on the matter in a controlled - * fashion, this is the place to do it, but would need to make a - * few minor changes to s3comms `s3r_t` and `s3r_read()`. - */ - HGOTO_ERROR(H5E_VFL, H5E_CANTOPENFILE, NULL, "could not open"); + HGOTO_ERROR(H5E_VFL, H5E_CANTOPENFILE, NULL, "s3r_open failed"); /* Create new file struct */ if (NULL == (file = H5FL_CALLOC(H5FD_ros3_t))) diff --git a/src/H5FDs3comms.c b/src/H5FDs3comms.c index 1c06c54758d..7bd7a6d69be 100644 --- a/src/H5FDs3comms.c +++ b/src/H5FDs3comms.c @@ -609,28 +609,25 @@ H5FD_s3comms_s3r_get_filesize(s3r_t *handle) static herr_t H5FD__s3comms_s3r_getsize(s3r_t *handle) { - uintmax_t content_length = 0; - CURL *curlh = NULL; - char *end = NULL; - char *headerresponse = NULL; - struct s3r_datastruct sds = {NULL, 0}; - char *start = NULL; - herr_t ret_value = SUCCEED; + uintmax_t content_length = 0; + CURL *curlh = NULL; + char *end = NULL; + char *header_response = NULL; + struct s3r_datastruct sds = {NULL, 0}; + char *start = NULL; + herr_t ret_value = SUCCEED; FUNC_ENTER_PACKAGE - if (handle == NULL) - HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "handle cannot be NULL"); - if (handle->curlhandle == NULL) - HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "handle->curlhandle cannot be NULL"); - if (handle->httpverb != NULL) - HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "handle->httpverb *MUST* be NULL"); + assert(handle); + assert(handle->curlhandle); + assert(handle->httpverb); /******************** * PREPARE FOR HEAD * ********************/ - /* Set handle and curlhandle to enact an HTTP HEAD request on file */ + /* Set handle and curlhandle to perform an HTTP HEAD request on file */ curlh = handle->curlhandle; if (CURLE_OK != curl_easy_setopt(curlh, CURLOPT_NOBODY, 1L)) @@ -639,13 +636,11 @@ H5FD__s3comms_s3r_getsize(s3r_t *handle) if (CURLE_OK != curl_easy_setopt(curlh, CURLOPT_HEADERDATA, &sds)) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "error while setting CURL option (CURLOPT_HEADERDATA)"); - if (NULL == (handle->httpverb = (char *)H5MM_malloc(sizeof(char) * 16))) - HGOTO_ERROR(H5E_VFL, H5E_CANTALLOC, FAIL, "unable to allocate space for S3 request HTTP verb"); - H5MM_memcpy(handle->httpverb, "HEAD", 5); + strcpy(handle->httpverb, "HEAD"); - if (NULL == (headerresponse = (char *)H5MM_malloc(sizeof(char) * CURL_MAX_HTTP_HEADER))) + if (NULL == (header_response = (char *)H5MM_malloc(sizeof(char) * CURL_MAX_HTTP_HEADER))) HGOTO_ERROR(H5E_VFL, H5E_CANTALLOC, FAIL, "unable to allocate space for curl header response"); - sds.data = headerresponse; + sds.data = header_response; /******************* * PERFORM REQUEST * @@ -671,7 +666,7 @@ H5FD__s3comms_s3r_getsize(s3r_t *handle) * headers, storing file size at handle->filesize. */ - if (NULL == (start = HDstrcasestr(headerresponse, "\r\nContent-Length: "))) + if (NULL == (start = HDstrcasestr(header_response, "\r\nContent-Length: "))) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "could not find \"Content-Length\" in response"); /* move "start" to beginning of value in line; find end of line */ @@ -710,8 +705,9 @@ H5FD__s3comms_s3r_getsize(s3r_t *handle) if (CURLE_OK != curl_easy_setopt(curlh, CURLOPT_HEADERDATA, NULL)) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "error while setting CURL option (CURLOPT_HEADERDATA)"); + strcpy(handle->httpverb, "GET"); done: - H5MM_xfree(headerresponse); + H5MM_xfree(header_response); FUNC_LEAVE_NOAPI(ret_value) } /* H5FD__s3comms_s3r_getsize */ @@ -721,8 +717,6 @@ H5FD__s3comms_s3r_getsize(s3r_t *handle) * * Purpose: Logically open a file hosted on S3 * - * To use default port to connect, port should be 0 - * * To prevent AWS4 authentication, pass NULL to region, * id, and signing_key. * @@ -743,8 +737,10 @@ H5FD_s3comms_s3r_open(const char *url, const char *region, const char *id, const FUNC_ENTER_NOAPI_NOINIT - if (url == NULL || url[0] == '\0') + if (url == NULL) HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, NULL, "url cannot be NULL"); + if (url[0] == '\0') + HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, NULL, "url cannot be an empty string"); /* Parse URL */ @@ -780,8 +776,12 @@ H5FD_s3comms_s3r_open(const char *url, const char *region, const char *id, const /* Create handle and set fields */ if (NULL == (handle = (s3r_t *)H5MM_calloc(sizeof(s3r_t)))) HGOTO_ERROR(H5E_VFL, H5E_CANTALLOC, NULL, "could not allocate space for handle"); + handle->purl = purl; + if (NULL == (handle->httpverb = (char *)H5MM_calloc(sizeof(char) * 16))) + HGOTO_ERROR(H5E_VFL, H5E_CANTALLOC, NULL, "unable to allocate space for S3 request HTTP verb"); + /************************************* * RECORD AUTHENTICATION INFORMATION * *************************************/ @@ -841,20 +841,16 @@ H5FD_s3comms_s3r_open(const char *url, const char *region, const char *id, const handle->curlhandle = curlh; - /******************* - * GET FILE SIZE * - *******************/ + /*************** + * FINISH UP * + ***************/ + /* Get the S3 object's size. This is the only time we touch the S3 object + * (and thus ensure it exists) during the VFD's open callback. + */ if (H5FD__s3comms_s3r_getsize(handle) < 0) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, NULL, "problem in H5FD__s3comms_s3r_getsize"); - /********************* - * FINAL PREPARATION * - *********************/ - - assert(handle->httpverb != NULL); - H5MM_memcpy(handle->httpverb, "GET", 4); - ret_value = handle; done: From 3743ca7680ebbd8f9850f00d66948259ea2c4a1e Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Tue, 14 Jan 2025 18:17:37 -0800 Subject: [PATCH 02/12] Add ros3 cache comments to the driver struct --- src/H5FDros3.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/H5FDros3.c b/src/H5FDros3.c index 0c378383650..5a5cb5275fc 100644 --- a/src/H5FDros3.c +++ b/src/H5FDros3.c @@ -77,28 +77,35 @@ typedef struct H5FD_ros3_stats_bin { * Stores all information needed to maintain access to a single HDF5 file * that has been stored as a S3 object. * - * `pub` (H5FD_t) + * pub * * Instance of H5FD_t which contains all fields common to all VFDs. * It must be the first item in this structure, since at higher levels, * this structure will be treated as an instance of H5FD_t. * - * `fa` (H5FD_ros3_fapl_t) + * fa * * Instance of `H5FD_ros3_fapl_t` containing the S3 configuration data * needed to "open" the HDF5 file. * - * `eoa` (haddr_t) + * eoa * * End of addressed space in file. After open, it should always * equal the file size. * - * `s3r_handle` (s3r_t *) + * s3r_handle * * Instance of S3 Request handle associated with the target resource. * Responsible for communicating with remote host and presenting file * contents as indistinguishable from a file on the local filesystem. * + * cache + * cache_size (in bytes) + * + * A simple cache of the first N bytes of the file. Especially useful + * at file open, when we perform several reads that would otherwise + * be uncached. + * * *** present only if ROS3_SATS is set to enable stats collection *** * * `meta` (H5FD_ros3_stats_bin_t[]) @@ -118,8 +125,8 @@ typedef struct H5FD_ros3_stats_bin { ***************************************************************************/ typedef struct H5FD_ros3_t { H5FD_t pub; - H5FD_ros3_fapl_t fa; haddr_t eoa; + H5FD_ros3_fapl_t fa; s3r_t *s3r_handle; uint8_t *cache; size_t cache_size; @@ -676,9 +683,9 @@ H5Pset_fapl_ros3_token(hid_t fapl_id, const char *token) /*------------------------------------------------------------------------- * Function: H5FD__ros3_open * - * Purpose: Create and/or open a file as an HDF5 file. + * Purpose: Create and/or open a file as an HDF5 file * - * Any flag except H5F_ACC_RDONLY will cause an error. + * Any flag except H5F_ACC_RDONLY will cause an error * * `url` param (as received from `H5FD_open()`) must conform to web url: * NAME :: HTTP "://" DOMAIN [PORT] ["/" [URI] [QUERY] ] @@ -697,9 +704,9 @@ H5FD__ros3_open(const char *url, unsigned flags, hid_t fapl_id, haddr_t maxaddr) { H5FD_ros3_t *file = NULL; unsigned char signing_key[SHA256_DIGEST_LENGTH]; - s3r_t *handle = NULL; - const H5FD_ros3_fapl_t *fa = NULL; - H5P_genplist_t *plist = NULL; + s3r_t *handle = NULL; + const H5FD_ros3_fapl_t *fa = NULL; + H5P_genplist_t *plist = NULL; H5FD_t *ret_value = NULL; FUNC_ENTER_PACKAGE From 014e7dc6187552bfc4d55559148700a7582cd8cd Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Tue, 14 Jan 2025 18:20:52 -0800 Subject: [PATCH 03/12] httpverb --> http_verb --- src/H5FDs3comms.c | 20 ++++++++++---------- src/H5FDs3comms.h | 4 ++-- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/H5FDs3comms.c b/src/H5FDs3comms.c index 7bd7a6d69be..cd05cdd2c95 100644 --- a/src/H5FDs3comms.c +++ b/src/H5FDs3comms.c @@ -565,7 +565,7 @@ H5FD_s3comms_s3r_close(s3r_t *handle) H5MM_xfree(handle->region); H5MM_xfree(handle->signing_key); H5MM_xfree(handle->token); - H5MM_xfree(handle->httpverb); + H5MM_xfree(handle->http_verb); if (H5FD_s3comms_free_purl(handle->purl) < 0) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "unable to release parsed url structure"); @@ -621,7 +621,7 @@ H5FD__s3comms_s3r_getsize(s3r_t *handle) assert(handle); assert(handle->curlhandle); - assert(handle->httpverb); + assert(handle->http_verb); /******************** * PREPARE FOR HEAD * @@ -636,7 +636,7 @@ H5FD__s3comms_s3r_getsize(s3r_t *handle) if (CURLE_OK != curl_easy_setopt(curlh, CURLOPT_HEADERDATA, &sds)) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "error while setting CURL option (CURLOPT_HEADERDATA)"); - strcpy(handle->httpverb, "HEAD"); + strcpy(handle->http_verb, "HEAD"); if (NULL == (header_response = (char *)H5MM_malloc(sizeof(char) * CURL_MAX_HTTP_HEADER))) HGOTO_ERROR(H5E_VFL, H5E_CANTALLOC, FAIL, "unable to allocate space for curl header response"); @@ -705,7 +705,7 @@ H5FD__s3comms_s3r_getsize(s3r_t *handle) if (CURLE_OK != curl_easy_setopt(curlh, CURLOPT_HEADERDATA, NULL)) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "error while setting CURL option (CURLOPT_HEADERDATA)"); - strcpy(handle->httpverb, "GET"); + strcpy(handle->http_verb, "GET"); done: H5MM_xfree(header_response); @@ -779,7 +779,7 @@ H5FD_s3comms_s3r_open(const char *url, const char *region, const char *id, const handle->purl = purl; - if (NULL == (handle->httpverb = (char *)H5MM_calloc(sizeof(char) * 16))) + if (NULL == (handle->http_verb = (char *)H5MM_calloc(sizeof(char) * 16))) HGOTO_ERROR(H5E_VFL, H5E_CANTALLOC, NULL, "unable to allocate space for S3 request HTTP verb"); /************************************* @@ -868,7 +868,7 @@ H5FD_s3comms_s3r_open(const char *url, const char *region, const char *id, const H5MM_xfree(handle->secret_id); H5MM_xfree(handle->signing_key); H5MM_xfree(handle->token); - H5MM_xfree(handle->httpverb); + H5MM_xfree(handle->http_verb); H5MM_xfree(handle); } } @@ -978,7 +978,7 @@ H5FD_s3comms_s3r_read(s3r_t *handle, haddr_t offset, size_t len, void *dest) } #if S3COMMS_CURL_VERBOSITY > 0 - fprintf(stdout, "%s: Bytes %" PRIuHADDR " - %" PRIuHADDR ", Request Size: %zu\n", handle->httpverb, + fprintf(stdout, "%s: Bytes %" PRIuHADDR " - %" PRIuHADDR ", Request Size: %zu\n", handle->http_verb, offset, offset + len - 1, len); fflush(stdout); #endif @@ -1055,8 +1055,8 @@ H5FD_s3comms_s3r_read(s3r_t *handle, haddr_t offset, size_t len, void *dest) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "handle must have non-NULL signing_key"); if (handle->token == NULL) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "handle must have non-NULL token"); - if (handle->httpverb == NULL) - HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "handle must have non-NULL httpverb"); + if (handle->http_verb == NULL) + HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "handle must have non-NULL http_verb"); if (handle->purl->host == NULL) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "handle must have non-NULL host"); if (handle->purl->path == NULL) @@ -1064,7 +1064,7 @@ H5FD_s3comms_s3r_read(s3r_t *handle, haddr_t offset, size_t len, void *dest) /**** CREATE HTTP REQUEST STRUCTURE (hrb_t) ****/ - request = H5FD_s3comms_hrb_init_request((const char *)handle->httpverb, + request = H5FD_s3comms_hrb_init_request((const char *)handle->http_verb, (const char *)handle->purl->path, "HTTP/1.1"); if (request == NULL) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "could not allocate hrb_t request"); diff --git a/src/H5FDs3comms.h b/src/H5FDs3comms.h index b4b4cfea6eb..f85491ba41d 100644 --- a/src/H5FDs3comms.h +++ b/src/H5FDs3comms.h @@ -368,7 +368,7 @@ typedef struct { * * Pointer to the curl_easy handle generated for the request. * - * `httpverb` (char *) + * `http_verb` (char *) * * Pointer to NULL-terminated string. HTTP verb, * e.g. "GET", "HEAD", "PUT", etc. @@ -418,7 +418,7 @@ typedef struct { typedef struct { CURL *curlhandle; size_t filesize; - char *httpverb; + char *http_verb; parsed_url_t *purl; char *region; char *secret_id; From 27971fc90ac4737f5cab16265e736906c5017748 Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Tue, 14 Jan 2025 18:27:50 -0800 Subject: [PATCH 04/12] Rename handle to aws_handle in s3r_t For parity with the same field in the fapl --- src/H5FDs3comms.c | 12 ++++++------ src/H5FDs3comms.h | 37 +++++++++++++++++++------------------ 2 files changed, 25 insertions(+), 24 deletions(-) diff --git a/src/H5FDs3comms.c b/src/H5FDs3comms.c index cd05cdd2c95..8896a0a2fed 100644 --- a/src/H5FDs3comms.c +++ b/src/H5FDs3comms.c @@ -562,7 +562,7 @@ H5FD_s3comms_s3r_close(s3r_t *handle) curl_easy_cleanup(handle->curlhandle); H5MM_xfree(handle->secret_id); - H5MM_xfree(handle->region); + H5MM_xfree(handle->aws_region); H5MM_xfree(handle->signing_key); H5MM_xfree(handle->token); H5MM_xfree(handle->http_verb); @@ -802,7 +802,7 @@ H5FD_s3comms_s3r_open(const char *url, const char *region, const char *id, const HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, NULL, "token cannot be NULL"); /* Copy strings */ - if (NULL == (handle->region = strdup(region))) + if (NULL == (handle->aws_region = strdup(region))) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, NULL, "could not copy region"); if (NULL == (handle->secret_id = strdup(id))) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, NULL, "could not copy ID"); @@ -864,7 +864,7 @@ H5FD_s3comms_s3r_open(const char *url, const char *region, const char *id, const H5MM_xfree(purl); if (handle != NULL) { - H5MM_xfree(handle->region); + H5MM_xfree(handle->aws_region); H5MM_xfree(handle->secret_id); H5MM_xfree(handle->signing_key); H5MM_xfree(handle->token); @@ -1047,7 +1047,7 @@ H5FD_s3comms_s3r_read(s3r_t *handle, haddr_t offset, size_t len, void *dest) /**** VERIFY INFORMATION EXISTS ****/ - if (handle->region == NULL) + if (handle->aws_region == NULL) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "handle must have non-NULL region"); if (handle->secret_id == NULL) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "handle must have non-NULL secret_id"); @@ -1113,7 +1113,7 @@ H5FD_s3comms_s3r_read(s3r_t *handle, haddr_t offset, size_t len, void *dest) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "bad canonical request"); } /* buffer2->string-to-sign */ - if (H5FD_s3comms_make_aws_stringtosign(buffer2, buffer1, iso8601now, handle->region) < 0) + if (H5FD_s3comms_make_aws_stringtosign(buffer2, buffer1, iso8601now, handle->aws_region) < 0) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "bad string-to-sign"); /* buffer1 -> signature */ @@ -1126,7 +1126,7 @@ H5FD_s3comms_s3r_read(s3r_t *handle, haddr_t offset, size_t len, void *dest) /* Trim to yyyyMMDD */ iso8601now[8] = 0; - ret = S3COMMS_FORMAT_CREDENTIAL(buffer2, handle->secret_id, iso8601now, handle->region, "s3"); + ret = S3COMMS_FORMAT_CREDENTIAL(buffer2, handle->secret_id, iso8601now, handle->aws_region, "s3"); if (ret == 0 || ret >= S3COMMS_MAX_CREDENTIAL_SIZE) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "unable to format aws4 credential string"); diff --git a/src/H5FDs3comms.h b/src/H5FDs3comms.h index f85491ba41d..9c3a20a373b 100644 --- a/src/H5FDs3comms.h +++ b/src/H5FDs3comms.h @@ -364,20 +364,20 @@ typedef struct { * undefined behavior if called to perform in multiple threads. * * - * `curlhandle` (CURL) + * curlhandle * - * Pointer to the curl_easy handle generated for the request. + * Pointer to the curl_easy handle generated for the request * - * `http_verb` (char *) + * http_verb * * Pointer to NULL-terminated string. HTTP verb, * e.g. "GET", "HEAD", "PUT", etc. * - * Default is NULL, resulting in a "GET" request. + * Default is NULL, resulting in a "GET" request * - * `purl` (parsed_url_t *) + * purl ("parsed url") * - * Pointer to structure holding the elements of URL for file open. + * Pointer to structure holding the elements of URL for file open * * e.g., "http://bucket.aws.com:8080/myfile.dat?q1=v1&q2=v2" * parsed into... @@ -388,31 +388,32 @@ typedef struct { * query: "q1=v1&q2=v2" * } * - * Cannot be NULL. + * Cannot be NULL * - * `region` (char *) + * aws_region * - * Pointer to NULL-terminated string, specifying S3 "region", + * Pointer to NULL-terminated string, specifying S3 "region" * e.g., "us-east-1". * - * Required to authenticate. + * Required to authenticate * - * `secret_id` (char *) + * secret_id * - * Pointer to NULL-terminated string for "secret" access id to S3 resource. + * Pointer to NULL-terminated string for "secret" access id to S3 resource * - * Required to authenticate. + * Required to authenticate * - * `signing_key` (unsigned char *) + * signing_key * * Pointer to `SHA256_DIGEST_LENGTH`-long string for "reusable" signing * key, generated via * `HMAC-SHA256(HMAC-SHA256(HMAC-SHA256(HMAC-SHA256("AWS4", * ""), ""), "aws4_request")` - * which may be re-used for several (up to seven (7)) days from creation? - * Computed once upon file open. + * which may be re-used for several (up to seven (7)) days from creation * - * Required to authenticate. + * Computed once upon file open from the secret key string in the fapl + * + * Required to authenticate *---------------------------------------------------------------------------- */ typedef struct { @@ -420,7 +421,7 @@ typedef struct { size_t filesize; char *http_verb; parsed_url_t *purl; - char *region; + char *aws_region; char *secret_id; unsigned char *signing_key; char *token; From 14bc7a19044f545ae3aadda980404b08d381414f Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Tue, 14 Jan 2025 18:43:30 -0800 Subject: [PATCH 05/12] Change the type of the signing key to uint8_t Better than unsigned char --- src/H5FDros3.c | 6 +++--- src/H5FDs3comms.c | 4 ++-- src/H5FDs3comms.h | 22 +++++++++++----------- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/H5FDros3.c b/src/H5FDros3.c index 5a5cb5275fc..0e5d3e2720e 100644 --- a/src/H5FDros3.c +++ b/src/H5FDros3.c @@ -702,8 +702,7 @@ H5Pset_fapl_ros3_token(hid_t fapl_id, const char *token) static H5FD_t * H5FD__ros3_open(const char *url, unsigned flags, hid_t fapl_id, haddr_t maxaddr) { - H5FD_ros3_t *file = NULL; - unsigned char signing_key[SHA256_DIGEST_LENGTH]; + H5FD_ros3_t *file = NULL; s3r_t *handle = NULL; const H5FD_ros3_fapl_t *fa = NULL; H5P_genplist_t *plist = NULL; @@ -740,6 +739,7 @@ H5FD__ros3_open(const char *url, unsigned flags, hid_t fapl_id, haddr_t maxaddr) * authenticate requests or not. */ if (fa->authenticate == true) { + uint8_t signing_key[SHA256_DIGEST_LENGTH]; htri_t token_exists; /* Does the token exist in the fapl? */ char *fapl_token = NULL; /* Token from fapl */ const char *token = NULL; /* const pointer passed to s3r_open */ @@ -766,7 +766,7 @@ H5FD__ros3_open(const char *url, unsigned flags, hid_t fapl_id, haddr_t maxaddr) token = ""; handle = H5FD_s3comms_s3r_open(url, (const char *)fa->aws_region, (const char *)fa->secret_id, - (const unsigned char *)signing_key, token); + (const uint8_t *)signing_key, token); } else handle = H5FD_s3comms_s3r_open(url, NULL, NULL, NULL, NULL); diff --git a/src/H5FDs3comms.c b/src/H5FDs3comms.c index 8896a0a2fed..3acbeb6fd19 100644 --- a/src/H5FDs3comms.c +++ b/src/H5FDs3comms.c @@ -725,7 +725,7 @@ H5FD__s3comms_s3r_getsize(s3r_t *handle) *---------------------------------------------------------------------------- */ s3r_t * -H5FD_s3comms_s3r_open(const char *url, const char *region, const char *id, const unsigned char *signing_key, +H5FD_s3comms_s3r_open(const char *url, const char *region, const char *id, const uint8_t *signing_key, const char *token) { CURL *curlh = NULL; @@ -811,7 +811,7 @@ H5FD_s3comms_s3r_open(const char *url, const char *region, const char *id, const /* Copy signing key (not a string) */ tmplen = SHA256_DIGEST_LENGTH; - if (NULL == (handle->signing_key = (unsigned char *)H5MM_malloc(sizeof(unsigned char) * tmplen))) + if (NULL == (handle->signing_key = (uint8_t *)H5MM_malloc(sizeof(uint8_t) * tmplen))) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, NULL, "could not allocate space for handle key"); H5MM_memcpy(handle->signing_key, signing_key, tmplen); } diff --git a/src/H5FDs3comms.h b/src/H5FDs3comms.h index 9c3a20a373b..2678647fa75 100644 --- a/src/H5FDs3comms.h +++ b/src/H5FDs3comms.h @@ -405,7 +405,7 @@ typedef struct { * * signing_key * - * Pointer to `SHA256_DIGEST_LENGTH`-long string for "reusable" signing + * Pointer to `SHA256_DIGEST_LENGTH`-long buffer for "reusable" signing * key, generated via * `HMAC-SHA256(HMAC-SHA256(HMAC-SHA256(HMAC-SHA256("AWS4", * ""), ""), "aws4_request")` @@ -417,14 +417,14 @@ typedef struct { *---------------------------------------------------------------------------- */ typedef struct { - CURL *curlhandle; - size_t filesize; - char *http_verb; - parsed_url_t *purl; - char *aws_region; - char *secret_id; - unsigned char *signing_key; - char *token; + CURL *curlhandle; + size_t filesize; + char *http_verb; + parsed_url_t *purl; + char *aws_region; + char *secret_id; + uint8_t *signing_key; + char *token; } s3r_t; #ifdef __cplusplus @@ -437,8 +437,8 @@ H5_DLL herr_t H5FD_s3comms_hrb_destroy(hrb_t *buf); H5_DLL herr_t H5FD_s3comms_hrb_node_set(hrb_node_t **L, const char *name, const char *value); /* S3 request buffer routines */ -H5_DLL s3r_t *H5FD_s3comms_s3r_open(const char url[], const char region[], const char id[], - const unsigned char signing_key[], const char token[]); +H5_DLL s3r_t *H5FD_s3comms_s3r_open(const char *url, const char *region, const char *id, + const uint8_t *signing_key, const char *token); H5_DLL herr_t H5FD_s3comms_s3r_close(s3r_t *handle); H5_DLL size_t H5FD_s3comms_s3r_get_filesize(s3r_t *handle); H5_DLL herr_t H5FD_s3comms_s3r_read(s3r_t *handle, haddr_t offset, size_t len, void *dest); From 319cbe81f7b5d30e974c3ad69c44a466149e9651 Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Tue, 14 Jan 2025 21:27:39 -0800 Subject: [PATCH 06/12] Move ISO-8601 functionality to a new function * Adds H5FD_s3comms_make_iso_8661_string() and moves associated functionality inside. * Removes gmnow() function * Removes unused RFC-7231 macros --- src/H5FDros3.c | 16 +++++----- src/H5FDs3comms.c | 76 +++++++++++++++++++++++------------------------ src/H5FDs3comms.h | 49 ++++-------------------------- test/s3comms.c | 51 ++++--------------------------- 4 files changed, 57 insertions(+), 135 deletions(-) diff --git a/src/H5FDros3.c b/src/H5FDros3.c index 0e5d3e2720e..62930d5a24a 100644 --- a/src/H5FDros3.c +++ b/src/H5FDros3.c @@ -740,18 +740,20 @@ H5FD__ros3_open(const char *url, unsigned flags, hid_t fapl_id, haddr_t maxaddr) */ if (fa->authenticate == true) { uint8_t signing_key[SHA256_DIGEST_LENGTH]; - htri_t token_exists; /* Does the token exist in the fapl? */ - char *fapl_token = NULL; /* Token from fapl */ - const char *token = NULL; /* const pointer passed to s3r_open */ - char iso8601_now[ISO8601_SIZE]; /* ISO 8601 string */ + char iso8601[ISO8601_SIZE]; /* ISO-8601 time string */ + htri_t token_exists; /* Does the token exist in the fapl? */ + char *fapl_token = NULL; /* Token from fapl */ + const char *token = NULL; /* const pointer passed to s3r_open */ + + /* Get the current time in ISO-8601 format */ + if (H5FD_s3comms_make_iso_8661_string(time(NULL), iso8601) < 0) + HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, NULL, "could not construct ISO-8601 string"); /* Compute signing key (part of AWS/S3 REST API). Can be re-used by * user/key for 7 days after creation. */ - if (ISO8601NOW(iso8601_now, gmnow()) != (ISO8601_SIZE - 1)) - HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, NULL, "problem while writing iso8601 timestamp"); if (H5FD_s3comms_make_aws_signing_key(signing_key, (const char *)fa->secret_key, - (const char *)fa->aws_region, (const char *)iso8601_now) < 0) + (const char *)fa->aws_region, (const char *)iso8601) < 0) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, NULL, "problem while computing signing key"); /* Get the token, if it exists */ diff --git a/src/H5FDs3comms.c b/src/H5FDs3comms.c index 3acbeb6fd19..ece78a38311 100644 --- a/src/H5FDs3comms.c +++ b/src/H5FDs3comms.c @@ -915,7 +915,6 @@ H5FD_s3comms_s3r_read(s3r_t *handle, haddr_t offset, size_t len, void *dest) struct curl_slist *curlheaders = NULL; hrb_node_t *headers = NULL; hrb_node_t *node = NULL; - struct tm *now = NULL; char *rangebytesstr = NULL; hrb_t *request = NULL; char *authorization = NULL; @@ -1007,11 +1006,6 @@ H5FD_s3comms_s3r_read(s3r_t *handle, haddr_t offset, size_t len, void *dest) unsigned char md[SHA256_DIGEST_LENGTH]; unsigned int md_len = SHA256_DIGEST_LENGTH; - /* Authenticate request */ - authorization = (char *)H5MM_malloc(512 + H5FD_ROS3_MAX_SECRET_TOK_LEN + 1); - if (authorization == NULL) - HGOTO_ERROR(H5E_VFL, H5E_NOSPACE, FAIL, "cannot make space for authorization variable"); - /* 4608 := approximate max length... * 67 @@ -1023,7 +1017,12 @@ H5FD_s3comms_s3r_read(s3r_t *handle, haddr_t offset, size_t len, void *dest) * + 4096 */ char buffer2[256 + 1]; /* -> String To Sign -> Credential */ - char iso8601now[ISO8601_SIZE]; + char iso8601[ISO8601_SIZE]; + + /* Authenticate request */ + authorization = (char *)H5MM_malloc(512 + H5FD_ROS3_MAX_SECRET_TOK_LEN + 1); + if (authorization == NULL) + HGOTO_ERROR(H5E_VFL, H5E_NOSPACE, FAIL, "cannot make space for authorization variable"); /* -> Canonical Request -> Signature */ buffer1 = (char *)H5MM_malloc(512 + H5FD_ROS3_MAX_SECRET_TOK_LEN + 1); @@ -1042,7 +1041,7 @@ H5FD_s3comms_s3r_read(s3r_t *handle, haddr_t offset, size_t len, void *dest) authorization[0] = '\0'; buffer1[0] = '\0'; buffer2[0] = '\0'; - iso8601now[0] = '\0'; + iso8601[0] = '\0'; signed_headers[0] = '\0'; /**** VERIFY INFORMATION EXISTS ****/ @@ -1069,11 +1068,11 @@ H5FD_s3comms_s3r_read(s3r_t *handle, haddr_t offset, size_t len, void *dest) if (request == NULL) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "could not allocate hrb_t request"); - now = gmnow(); - if (ISO8601NOW(iso8601now, now) != (ISO8601_SIZE - 1)) - HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "could not format ISO8601 time"); + /* Get a time string for the current time in ISO-8601 format */ + if (H5FD_s3comms_make_iso_8661_string(time(NULL), iso8601) < 0) + HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "could not format ISO-8601 time"); - if (H5FD_s3comms_hrb_node_set(&headers, "x-amz-date", (const char *)iso8601now) < 0) + if (H5FD_s3comms_hrb_node_set(&headers, "x-amz-date", (const char *)iso8601) < 0) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "unable to set x-amz-date header"); if (headers == NULL) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "problem building headers list"); @@ -1113,7 +1112,7 @@ H5FD_s3comms_s3r_read(s3r_t *handle, haddr_t offset, size_t len, void *dest) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "bad canonical request"); } /* buffer2->string-to-sign */ - if (H5FD_s3comms_make_aws_stringtosign(buffer2, buffer1, iso8601now, handle->aws_region) < 0) + if (H5FD_s3comms_make_aws_stringtosign(buffer2, buffer1, iso8601, handle->aws_region) < 0) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "bad string-to-sign"); /* buffer1 -> signature */ @@ -1124,9 +1123,9 @@ H5FD_s3comms_s3r_read(s3r_t *handle, haddr_t offset, size_t len, void *dest) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "could not convert to hex string."); /* Trim to yyyyMMDD */ - iso8601now[8] = 0; + iso8601[8] = 0; - ret = S3COMMS_FORMAT_CREDENTIAL(buffer2, handle->secret_id, iso8601now, handle->aws_region, "s3"); + ret = S3COMMS_FORMAT_CREDENTIAL(buffer2, handle->secret_id, iso8601, handle->aws_region, "s3"); if (ret == 0 || ret >= S3COMMS_MAX_CREDENTIAL_SIZE) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "unable to format aws4 credential string"); @@ -1237,28 +1236,28 @@ H5FD_s3comms_s3r_read(s3r_t *handle, haddr_t offset, size_t len, void *dest) ****************************************************************************/ /*---------------------------------------------------------------------------- - * Function: gmnow + * Function: H5FD_s3comms_make_iso_8661_string * - * Purpose: Call gmtime() using the current time + * Purpose: Create an ISO-8601 string from a time_t * - * Return: struct tm pointer + * Return: SUCCEED/FAIL *---------------------------------------------------------------------------- */ -struct tm * -gmnow(void) +herr_t +H5FD_s3comms_make_iso_8661_string(time_t time, char iso8601[ISO8601_SIZE]) { - time_t now; - time_t *now_ptr = &now; - struct tm *ret_value = NULL; + herr_t ret_value = SUCCEED; - /* Doctor assert, checks against error in time() */ - if ((time_t)(-1) != time(now_ptr)) - ret_value = gmtime(now_ptr); + FUNC_ENTER_NOAPI_NOINIT - assert(ret_value != NULL); + assert(iso8601); - return ret_value; -} /* end gmnow() */ + if (strftime(iso8601, ISO8601_SIZE, "%Y%m%dT%H%M%SZ", gmtime(&time)) != (ISO8601_SIZE - 1)) + HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "could not construct ISO-8601 string"); + +done: + FUNC_LEAVE_NOAPI(ret_value) +} /* end H5FD_s3comms_make_iso_8661_string() */ /*---------------------------------------------------------------------------- * Function: H5FD_s3comms_make_aws_canonical_request @@ -1291,7 +1290,6 @@ H5FD_s3comms_make_aws_canonical_request(char *canonical_request_dest, int _cr_si { hrb_node_t *node = NULL; const char *query_params = ""; /* unused at present */ - herr_t ret_value = SUCCEED; int ret = 0; size_t cr_size = (size_t)_cr_size; size_t sh_size = (size_t)_sh_size; @@ -1299,6 +1297,7 @@ H5FD_s3comms_make_aws_canonical_request(char *canonical_request_dest, int _cr_si size_t sh_len = 0; /* working length of signed headers str */ char *tmpstr = NULL; const size_t TMP_STR_SIZE = sizeof(char) * H5FD_ROS3_MAX_SECRET_TOK_LEN; + herr_t ret_value = SUCCEED; /* "query params" refers to the optional element in the URL, e.g. * http://bucket.aws.com/myfile.txt?max-keys=2&prefix=J @@ -1636,14 +1635,15 @@ H5FD_s3comms_load_aws_profile(const char *profile_name, char *key_id_out, char * * Purpose: Create AWS4 "Signing Key" from secret key, AWS region, and * timestamp * - * `secret` is `access key id` for targeted service/bucket/resource. + * `secret` is `access key id` for targeted service/bucket/resource * - * `iso8601now` must conform to format, yyyyMMDD'T'hhmmss'Z' - * e.g. "19690720T201740Z". + * `region` should be one of AWS service region names, e.g. "us-east-1" * - * `region` should be one of AWS service region names, e.g. "us-east-1". + * `iso8601` is an ISO-8601 time string with no punctuation + * (e.g.: "20170713T145903Z", so... YYYYmmdd'T'HHMMSSZ). + * This can be constructed using H5FD_s3comms_make_iso_8661_string(). * - * Hard-coded "service" algorithm requirement to "s3". + * Hard-coded "service" algorithm requirement to "s3" * * Writes to `md` the raw byte data, length of `SHA256_DIGEST_LENGTH`. * Programmer must ensure that `md` is appropriately allocated. @@ -1653,7 +1653,7 @@ H5FD_s3comms_load_aws_profile(const char *profile_name, char *key_id_out, char * */ herr_t H5FD_s3comms_make_aws_signing_key(unsigned char *md, const char *secret, const char *region, - const char *iso8601now) + const char *iso8601) { char *AWS4_secret = NULL; size_t AWS4_secret_len = 0; @@ -1671,8 +1671,6 @@ H5FD_s3comms_make_aws_signing_key(unsigned char *md, const char *secret, const c HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "`secret` cannot be NULL"); if (region == NULL) HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "`region` cannot be NULL"); - if (iso8601now == NULL) - HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "`iso8601now` cannot be NULL"); AWS4_secret_len = 4 + strlen(secret) + 1; AWS4_secret = (char *)H5MM_malloc(AWS4_secret_len); @@ -1692,7 +1690,7 @@ H5FD_s3comms_make_aws_signing_key(unsigned char *md, const char *secret, const c * we know digest length, so ignore via NULL */ HMAC(EVP_sha256(), (const unsigned char *)AWS4_secret, (int)strlen(AWS4_secret), - (const unsigned char *)iso8601now, 8, /* 8 --> length of 8 --> "yyyyMMDD" */ + (const unsigned char *)iso8601, 8, /* 8 --> length of 8 --> "yyyyMMDD" */ datekey, NULL); HMAC(EVP_sha256(), (const unsigned char *)datekey, SHA256_DIGEST_LENGTH, (const unsigned char *)region, diff --git a/src/H5FDs3comms.h b/src/H5FDs3comms.h index 2678647fa75..0b8ffab0f0b 100644 --- a/src/H5FDs3comms.h +++ b/src/H5FDs3comms.h @@ -56,9 +56,9 @@ #include #include -/***************** - * PUBLIC MACROS * - *****************/ +/********** + * MACROS * + **********/ /* hexadecimal string of pre-computed sha256 checksum of the empty string * hex(sha256sum("")) @@ -70,43 +70,6 @@ */ #define ISO8601_SIZE 17 -/* string length (plus null terminator) - * example RFC7231-format string: "Fri, 30 Jun 2017 20:41:55 GMT" - */ -#define RFC7231_SIZE 30 - -/*--------------------------------------------------------------------------- - * Macro: ISO8601NOW() - * - * Purpose: - * - * write "YYYYmmdd'T'HHMMSS'Z'" (less single-quotes) to dest - * e.g., "20170630T204155Z" - * - * wrapper for strftime() - * - * It is left to the programmer to check return value of - * ISO8601NOW (should equal ISO8601_SIZE - 1). - *--------------------------------------------------------------------------- - */ -#define ISO8601NOW(dest, now_gm) strftime((dest), ISO8601_SIZE, "%Y%m%dT%H%M%SZ", (now_gm)) - -/*--------------------------------------------------------------------------- - * Macro: RFC7231NOW() - * - * Purpose: - * - * write "Day, dd Mmm YYYY HH:MM:SS GMT" to dest - * e.g., "Fri, 30 Jun 2017 20:41:55 GMT" - * - * wrapper for strftime() - * - * It is left to the programmer to check return value of - * RFC7231NOW (should equal RFC7231_SIZE - 1). - *--------------------------------------------------------------------------- - */ -#define RFC7231NOW(dest, now_gm) strftime((dest), RFC7231_SIZE, "%a, %d %b %Y %H:%M:%S GMT", (now_gm)) - /* Reasonable maximum length of a credential string. * Provided for error-checking S3COMMS_FORMAT_CREDENTIAL (below). * 17 <- "////aws4_request\0" @@ -448,13 +411,13 @@ H5_DLL herr_t H5FD_s3comms_make_aws_canonical_request(char *canonical_request_de char *signed_headers_dest, int sh_size, hrb_t *http_request); H5_DLL herr_t H5FD_s3comms_make_aws_signing_key(unsigned char *md, const char *secret, const char *region, - const char *iso8601now); + const char *iso8601); H5_DLL herr_t H5FD_s3comms_make_aws_stringtosign(char *dest, const char *req_str, const char *now, const char *region); /* Misc routines */ -H5_DLL struct tm *gmnow(void); -H5_DLL herr_t H5FD_s3comms_free_purl(parsed_url_t *purl); +H5_DLL herr_t H5FD_s3comms_make_iso_8661_string(time_t time, char iso8601[ISO8601_SIZE]); +H5_DLL herr_t H5FD_s3comms_free_purl(parsed_url_t *purl); /* Testing routines */ #ifdef H5FD_S3COMMS_TESTING diff --git a/test/s3comms.c b/test/s3comms.c index 297544ea619..8da35160dda 100644 --- a/test/s3comms.c +++ b/test/s3comms.c @@ -735,6 +735,7 @@ test_hrb_node_set(void) } /* end test_hrb_node_set() */ +/* This is difficult to test since we took away the time parameter */ /*--------------------------------------------------------------------------- * Function: test_make_aws_signing_key * @@ -769,7 +770,6 @@ test_make_aws_signing_key(void) unsigned char *key = NULL; const int NCASES = 1; - herr_t ret; TESTING("make AWS signing key"); @@ -787,45 +787,6 @@ test_make_aws_signing_key(void) key = NULL; } - /* ERROR CASES */ - - if (NULL == (key = (unsigned char *)malloc(sizeof(unsigned char) * SHA256_DIGEST_LENGTH))) - TEST_ERROR; - - H5E_BEGIN_TRY - { - ret = H5FD_s3comms_make_aws_signing_key(NULL, cases[0].secret_key, cases[0].region, cases[0].when); - } - H5E_END_TRY - if (ret == SUCCEED) - FAIL_PUTS_ERROR("destination cannot be NULL"); - - H5E_BEGIN_TRY - { - ret = H5FD_s3comms_make_aws_signing_key(key, NULL, cases[0].region, cases[0].when); - } - H5E_END_TRY - if (ret == SUCCEED) - FAIL_PUTS_ERROR("secret key cannot be NULL"); - - H5E_BEGIN_TRY - { - ret = H5FD_s3comms_make_aws_signing_key(key, cases[0].secret_key, NULL, cases[0].when); - } - H5E_END_TRY - if (ret == SUCCEED) - FAIL_PUTS_ERROR("aws region cannot be NULL"); - - H5E_BEGIN_TRY - { - ret = H5FD_s3comms_make_aws_signing_key(key, cases[0].secret_key, cases[0].region, NULL); - } - H5E_END_TRY - if (ret == SUCCEED) - FAIL_PUTS_ERROR("time string cannot be NULL"); - - free(key); - PASSED(); return 0; @@ -963,12 +924,11 @@ test_s3r_get_filesize(void) static int test_s3r_open(void) { + char iso8601[ISO8601_SIZE]; /* ISO-8601 time string */ char url_missing[S3_TEST_MAX_URL_SIZE]; char url_raven[S3_TEST_MAX_URL_SIZE]; char url_shakespeare[S3_TEST_MAX_URL_SIZE]; unsigned char signing_key[SHA256_DIGEST_LENGTH]; - struct tm *now = NULL; - char iso8601now[ISO8601_SIZE]; s3r_t *handle = NULL; TESTING("s3r_open"); @@ -1002,16 +962,15 @@ test_s3r_open(void) snprintf(url_raven, S3_TEST_MAX_URL_SIZE, "%s/%s", s3_test_bucket_url, S3_TEST_RESOURCE_TEXT_PUBLIC)) TEST_ERROR; - if (NULL == (now = gmnow())) - TEST_ERROR; - if (ISO8601NOW(iso8601now, now) != (ISO8601_SIZE - 1)) + /* Get the current time in ISO-8601 format */ + if (H5FD_s3comms_make_iso_8661_string(time(NULL), iso8601) < 0) TEST_ERROR; /* It is desired to have means available to verify that signing_key * was set successfully and to an expected value. */ if (H5FD_s3comms_make_aws_signing_key(signing_key, (const char *)s3_test_aws_secret_access_key, - (const char *)s3_test_aws_region, (const char *)iso8601now) < 0) + (const char *)s3_test_aws_region, (const char *)iso8601) < 0) TEST_ERROR; /************************* From 38975488a201f8265f61b2756973706ae238cffe Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Wed, 15 Jan 2025 00:45:00 -0800 Subject: [PATCH 07/12] Convert H5FDs3comms functions to package functions --- src/H5FDros3.c | 26 ++++---- src/H5FDs3comms.c | 150 +++++++++++++++++++++++----------------------- src/H5FDs3comms.h | 42 ++++++------- test/ros3.c | 4 +- test/s3comms.c | 140 +++++++++++++++++++++---------------------- 5 files changed, 181 insertions(+), 181 deletions(-) diff --git a/src/H5FDros3.c b/src/H5FDros3.c index 62930d5a24a..680a2f727ae 100644 --- a/src/H5FDros3.c +++ b/src/H5FDros3.c @@ -746,14 +746,14 @@ H5FD__ros3_open(const char *url, unsigned flags, hid_t fapl_id, haddr_t maxaddr) const char *token = NULL; /* const pointer passed to s3r_open */ /* Get the current time in ISO-8601 format */ - if (H5FD_s3comms_make_iso_8661_string(time(NULL), iso8601) < 0) + if (H5FD__s3comms_make_iso_8661_string(time(NULL), iso8601) < 0) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, NULL, "could not construct ISO-8601 string"); /* Compute signing key (part of AWS/S3 REST API). Can be re-used by * user/key for 7 days after creation. */ - if (H5FD_s3comms_make_aws_signing_key(signing_key, (const char *)fa->secret_key, - (const char *)fa->aws_region, (const char *)iso8601) < 0) + if (H5FD__s3comms_make_aws_signing_key(signing_key, (const char *)fa->secret_key, + (const char *)fa->aws_region, (const char *)iso8601) < 0) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, NULL, "problem while computing signing key"); /* Get the token, if it exists */ @@ -767,11 +767,11 @@ H5FD__ros3_open(const char *url, unsigned flags, hid_t fapl_id, haddr_t maxaddr) else token = ""; - handle = H5FD_s3comms_s3r_open(url, (const char *)fa->aws_region, (const char *)fa->secret_id, - (const uint8_t *)signing_key, token); + handle = H5FD__s3comms_s3r_open(url, (const char *)fa->aws_region, (const char *)fa->secret_id, + (const uint8_t *)signing_key, token); } else - handle = H5FD_s3comms_s3r_open(url, NULL, NULL, NULL, NULL); + handle = H5FD__s3comms_s3r_open(url, NULL, NULL, NULL, NULL); if (handle == NULL) HGOTO_ERROR(H5E_VFL, H5E_CANTOPENFILE, NULL, "s3r_open failed"); @@ -790,13 +790,13 @@ H5FD__ros3_open(const char *url, unsigned flags, hid_t fapl_id, haddr_t maxaddr) /* Cache the initial bytes of the file */ { - size_t filesize = H5FD_s3comms_s3r_get_filesize(file->s3r_handle); + size_t filesize = H5FD__s3comms_s3r_get_filesize(file->s3r_handle); file->cache_size = (filesize < ROS3_MAX_CACHE_SIZE) ? filesize : ROS3_MAX_CACHE_SIZE; if (NULL == (file->cache = (uint8_t *)H5MM_calloc(file->cache_size))) HGOTO_ERROR(H5E_VFL, H5E_NOSPACE, NULL, "unable to allocate cache memory"); - if (H5FD_s3comms_s3r_read(file->s3r_handle, 0, file->cache_size, file->cache) < 0) + if (H5FD__s3comms_s3r_read(file->s3r_handle, 0, file->cache_size, file->cache) < 0) HGOTO_ERROR(H5E_VFL, H5E_READERROR, NULL, "unable to execute read"); } @@ -805,7 +805,7 @@ H5FD__ros3_open(const char *url, unsigned flags, hid_t fapl_id, haddr_t maxaddr) done: if (ret_value == NULL) { if (handle != NULL) - if (H5FD_s3comms_s3r_close(handle) < 0) + if (H5FD__s3comms_s3r_close(handle) < 0) HDONE_ERROR(H5E_VFL, H5E_CANTCLOSEFILE, NULL, "unable to close s3 file handle"); if (file != NULL) { H5MM_xfree(file->cache); @@ -842,7 +842,7 @@ H5FD__ros3_close(H5FD_t H5_ATTR_UNUSED *_file) #endif /* Close the underlying request handle */ - if (H5FD_s3comms_s3r_close(file->s3r_handle) < 0) + if (H5FD__s3comms_s3r_close(file->s3r_handle) < 0) HGOTO_ERROR(H5E_VFL, H5E_CANTCLOSEFILE, FAIL, "unable to close S3 request handle"); /* Release the file info */ @@ -1055,7 +1055,7 @@ H5FD__ros3_get_eof(const H5FD_t *_file, H5FD_mem_t H5_ATTR_UNUSED type) FUNC_ENTER_PACKAGE_NOERR - FUNC_LEAVE_NOAPI(H5FD_s3comms_s3r_get_filesize(file->s3r_handle)) + FUNC_LEAVE_NOAPI(H5FD__s3comms_s3r_get_filesize(file->s3r_handle)) } /* end H5FD__ros3_get_eof() */ /*------------------------------------------------------------------------- @@ -1108,7 +1108,7 @@ H5FD__ros3_read(H5FD_t *_file, H5FD_mem_t H5_ATTR_UNUSED type, hid_t H5_ATTR_UNU assert(file->s3r_handle); assert(buf); - filesize = H5FD_s3comms_s3r_get_filesize(file->s3r_handle); + filesize = H5FD__s3comms_s3r_get_filesize(file->s3r_handle); if ((addr > filesize) || ((addr + size) > filesize)) HGOTO_ERROR(H5E_ARGS, H5E_OVERFLOW, FAIL, "range exceeds file address"); @@ -1120,7 +1120,7 @@ H5FD__ros3_read(H5FD_t *_file, H5FD_mem_t H5_ATTR_UNUSED type, hid_t H5_ATTR_UNU memcpy(buf, file->cache + addr, size); } else { - if (H5FD_s3comms_s3r_read(file->s3r_handle, addr, size, buf) < 0) + if (H5FD__s3comms_s3r_read(file->s3r_handle, addr, size, buf) < 0) HGOTO_ERROR(H5E_VFL, H5E_READERROR, FAIL, "unable to execute read"); #ifdef ROS3_STATS diff --git a/src/H5FDs3comms.c b/src/H5FDs3comms.c index ece78a38311..034b325275c 100644 --- a/src/H5FDs3comms.c +++ b/src/H5FDs3comms.c @@ -137,7 +137,7 @@ H5FD__s3comms_curl_write_callback(char *ptr, size_t size, size_t nmemb, void *us } /* end H5FD__s3comms_curl_write_callback() */ /*---------------------------------------------------------------------------- - * Function: H5FD_s3comms_hrb_node_set + * Function: H5FD__s3comms_hrb_node_set * * Purpose: * @@ -186,7 +186,7 @@ H5FD__s3comms_curl_write_callback(char *ptr, size_t size, size_t nmemb, void *us *---------------------------------------------------------------------------- */ herr_t -H5FD_s3comms_hrb_node_set(hrb_node_t **L, const char *name, const char *value) +H5FD__s3comms_hrb_node_set(hrb_node_t **L, const char *name, const char *value) { size_t i = 0; char *valuecpy = NULL; @@ -199,7 +199,7 @@ H5FD_s3comms_hrb_node_set(hrb_node_t **L, const char *name, const char *value) bool is_looking = true; herr_t ret_value = SUCCEED; - FUNC_ENTER_NOAPI_NOINIT + FUNC_ENTER_PACKAGE if (name == NULL) HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "unable to operate on NULL name"); @@ -430,10 +430,10 @@ H5FD_s3comms_hrb_node_set(hrb_node_t **L, const char *name, const char *value) } FUNC_LEAVE_NOAPI(ret_value) -} /* end H5FD_s3comms_hrb_node_set() */ +} /* end H5FD__s3comms_hrb_node_set() */ /*---------------------------------------------------------------------------- - * Function: H5FD_s3comms_hrb_destroy + * Function: H5FD__s3comms_hrb_destroy * * Purpose: Destroy and free resources associated with an HTTP buffer * @@ -445,9 +445,9 @@ H5FD_s3comms_hrb_node_set(hrb_node_t **L, const char *name, const char *value) *---------------------------------------------------------------------------- */ herr_t -H5FD_s3comms_hrb_destroy(hrb_t *buf) +H5FD__s3comms_hrb_destroy(hrb_t *buf) { - FUNC_ENTER_NOAPI_NOINIT_NOERR + FUNC_ENTER_PACKAGE_NOERR if (buf != NULL) { H5MM_xfree(buf->verb); @@ -457,10 +457,10 @@ H5FD_s3comms_hrb_destroy(hrb_t *buf) } FUNC_LEAVE_NOAPI(SUCCEED) -} /* end H5FD_s3comms_hrb_destroy() */ +} /* end H5FD__s3comms_hrb_destroy() */ /*---------------------------------------------------------------------------- - * Function: H5FD_s3comms_hrb_init_request + * Function: H5FD__s3comms_hrb_init_request * * Purpose: Create a new HTTP Request Buffer * @@ -476,7 +476,7 @@ H5FD_s3comms_hrb_destroy(hrb_t *buf) *---------------------------------------------------------------------------- */ hrb_t * -H5FD_s3comms_hrb_init_request(const char *_verb, const char *_resource, const char *_http_version) +H5FD__s3comms_hrb_init_request(const char *_verb, const char *_resource, const char *_http_version) { hrb_t *request = NULL; char *res = NULL; @@ -484,7 +484,7 @@ H5FD_s3comms_hrb_init_request(const char *_verb, const char *_resource, const ch char *vrsn = NULL; hrb_t *ret_value = NULL; - FUNC_ENTER_NOAPI_NOINIT + FUNC_ENTER_PACKAGE if (_resource == NULL) HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, NULL, "resource string cannot be NULL"); @@ -534,14 +534,14 @@ H5FD_s3comms_hrb_init_request(const char *_verb, const char *_resource, const ch } FUNC_LEAVE_NOAPI(ret_value) -} /* end H5FD_s3comms_hrb_init_request() */ +} /* end H5FD__s3comms_hrb_init_request() */ /**************************************************************************** * S3R FUNCTIONS ****************************************************************************/ /*---------------------------------------------------------------------------- - * Function: H5FD_s3comms_s3r_close + * Function: H5FD__s3comms_s3r_close * * Purpose: Close communications through given S3 Request Handle (s3r_t) * and clean up associated resources @@ -550,11 +550,11 @@ H5FD_s3comms_hrb_init_request(const char *_verb, const char *_resource, const ch *---------------------------------------------------------------------------- */ herr_t -H5FD_s3comms_s3r_close(s3r_t *handle) +H5FD__s3comms_s3r_close(s3r_t *handle) { herr_t ret_value = SUCCEED; - FUNC_ENTER_NOAPI_NOINIT + FUNC_ENTER_PACKAGE if (handle == NULL) HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "handle cannot be NULL"); @@ -567,7 +567,7 @@ H5FD_s3comms_s3r_close(s3r_t *handle) H5MM_xfree(handle->token); H5MM_xfree(handle->http_verb); - if (H5FD_s3comms_free_purl(handle->purl) < 0) + if (H5FD__s3comms_free_purl(handle->purl) < 0) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "unable to release parsed url structure"); H5MM_xfree(handle->purl); @@ -575,10 +575,10 @@ H5FD_s3comms_s3r_close(s3r_t *handle) done: FUNC_LEAVE_NOAPI(ret_value) -} /* H5FD_s3comms_s3r_close */ +} /* H5FD__s3comms_s3r_close */ /*---------------------------------------------------------------------------- - * Function: H5FD_s3comms_s3r_get_filesize + * Function: H5FD__s3comms_s3r_get_filesize * * Purpose: Retrieve the filesize of an open request handle * @@ -586,17 +586,17 @@ H5FD_s3comms_s3r_close(s3r_t *handle) *---------------------------------------------------------------------------- */ size_t -H5FD_s3comms_s3r_get_filesize(s3r_t *handle) +H5FD__s3comms_s3r_get_filesize(s3r_t *handle) { size_t ret_value = 0; - FUNC_ENTER_NOAPI_NOINIT_NOERR + FUNC_ENTER_PACKAGE_NOERR if (handle != NULL) ret_value = handle->filesize; FUNC_LEAVE_NOAPI(ret_value) -} /* H5FD_s3comms_s3r_get_filesize */ +} /* H5FD__s3comms_s3r_get_filesize */ /*---------------------------------------------------------------------------- * Function: H5FD__s3comms_s3r_getsize @@ -650,7 +650,7 @@ H5FD__s3comms_s3r_getsize(s3r_t *handle) * NOBODY and HEADERDATA supplied above, only http metadata will be sent by * the server and recorded by s3comms */ - if (H5FD_s3comms_s3r_read(handle, 0, 0, NULL) < 0) + if (H5FD__s3comms_s3r_read(handle, 0, 0, NULL) < 0) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "problem in reading during getsize"); if (sds.size > CURL_MAX_HTTP_HEADER) @@ -713,7 +713,7 @@ H5FD__s3comms_s3r_getsize(s3r_t *handle) } /* H5FD__s3comms_s3r_getsize */ /*---------------------------------------------------------------------------- - * Function: H5FD_s3comms_s3r_open + * Function: H5FD__s3comms_s3r_open * * Purpose: Logically open a file hosted on S3 * @@ -725,8 +725,8 @@ H5FD__s3comms_s3r_getsize(s3r_t *handle) *---------------------------------------------------------------------------- */ s3r_t * -H5FD_s3comms_s3r_open(const char *url, const char *region, const char *id, const uint8_t *signing_key, - const char *token) +H5FD__s3comms_s3r_open(const char *url, const char *region, const char *id, const uint8_t *signing_key, + const char *token) { CURL *curlh = NULL; s3r_t *handle = NULL; @@ -735,7 +735,7 @@ H5FD_s3comms_s3r_open(const char *url, const char *region, const char *id, const CURLUcode rc; s3r_t *ret_value = NULL; - FUNC_ENTER_NOAPI_NOINIT + FUNC_ENTER_PACKAGE if (url == NULL) HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, NULL, "url cannot be NULL"); @@ -859,7 +859,7 @@ H5FD_s3comms_s3r_open(const char *url, const char *region, const char *id, const if (ret_value == NULL) { curl_easy_cleanup(curlh); - if (H5FD_s3comms_free_purl(purl) < 0) + if (H5FD__s3comms_free_purl(purl) < 0) HDONE_ERROR(H5E_VFL, H5E_BADVALUE, NULL, "unable to free parsed url structure"); H5MM_xfree(purl); @@ -874,10 +874,10 @@ H5FD_s3comms_s3r_open(const char *url, const char *region, const char *id, const } FUNC_LEAVE_NOAPI(ret_value) -} /* H5FD_s3comms_s3r_open */ +} /* H5FD__s3comms_s3r_open */ /*---------------------------------------------------------------------------- - * Function: H5FD_s3comms_s3r_read + * Function: H5FD__s3comms_s3r_read * * Purpose: Read file pointed to by request handle, writing specified * offset .. (offset + len) bytes to buffer dest @@ -908,7 +908,7 @@ H5FD_s3comms_s3r_open(const char *url, const char *region, const char *id, const *---------------------------------------------------------------------------- */ herr_t -H5FD_s3comms_s3r_read(s3r_t *handle, haddr_t offset, size_t len, void *dest) +H5FD__s3comms_s3r_read(s3r_t *handle, haddr_t offset, size_t len, void *dest) { CURL *curlh = NULL; CURLcode p_status = CURLE_OK; @@ -924,7 +924,7 @@ H5FD_s3comms_s3r_read(s3r_t *handle, haddr_t offset, size_t len, void *dest) int ret = 0; herr_t ret_value = SUCCEED; - FUNC_ENTER_NOAPI_NOINIT + FUNC_ENTER_PACKAGE /************************************** * ABSOLUTELY NECESSARY SANITY-CHECKS * @@ -1063,40 +1063,40 @@ H5FD_s3comms_s3r_read(s3r_t *handle, haddr_t offset, size_t len, void *dest) /**** CREATE HTTP REQUEST STRUCTURE (hrb_t) ****/ - request = H5FD_s3comms_hrb_init_request((const char *)handle->http_verb, - (const char *)handle->purl->path, "HTTP/1.1"); + request = H5FD__s3comms_hrb_init_request((const char *)handle->http_verb, + (const char *)handle->purl->path, "HTTP/1.1"); if (request == NULL) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "could not allocate hrb_t request"); /* Get a time string for the current time in ISO-8601 format */ - if (H5FD_s3comms_make_iso_8661_string(time(NULL), iso8601) < 0) + if (H5FD__s3comms_make_iso_8661_string(time(NULL), iso8601) < 0) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "could not format ISO-8601 time"); - if (H5FD_s3comms_hrb_node_set(&headers, "x-amz-date", (const char *)iso8601) < 0) + if (H5FD__s3comms_hrb_node_set(&headers, "x-amz-date", (const char *)iso8601) < 0) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "unable to set x-amz-date header"); if (headers == NULL) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "problem building headers list"); - if (H5FD_s3comms_hrb_node_set(&headers, "x-amz-content-sha256", (const char *)EMPTY_SHA256) < 0) + if (H5FD__s3comms_hrb_node_set(&headers, "x-amz-content-sha256", (const char *)EMPTY_SHA256) < 0) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "unable to set x-amz-content-sha256 header"); if (headers == NULL) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "problem building headers list"); if (strlen((const char *)handle->token) > 0) { - if (H5FD_s3comms_hrb_node_set(&headers, "x-amz-security-token", (const char *)handle->token) < 0) + if (H5FD__s3comms_hrb_node_set(&headers, "x-amz-security-token", (const char *)handle->token) < 0) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "unable to set x-amz-security-token header"); if (headers == NULL) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "problem building headers list"); } if (rangebytesstr != NULL) { - if (H5FD_s3comms_hrb_node_set(&headers, "Range", rangebytesstr) < 0) + if (H5FD__s3comms_hrb_node_set(&headers, "Range", rangebytesstr) < 0) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "unable to set range header"); if (headers == NULL) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "problem building headers list"); } - if (H5FD_s3comms_hrb_node_set(&headers, "Host", handle->purl->host) < 0) + if (H5FD__s3comms_hrb_node_set(&headers, "Host", handle->purl->host) < 0) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "unable to set host header"); if (headers == NULL) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "problem building headers list"); @@ -1106,13 +1106,13 @@ H5FD_s3comms_s3r_read(s3r_t *handle, haddr_t offset, size_t len, void *dest) /**** COMPUTE AUTHORIZATION ****/ /* buffer1 -> canonical request */ - if (H5FD_s3comms_make_aws_canonical_request(buffer1, 512 + H5FD_ROS3_MAX_SECRET_TOK_LEN, - signed_headers, 48 + H5FD_ROS3_MAX_SECRET_TOK_LEN, - request) < 0) { + if (H5FD__s3comms_make_aws_canonical_request(buffer1, 512 + H5FD_ROS3_MAX_SECRET_TOK_LEN, + signed_headers, 48 + H5FD_ROS3_MAX_SECRET_TOK_LEN, + request) < 0) { HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "bad canonical request"); } /* buffer2->string-to-sign */ - if (H5FD_s3comms_make_aws_stringtosign(buffer2, buffer1, iso8601, handle->aws_region) < 0) + if (H5FD__s3comms_make_aws_stringtosign(buffer2, buffer1, iso8601, handle->aws_region) < 0) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "bad string-to-sign"); /* buffer1 -> signature */ @@ -1136,7 +1136,7 @@ H5FD_s3comms_s3r_read(s3r_t *handle, haddr_t offset, size_t len, void *dest) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "unable to format aws4 authorization string"); /* Append authorization header to http request buffer */ - if (H5FD_s3comms_hrb_node_set(&headers, "Authorization", (const char *)authorization) < 0) + if (H5FD__s3comms_hrb_node_set(&headers, "Authorization", (const char *)authorization) < 0) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "unable to set Authorization header"); if (headers == NULL) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "problem building headers list"); @@ -1211,10 +1211,10 @@ H5FD_s3comms_s3r_read(s3r_t *handle, haddr_t offset, size_t len, void *dest) curl_slist_free_all(curlheaders); if (request != NULL) { while (headers != NULL) - if (H5FD_s3comms_hrb_node_set(&headers, headers->name, NULL) < 0) + if (H5FD__s3comms_hrb_node_set(&headers, headers->name, NULL) < 0) HDONE_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "cannot release header node"); assert(NULL == headers); - if (H5FD_s3comms_hrb_destroy(request) < 0) + if (H5FD__s3comms_hrb_destroy(request) < 0) HDONE_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "cannot release header request structure"); } @@ -1229,14 +1229,14 @@ H5FD_s3comms_s3r_read(s3r_t *handle, haddr_t offset, size_t len, void *dest) } FUNC_LEAVE_NOAPI(ret_value) -} /* H5FD_s3comms_s3r_read */ +} /* H5FD__s3comms_s3r_read */ /**************************************************************************** * MISCELLANEOUS FUNCTIONS ****************************************************************************/ /*---------------------------------------------------------------------------- - * Function: H5FD_s3comms_make_iso_8661_string + * Function: H5FD__s3comms_make_iso_8661_string * * Purpose: Create an ISO-8601 string from a time_t * @@ -1244,11 +1244,11 @@ H5FD_s3comms_s3r_read(s3r_t *handle, haddr_t offset, size_t len, void *dest) *---------------------------------------------------------------------------- */ herr_t -H5FD_s3comms_make_iso_8661_string(time_t time, char iso8601[ISO8601_SIZE]) +H5FD__s3comms_make_iso_8661_string(time_t time, char iso8601[ISO8601_SIZE]) { herr_t ret_value = SUCCEED; - FUNC_ENTER_NOAPI_NOINIT + FUNC_ENTER_PACKAGE assert(iso8601); @@ -1257,10 +1257,10 @@ H5FD_s3comms_make_iso_8661_string(time_t time, char iso8601[ISO8601_SIZE]) done: FUNC_LEAVE_NOAPI(ret_value) -} /* end H5FD_s3comms_make_iso_8661_string() */ +} /* end H5FD__s3comms_make_iso_8661_string() */ /*---------------------------------------------------------------------------- - * Function: H5FD_s3comms_make_aws_canonical_request + * Function: H5FD__s3comms_make_aws_canonical_request * * Purpose: Compose AWS "Canonical Request" (and signed headers string) * as defined in the REST API documentation. @@ -1285,8 +1285,8 @@ H5FD_s3comms_make_iso_8661_string(time_t time, char iso8601[ISO8601_SIZE]) *---------------------------------------------------------------------------- */ herr_t -H5FD_s3comms_make_aws_canonical_request(char *canonical_request_dest, int _cr_size, char *signed_headers_dest, - int _sh_size, hrb_t *http_request) +H5FD__s3comms_make_aws_canonical_request(char *canonical_request_dest, int _cr_size, + char *signed_headers_dest, int _sh_size, hrb_t *http_request) { hrb_node_t *node = NULL; const char *query_params = ""; /* unused at present */ @@ -1309,7 +1309,7 @@ H5FD_s3comms_make_aws_canonical_request(char *canonical_request_dest, int _cr_si * VFD use-cases. */ - FUNC_ENTER_NOAPI_NOINIT + FUNC_ENTER_PACKAGE if (http_request == NULL) HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "hrb object cannot be NULL"); @@ -1373,7 +1373,7 @@ H5FD_s3comms_make_aws_canonical_request(char *canonical_request_dest, int _cr_si free(tmpstr); FUNC_LEAVE_NOAPI(ret_value) -} /* end H5FD_s3comms_make_aws_canonical_request() */ +} /* end H5FD__s3comms_make_aws_canonical_request() */ /*---------------------------------------------------------------------------- * Function: H5FD__s3comms_bytes_to_hex @@ -1409,7 +1409,7 @@ H5FD__s3comms_bytes_to_hex(char *dest, size_t dest_len, const unsigned char *msg } /* end H5FD__s3comms_bytes_to_hex() */ /*---------------------------------------------------------------------------- - * Function: H5FD_s3comms_free_purl + * Function: H5FD__s3comms_free_purl * * Purpose: Release resources from a parsed_url_t pointer * @@ -1417,11 +1417,11 @@ H5FD__s3comms_bytes_to_hex(char *dest, size_t dest_len, const unsigned char *msg *---------------------------------------------------------------------------- */ herr_t -H5FD_s3comms_free_purl(parsed_url_t *purl) +H5FD__s3comms_free_purl(parsed_url_t *purl) { herr_t ret_value = SUCCEED; - FUNC_ENTER_NOAPI_NOINIT_NOERR + FUNC_ENTER_PACKAGE_NOERR if (NULL == purl) HGOTO_DONE(SUCCEED); @@ -1434,7 +1434,7 @@ H5FD_s3comms_free_purl(parsed_url_t *purl) done: FUNC_LEAVE_NOAPI(ret_value) -} /* end H5FD_s3comms_free_purl() */ +} /* end H5FD__s3comms_free_purl() */ /*----------------------------------------------------------------------------- * Function: H5FD__s3comms_load_aws_creds_from_file @@ -1553,7 +1553,7 @@ H5FD__s3comms_load_aws_creds_from_file(FILE *file, const char *profile_name, cha } /* end H5FD__s3comms_load_aws_creds_from_file() */ /*---------------------------------------------------------------------------- - * Function: H5FD_s3comms_load_aws_profile + * Function: H5FD__s3comms_load_aws_profile * * Purpose: Read AWS profile elements from ~/.aws/config and credentials * @@ -1561,8 +1561,8 @@ H5FD__s3comms_load_aws_creds_from_file(FILE *file, const char *profile_name, cha *---------------------------------------------------------------------------- */ herr_t -H5FD_s3comms_load_aws_profile(const char *profile_name, char *key_id_out, char *secret_access_key_out, - char *aws_region_out) +H5FD__s3comms_load_aws_profile(const char *profile_name, char *key_id_out, char *secret_access_key_out, + char *aws_region_out) { herr_t ret_value = SUCCEED; FILE *credfile = NULL; @@ -1570,7 +1570,7 @@ H5FD_s3comms_load_aws_profile(const char *profile_name, char *key_id_out, char * char filepath[128]; int ret = 0; - FUNC_ENTER_NOAPI_NOINIT + FUNC_ENTER_PACKAGE #ifdef H5_HAVE_WIN32_API ret = snprintf(awspath, 117, "%s/.aws/", getenv("USERPROFILE")); @@ -1627,10 +1627,10 @@ H5FD_s3comms_load_aws_profile(const char *profile_name, char *key_id_out, char * HDONE_ERROR(H5E_VFL, H5E_VFL, FAIL, "problem error-closing aws configuration file"); FUNC_LEAVE_NOAPI(ret_value) -} /* end H5FD_s3comms_load_aws_profile() */ +} /* end H5FD__s3comms_load_aws_profile() */ /*---------------------------------------------------------------------------- - * Function: H5FD_s3comms_make_aws_signing_key + * Function: H5FD__s3comms_make_aws_signing_key * * Purpose: Create AWS4 "Signing Key" from secret key, AWS region, and * timestamp @@ -1641,7 +1641,7 @@ H5FD_s3comms_load_aws_profile(const char *profile_name, char *key_id_out, char * * * `iso8601` is an ISO-8601 time string with no punctuation * (e.g.: "20170713T145903Z", so... YYYYmmdd'T'HHMMSSZ). - * This can be constructed using H5FD_s3comms_make_iso_8661_string(). + * This can be constructed using H5FD__s3comms_make_iso_8661_string(). * * Hard-coded "service" algorithm requirement to "s3" * @@ -1652,8 +1652,8 @@ H5FD_s3comms_load_aws_profile(const char *profile_name, char *key_id_out, char * *---------------------------------------------------------------------------- */ herr_t -H5FD_s3comms_make_aws_signing_key(unsigned char *md, const char *secret, const char *region, - const char *iso8601) +H5FD__s3comms_make_aws_signing_key(unsigned char *md, const char *secret, const char *region, + const char *iso8601) { char *AWS4_secret = NULL; size_t AWS4_secret_len = 0; @@ -1663,7 +1663,7 @@ H5FD_s3comms_make_aws_signing_key(unsigned char *md, const char *secret, const c int ret = 0; /* return value of snprintf */ herr_t ret_value = SUCCEED; - FUNC_ENTER_NOAPI_NOINIT + FUNC_ENTER_PACKAGE if (md == NULL) HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "Destination `md` cannot be NULL"); @@ -1706,11 +1706,11 @@ H5FD_s3comms_make_aws_signing_key(unsigned char *md, const char *secret, const c H5MM_xfree(AWS4_secret); FUNC_LEAVE_NOAPI(ret_value) -} /* end H5FD_s3comms_make_aws_signing_key() */ +} /* end H5FD__s3comms_make_aws_signing_key() */ /*---------------------------------------------------------------------------- * - * Function: H5FD_s3comms_make_aws_stringtosign() + * Function: H5FD__s3comms_make_aws_stringtosign() * * Purpose: * @@ -1742,7 +1742,7 @@ H5FD_s3comms_make_aws_signing_key(unsigned char *md, const char *secret, const c *---------------------------------------------------------------------------- */ herr_t -H5FD_s3comms_make_aws_stringtosign(char *dest, const char *req, const char *now, const char *region) +H5FD__s3comms_make_aws_stringtosign(char *dest, const char *req, const char *now, const char *region) { unsigned char checksum[S3COMMS_SHA256_HEXSTR_LENGTH]; size_t d = 0; @@ -1753,7 +1753,7 @@ H5FD_s3comms_make_aws_stringtosign(char *dest, const char *req, const char *now, herr_t ret_value = SUCCEED; char tmp[128]; - FUNC_ENTER_NOAPI_NOINIT + FUNC_ENTER_PACKAGE if (dest == NULL) HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "destination buffer cannot be NULL"); @@ -1800,6 +1800,6 @@ H5FD_s3comms_make_aws_stringtosign(char *dest, const char *req, const char *now, done: FUNC_LEAVE_NOAPI(ret_value) -} /* end H5ros3_make_aws_stringtosign() */ +} /* end H5FD__s3comms_make_aws_stringtosign() */ #endif /* H5_HAVE_ROS3_VFD */ diff --git a/src/H5FDs3comms.h b/src/H5FDs3comms.h index 0b8ffab0f0b..820c0589209 100644 --- a/src/H5FDs3comms.h +++ b/src/H5FDs3comms.h @@ -317,11 +317,11 @@ typedef struct { * * Holds persistent information for Amazon S3 requests. * - * Instantiated through `H5FD_s3comms_s3r_open()`, copies data into self. + * Instantiated through `H5FD__s3comms_s3r_open()`, copies data into self. * * Intended to be re-used for operations on a remote object. * - * Cleaned up through `H5FD_s3comms_s3r_close()`. + * Cleaned up through `H5FD__s3comms_s3r_close()`. * * _DO NOT_ share handle between threads: curl easy handle `curlhandle` has * undefined behavior if called to perform in multiple threads. @@ -395,34 +395,34 @@ extern "C" { #endif /* HTTP request buffer routines */ -H5_DLL hrb_t *H5FD_s3comms_hrb_init_request(const char *verb, const char *resource, const char *host); -H5_DLL herr_t H5FD_s3comms_hrb_destroy(hrb_t *buf); -H5_DLL herr_t H5FD_s3comms_hrb_node_set(hrb_node_t **L, const char *name, const char *value); +H5_DLL hrb_t *H5FD__s3comms_hrb_init_request(const char *verb, const char *resource, const char *host); +H5_DLL herr_t H5FD__s3comms_hrb_destroy(hrb_t *buf); +H5_DLL herr_t H5FD__s3comms_hrb_node_set(hrb_node_t **L, const char *name, const char *value); /* S3 request buffer routines */ -H5_DLL s3r_t *H5FD_s3comms_s3r_open(const char *url, const char *region, const char *id, - const uint8_t *signing_key, const char *token); -H5_DLL herr_t H5FD_s3comms_s3r_close(s3r_t *handle); -H5_DLL size_t H5FD_s3comms_s3r_get_filesize(s3r_t *handle); -H5_DLL herr_t H5FD_s3comms_s3r_read(s3r_t *handle, haddr_t offset, size_t len, void *dest); +H5_DLL s3r_t *H5FD__s3comms_s3r_open(const char *url, const char *region, const char *id, + const uint8_t *signing_key, const char *token); +H5_DLL herr_t H5FD__s3comms_s3r_close(s3r_t *handle); +H5_DLL size_t H5FD__s3comms_s3r_get_filesize(s3r_t *handle); +H5_DLL herr_t H5FD__s3comms_s3r_read(s3r_t *handle, haddr_t offset, size_t len, void *dest); /* Functions that construct AWS things */ -H5_DLL herr_t H5FD_s3comms_make_aws_canonical_request(char *canonical_request_dest, int cr_size, - char *signed_headers_dest, int sh_size, - hrb_t *http_request); -H5_DLL herr_t H5FD_s3comms_make_aws_signing_key(unsigned char *md, const char *secret, const char *region, - const char *iso8601); -H5_DLL herr_t H5FD_s3comms_make_aws_stringtosign(char *dest, const char *req_str, const char *now, - const char *region); +H5_DLL herr_t H5FD__s3comms_make_aws_canonical_request(char *canonical_request_dest, int cr_size, + char *signed_headers_dest, int sh_size, + hrb_t *http_request); +H5_DLL herr_t H5FD__s3comms_make_aws_signing_key(unsigned char *md, const char *secret, const char *region, + const char *iso8601); +H5_DLL herr_t H5FD__s3comms_make_aws_stringtosign(char *dest, const char *req_str, const char *now, + const char *region); /* Misc routines */ -H5_DLL herr_t H5FD_s3comms_make_iso_8661_string(time_t time, char iso8601[ISO8601_SIZE]); -H5_DLL herr_t H5FD_s3comms_free_purl(parsed_url_t *purl); +H5_DLL herr_t H5FD__s3comms_make_iso_8661_string(time_t time, char iso8601[ISO8601_SIZE]); +H5_DLL herr_t H5FD__s3comms_free_purl(parsed_url_t *purl); /* Testing routines */ #ifdef H5FD_S3COMMS_TESTING -H5_DLL herr_t H5FD_s3comms_load_aws_profile(const char *name, char *key_id_out, char *secret_access_key_out, - char *aws_region_out); +H5_DLL herr_t H5FD__s3comms_load_aws_profile(const char *name, char *key_id_out, char *secret_access_key_out, + char *aws_region_out); #endif /* H5FD_S3COMMS_TESTING */ #ifdef __cplusplus diff --git a/test/ros3.c b/test/ros3.c index 65f5fdf42c0..26610c26000 100644 --- a/test/ros3.c +++ b/test/ros3.c @@ -1090,8 +1090,8 @@ main(void) s3_test_aws_region[0] = '\0'; /* Attempt to load test credentials - if unable, certain tests will be skipped */ - if (SUCCEED == H5FD_s3comms_load_aws_profile(S3_TEST_PROFILE_NAME, s3_test_aws_access_key_id, - s3_test_aws_secret_access_key, s3_test_aws_region)) { + if (SUCCEED == H5FD__s3comms_load_aws_profile(S3_TEST_PROFILE_NAME, s3_test_aws_access_key_id, + s3_test_aws_secret_access_key, s3_test_aws_region)) { s3_test_credentials_loaded = 1; strncpy(restricted_access_fa.aws_region, (const char *)s3_test_aws_region, H5FD_ROS3_MAX_REGION_LEN); strncpy(restricted_access_fa.secret_id, (const char *)s3_test_aws_access_key_id, diff --git a/test/s3comms.c b/test/s3comms.c index 8da35160dda..72553dbfc3f 100644 --- a/test/s3comms.c +++ b/test/s3comms.c @@ -177,19 +177,19 @@ test_make_aws_canonical_request(void) } /* Create HTTP request object with given verb, resource/path */ - hrb = H5FD_s3comms_hrb_init_request(C->verb, C->resource, "HTTP/1.1"); + hrb = H5FD__s3comms_hrb_init_request(C->verb, C->resource, "HTTP/1.1"); assert(hrb->body == NULL); /* Create headers list from test case input */ for (int j = 0; j < C->listsize; j++) { - if (H5FD_s3comms_hrb_node_set(&node, C->list[j].name, C->list[j].value) < 0) + if (H5FD__s3comms_hrb_node_set(&node, C->list[j].name, C->list[j].value) < 0) TEST_ERROR; } hrb->first_header = node; /* Test */ - if (H5FD_s3comms_make_aws_canonical_request(cr_dest, 512, sh_dest, 64, hrb) < 0) + if (H5FD__s3comms_make_aws_canonical_request(cr_dest, 512, sh_dest, 64, hrb) < 0) TEST_ERROR; if (strncmp(C->exp_headers, sh_dest, 512)) FAIL_PUTS_ERROR("header string mismatch"); @@ -198,26 +198,26 @@ test_make_aws_canonical_request(void) /* Tear-down */ while (node != NULL) { - if (H5FD_s3comms_hrb_node_set(&node, node->name, NULL) < 0) + if (H5FD__s3comms_hrb_node_set(&node, node->name, NULL) < 0) TEST_ERROR; } - if (H5FD_s3comms_hrb_destroy(hrb) < 0) + if (H5FD__s3comms_hrb_destroy(hrb) < 0) TEST_ERROR; } /* ERROR CASES - Malformed hrb and/or node-list */ H5E_BEGIN_TRY { - ret = H5FD_s3comms_make_aws_canonical_request(cr_dest, 20, sh_dest, 20, NULL); + ret = H5FD__s3comms_make_aws_canonical_request(cr_dest, 20, sh_dest, 20, NULL); } H5E_END_TRY if (ret == SUCCEED) FAIL_PUTS_ERROR("http request object cannot be null"); - hrb = H5FD_s3comms_hrb_init_request("GET", "/", "HTTP/1.1"); + hrb = H5FD__s3comms_hrb_init_request("GET", "/", "HTTP/1.1"); H5E_BEGIN_TRY { - ret = H5FD_s3comms_make_aws_canonical_request(NULL, 20, sh_dest, 20, hrb); + ret = H5FD__s3comms_make_aws_canonical_request(NULL, 20, sh_dest, 20, hrb); } H5E_END_TRY if (ret == SUCCEED) @@ -225,13 +225,13 @@ test_make_aws_canonical_request(void) H5E_BEGIN_TRY { - ret = H5FD_s3comms_make_aws_canonical_request(cr_dest, 20, NULL, 20, hrb); + ret = H5FD__s3comms_make_aws_canonical_request(cr_dest, 20, NULL, 20, hrb); } H5E_END_TRY if (ret == SUCCEED) FAIL_PUTS_ERROR("signed headers destination cannot be null"); - if (H5FD_s3comms_hrb_destroy(hrb) < 0) + if (H5FD__s3comms_hrb_destroy(hrb) < 0) TEST_ERROR; PASSED(); @@ -241,9 +241,9 @@ test_make_aws_canonical_request(void) if (node != NULL) { while (node != NULL) - H5FD_s3comms_hrb_node_set(&node, node->name, NULL); + H5FD__s3comms_hrb_node_set(&node, node->name, NULL); } - H5FD_s3comms_hrb_destroy(hrb); + H5FD__s3comms_hrb_destroy(hrb); return 1; @@ -252,7 +252,7 @@ test_make_aws_canonical_request(void) /*--------------------------------------------------------------------------- * Function: test_hrb_init_request * - * Purpose: Define and verify behavior of `H5FD_s3comms_hrb_init_request()` + * Purpose: Define and verify behavior of `H5FD__s3comms_hrb_init_request()` * * Return: PASS : 0 * FAIL : 1 @@ -320,7 +320,7 @@ test_hrb_init_request(void) for (int i = 0; i < NCASES; i++) { struct testcase *C = &cases[i]; - req = H5FD_s3comms_hrb_init_request(C->verb, C->resource, C->version); + req = H5FD__s3comms_hrb_init_request(C->verb, C->resource, C->version); if (cases[i].ret_null == true) { if (req != NULL) @@ -347,7 +347,7 @@ test_hrb_init_request(void) TEST_ERROR; if (0 != req->body_len) TEST_ERROR; - if (H5FD_s3comms_hrb_destroy(req) < 0) + if (H5FD__s3comms_hrb_destroy(req) < 0) FAIL_PUTS_ERROR("unable to destroy hrb_t"); } } @@ -356,7 +356,7 @@ test_hrb_init_request(void) return 0; error: - H5FD_s3comms_hrb_destroy(req); + H5FD__s3comms_hrb_destroy(req); return 1; } /* end test_hrb_init_request() */ @@ -685,14 +685,14 @@ test_hrb_node_set(void) const char *name = test->given[mock_i]; const char *value = test->given[mock_i + 1]; - if (H5FD_s3comms_hrb_node_set(&list, name, value) < 0) + if (H5FD__s3comms_hrb_node_set(&list, name, value) < 0) TEST_ERROR; } /* TEST */ /* Modify list */ - if (test->returned != H5FD_s3comms_hrb_node_set(&list, test->delta.name, test->delta.value)) + if (test->returned != H5FD__s3comms_hrb_node_set(&list, test->delta.name, test->delta.value)) FAIL_PUTS_ERROR(test->message); /* Verify resulting list */ @@ -718,7 +718,7 @@ test_hrb_node_set(void) /* TEARDOWN */ while (list != NULL) { - if (H5FD_s3comms_hrb_node_set(&list, list->name, NULL) < 0) + if (H5FD__s3comms_hrb_node_set(&list, list->name, NULL) < 0) TEST_ERROR; } } @@ -728,7 +728,7 @@ test_hrb_node_set(void) error: while (list != NULL) { - H5FD_s3comms_hrb_node_set(&list, list->name, NULL); + H5FD__s3comms_hrb_node_set(&list, list->name, NULL); } return 1; @@ -739,7 +739,7 @@ test_hrb_node_set(void) /*--------------------------------------------------------------------------- * Function: test_make_aws_signing_key * - * Purpose: Verify behavior of `H5FD_s3comms_make_aws_signing_key()` + * Purpose: Verify behavior of `H5FD__s3comms_make_aws_signing_key()` * * Return: PASS : 0 * FAIL : 1 @@ -777,7 +777,7 @@ test_make_aws_signing_key(void) if (NULL == (key = (unsigned char *)malloc(sizeof(unsigned char) * SHA256_DIGEST_LENGTH))) TEST_ERROR; - if (H5FD_s3comms_make_aws_signing_key(key, cases[i].secret_key, cases[i].region, cases[i].when) < 0) + if (H5FD__s3comms_make_aws_signing_key(key, cases[i].secret_key, cases[i].region, cases[i].when) < 0) TEST_ERROR; if (strncmp((const char *)cases[i].exp, (const char *)key, SHA256_DIGEST_LENGTH)) @@ -820,7 +820,7 @@ test_make_aws_stringtosign(void) TESTING("make AWS stringtosign"); - if (H5FD_s3comms_make_aws_stringtosign(s2s, canonreq, iso8601now, region) < 0) + if (H5FD__s3comms_make_aws_stringtosign(s2s, canonreq, iso8601now, region) < 0) FAIL_PUTS_ERROR("unable to create string to sign"); if (strncmp("AWS4-HMAC-SHA256\n20130524T000000Z\n20130524/us-east-1/s3/" @@ -832,7 +832,7 @@ test_make_aws_stringtosign(void) H5E_BEGIN_TRY { - ret = H5FD_s3comms_make_aws_stringtosign(s2s, NULL, iso8601now, region); + ret = H5FD__s3comms_make_aws_stringtosign(s2s, NULL, iso8601now, region); } H5E_END_TRY if (ret == SUCCEED) @@ -840,7 +840,7 @@ test_make_aws_stringtosign(void) H5E_BEGIN_TRY { - ret = H5FD_s3comms_make_aws_stringtosign(s2s, canonreq, NULL, region); + ret = H5FD__s3comms_make_aws_stringtosign(s2s, canonreq, NULL, region); } H5E_END_TRY if (ret == SUCCEED) @@ -848,7 +848,7 @@ test_make_aws_stringtosign(void) H5E_BEGIN_TRY { - ret = H5FD_s3comms_make_aws_stringtosign(s2s, canonreq, iso8601now, NULL); + ret = H5FD__s3comms_make_aws_stringtosign(s2s, canonreq, iso8601now, NULL); } H5E_END_TRY if (ret == SUCCEED) @@ -865,7 +865,7 @@ test_make_aws_stringtosign(void) /*--------------------------------------------------------------------------- * Function: test_s3r_get_filesize * - * Purpose: Test H5FD_s3comms_s3r_get_filesize() + * Purpose: Test H5FD__s3comms_s3r_get_filesize() * * Return: PASS : 0 * FAIL : 1 @@ -891,16 +891,16 @@ test_s3r_get_filesize(void) snprintf(url_raven, S3_TEST_MAX_URL_SIZE, "%s/%s", s3_test_bucket_url, S3_TEST_RESOURCE_TEXT_PUBLIC)) TEST_ERROR; - if (0 != H5FD_s3comms_s3r_get_filesize(NULL)) + if (0 != H5FD__s3comms_s3r_get_filesize(NULL)) FAIL_PUTS_ERROR("filesize of the null handle should be 0"); - if (NULL == (handle = H5FD_s3comms_s3r_open(url_raven, NULL, NULL, NULL, NULL))) + if (NULL == (handle = H5FD__s3comms_s3r_open(url_raven, NULL, NULL, NULL, NULL))) TEST_ERROR; - if (6464 != H5FD_s3comms_s3r_get_filesize(handle)) + if (6464 != H5FD__s3comms_s3r_get_filesize(handle)) FAIL_PUTS_ERROR("incorrect file size - fragile, make sure the file size didn't change"); - if (H5FD_s3comms_s3r_close(handle) < 0) + if (H5FD__s3comms_s3r_close(handle) < 0) TEST_ERROR; PASSED(); @@ -908,14 +908,14 @@ test_s3r_get_filesize(void) error: if (handle != NULL) - H5FD_s3comms_s3r_close(handle); + H5FD__s3comms_s3r_close(handle); return 1; } /* end test_s3r_get_filesize() */ /*--------------------------------------------------------------------------- * Function: test_s3r_open * - * Purpose: Test H5FD_s3comms_s3r_open() + * Purpose: Test H5FD__s3comms_s3r_open() * * Return: PASS : 0 * FAIL : 1 @@ -963,14 +963,14 @@ test_s3r_open(void) TEST_ERROR; /* Get the current time in ISO-8601 format */ - if (H5FD_s3comms_make_iso_8661_string(time(NULL), iso8601) < 0) + if (H5FD__s3comms_make_iso_8661_string(time(NULL), iso8601) < 0) TEST_ERROR; /* It is desired to have means available to verify that signing_key * was set successfully and to an expected value. */ - if (H5FD_s3comms_make_aws_signing_key(signing_key, (const char *)s3_test_aws_secret_access_key, - (const char *)s3_test_aws_region, (const char *)iso8601) < 0) + if (H5FD__s3comms_make_aws_signing_key(signing_key, (const char *)s3_test_aws_secret_access_key, + (const char *)s3_test_aws_region, (const char *)iso8601) < 0) TEST_ERROR; /************************* @@ -980,7 +980,7 @@ test_s3r_open(void) /* Attempt anonymously */ H5E_BEGIN_TRY { - handle = H5FD_s3comms_s3r_open(url_missing, NULL, NULL, NULL, NULL); + handle = H5FD__s3comms_s3r_open(url_missing, NULL, NULL, NULL, NULL); } H5E_END_TRY if (handle != NULL) @@ -989,7 +989,7 @@ test_s3r_open(void) /* Attempt with authentication */ H5E_BEGIN_TRY { - handle = H5FD_s3comms_s3r_open( + handle = H5FD__s3comms_s3r_open( url_missing, (const char *)s3_test_aws_region, (const char *)s3_test_aws_access_key_id, (const unsigned char *)signing_key, (const char *)s3_test_aws_security_token); } @@ -1004,7 +1004,7 @@ test_s3r_open(void) /* Anonymous access on restricted file */ H5E_BEGIN_TRY { - handle = H5FD_s3comms_s3r_open(url_shakespeare, NULL, NULL, NULL, NULL); + handle = H5FD__s3comms_s3r_open(url_shakespeare, NULL, NULL, NULL, NULL); } H5E_END_TRY if (handle != NULL) @@ -1013,9 +1013,9 @@ test_s3r_open(void) /* Pass in a bad ID */ H5E_BEGIN_TRY { - handle = H5FD_s3comms_s3r_open(url_shakespeare, (const char *)s3_test_aws_region, "I_MADE_UP_MY_ID", - (const unsigned char *)signing_key, - (const char *)s3_test_aws_security_token); + handle = H5FD__s3comms_s3r_open(url_shakespeare, (const char *)s3_test_aws_region, "I_MADE_UP_MY_ID", + (const unsigned char *)signing_key, + (const char *)s3_test_aws_security_token); } H5E_END_TRY if (handle != NULL) @@ -1024,7 +1024,7 @@ test_s3r_open(void) /* Using an invalid signing key */ H5E_BEGIN_TRY { - handle = H5FD_s3comms_s3r_open( + handle = H5FD__s3comms_s3r_open( url_shakespeare, (const char *)s3_test_aws_region, (const char *)s3_test_aws_access_key_id, (const unsigned char *)EMPTY_SHA256, (const char *)s3_test_aws_security_token); } @@ -1037,36 +1037,36 @@ test_s3r_open(void) *******************************/ /* Anonymous */ - handle = H5FD_s3comms_s3r_open(url_raven, NULL, NULL, NULL, NULL); + handle = H5FD__s3comms_s3r_open(url_raven, NULL, NULL, NULL, NULL); if (handle == NULL) TEST_ERROR; - if (6464 != H5FD_s3comms_s3r_get_filesize(handle)) + if (6464 != H5FD__s3comms_s3r_get_filesize(handle)) FAIL_PUTS_ERROR("did not get expected filesize"); - if (H5FD_s3comms_s3r_close(handle) < 0) + if (H5FD__s3comms_s3r_close(handle) < 0) TEST_ERROR; handle = NULL; /* Using authentication on anonymously-accessible file? */ - handle = H5FD_s3comms_s3r_open( + handle = H5FD__s3comms_s3r_open( url_raven, (const char *)s3_test_aws_region, (const char *)s3_test_aws_access_key_id, (const unsigned char *)signing_key, (const char *)s3_test_aws_security_token); if (handle == NULL) TEST_ERROR; - if (6464 != H5FD_s3comms_s3r_get_filesize(handle)) + if (6464 != H5FD__s3comms_s3r_get_filesize(handle)) FAIL_PUTS_ERROR("did not get expected filesize"); - if (H5FD_s3comms_s3r_close(handle)) + if (H5FD__s3comms_s3r_close(handle)) TEST_ERROR; handle = NULL; /* Authenticating */ - handle = H5FD_s3comms_s3r_open( + handle = H5FD__s3comms_s3r_open( url_shakespeare, (const char *)s3_test_aws_region, (const char *)s3_test_aws_access_key_id, (const unsigned char *)signing_key, (const char *)s3_test_aws_security_token); if (handle == NULL) TEST_ERROR; - if (5458199 != H5FD_s3comms_s3r_get_filesize(handle)) + if (5458199 != H5FD__s3comms_s3r_get_filesize(handle)) FAIL_PUTS_ERROR("did not get expected filesize"); - if (H5FD_s3comms_s3r_close(handle) < 0) + if (H5FD__s3comms_s3r_close(handle) < 0) TEST_ERROR; handle = NULL; @@ -1074,7 +1074,7 @@ test_s3r_open(void) return 0; error: if (handle != NULL) - H5FD_s3comms_s3r_close(handle); + H5FD__s3comms_s3r_close(handle); return 1; } /* end test_s3r_open() */ @@ -1085,10 +1085,10 @@ test_s3r_open(void) * Purpose: Specify and demonstrate the use and life cycle of an S3 * request handle `s3r_t`, through its related functions. * - * H5FD_s3comms_s3r_open + * H5FD__s3comms_s3r_open * H5FD_s3comms_s3r_getsize << called by open() _only_ - * H5FD_s3comms_s3r_read << called by getsize(), multiple times working - * H5FD_s3comms_s3r_close + * H5FD__s3comms_s3r_read << called by getsize(), multiple times working + * H5FD__s3comms_s3r_close * * Shows most basic curl iteration * @@ -1119,10 +1119,10 @@ test_s3r_read(void) TEST_ERROR; /* Open file */ - handle = H5FD_s3comms_s3r_open(url_raven, NULL, NULL, NULL, NULL); + handle = H5FD__s3comms_s3r_open(url_raven, NULL, NULL, NULL, NULL); if (handle == NULL) TEST_ERROR; - if (6464 != H5FD_s3comms_s3r_get_filesize(handle)) + if (6464 != H5FD__s3comms_s3r_get_filesize(handle)) TEST_ERROR; /***************************** @@ -1131,7 +1131,7 @@ test_s3r_read(void) /* Read from start of file */ memset(buffer, 0, S3COMMS_READ_BUFFER_SIZE); - if (H5FD_s3comms_s3r_read(handle, (haddr_t)0, (size_t)118, buffer) < 0) + if (H5FD__s3comms_s3r_read(handle, (haddr_t)0, (size_t)118, buffer) < 0) TEST_ERROR; if (strcmp("Once upon a midnight dreary, while I pondered, weak and weary,\n" "Over many a quaint and curious volume of forgotten lore", @@ -1140,21 +1140,21 @@ test_s3r_read(void) /* Read arbitrary range */ memset(buffer, 0, S3COMMS_READ_BUFFER_SIZE); - if (H5FD_s3comms_s3r_read(handle, (haddr_t)2540, (size_t)54, buffer) < 0) + if (H5FD__s3comms_s3r_read(handle, (haddr_t)2540, (size_t)54, buffer) < 0) TEST_ERROR; if (strcmp("the grave and stern decorum of the countenance it wore", buffer)) TEST_ERROR; /* Read one character */ memset(buffer, 0, S3COMMS_READ_BUFFER_SIZE); - if (H5FD_s3comms_s3r_read(handle, (haddr_t)2540, (size_t)1, buffer) < 0) + if (H5FD__s3comms_s3r_read(handle, (haddr_t)2540, (size_t)1, buffer) < 0) TEST_ERROR; if (strcmp("t", buffer)) TEST_ERROR; /* Read to EOF */ memset(buffer, 0, S3COMMS_READ_BUFFER_SIZE); - if (H5FD_s3comms_s3r_read(handle, (haddr_t)6370, (size_t)0, buffer) < 0) + if (H5FD__s3comms_s3r_read(handle, (haddr_t)6370, (size_t)0, buffer) < 0) TEST_ERROR; if (strncmp( buffer, @@ -1170,7 +1170,7 @@ test_s3r_read(void) memset(buffer, 0, S3COMMS_READ_BUFFER_SIZE); H5E_BEGIN_TRY { - ret = H5FD_s3comms_s3r_read(handle, (haddr_t)6400, (size_t)100, /* 6400+100 > 6464 */ buffer); + ret = H5FD__s3comms_s3r_read(handle, (haddr_t)6400, (size_t)100, /* 6400+100 > 6464 */ buffer); } H5E_END_TRY if (ret == SUCCEED) @@ -1182,7 +1182,7 @@ test_s3r_read(void) memset(buffer, 0, S3COMMS_READ_BUFFER_SIZE); H5E_BEGIN_TRY { - ret = H5FD_s3comms_s3r_read(handle, (haddr_t)1200699, /* 1200699 > 6464 */ (size_t)100, buffer); + ret = H5FD__s3comms_s3r_read(handle, (haddr_t)1200699, /* 1200699 > 6464 */ (size_t)100, buffer); } H5E_END_TRY if (ret == SUCCEED) @@ -1194,7 +1194,7 @@ test_s3r_read(void) memset(buffer, 0, S3COMMS_READ_BUFFER_SIZE); H5E_BEGIN_TRY { - ret = H5FD_s3comms_s3r_read(handle, (haddr_t)6464, (size_t)0, buffer); + ret = H5FD__s3comms_s3r_read(handle, (haddr_t)6464, (size_t)0, buffer); } H5E_END_TRY if (ret == SUCCEED) @@ -1206,7 +1206,7 @@ test_s3r_read(void) * TEAR DOWN * *************/ - if (H5FD_s3comms_s3r_close(handle) < 0) + if (H5FD__s3comms_s3r_close(handle) < 0) TEST_ERROR; PASSED(); @@ -1214,7 +1214,7 @@ test_s3r_read(void) error: if (handle != NULL) - H5FD_s3comms_s3r_close(handle); + H5FD__s3comms_s3r_close(handle); return 1; } /* end test_s3r_read() */ @@ -1249,15 +1249,15 @@ main(void) s3_test_aws_region[0] = '\0'; s3_test_bucket_url[0] = '\0'; - /* TODO: unit/regression test for H5FD_s3comms_load_aws_profile() + /* TODO: unit/regression test for H5FD__s3comms_load_aws_profile() * requires a few test files and/or manipulation of default path */ /* attempt to load test credentials * if unable, certain tests will be skipped */ - if (SUCCEED == H5FD_s3comms_load_aws_profile(S3_TEST_PROFILE_NAME, s3_test_aws_access_key_id, - s3_test_aws_secret_access_key, s3_test_aws_region)) { + if (SUCCEED == H5FD__s3comms_load_aws_profile(S3_TEST_PROFILE_NAME, s3_test_aws_access_key_id, + s3_test_aws_secret_access_key, s3_test_aws_region)) { s3_test_credentials_loaded = 1; } From b89c77c21913975f32de12734ece22c5be6424a3 Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Wed, 15 Jan 2025 03:55:00 -0800 Subject: [PATCH 08/12] Move last AWS config/auth code to H5FDs3comms.c All AWS configuration and authentication logic is now centralized in H5FDs3comms.c. This will make it easier to load config data from AWS environment variables, AWS config files, and fapl parameters. --- src/H5FDros3.c | 53 +++++++++------------------- src/H5FDros3.h | 3 +- src/H5FDs3comms.c | 89 ++++++++++++++++++++++++++++++----------------- src/H5FDs3comms.h | 8 ++--- test/s3comms.c | 71 ++++++++++++++++++------------------- 5 files changed, 111 insertions(+), 113 deletions(-) diff --git a/src/H5FDros3.c b/src/H5FDros3.c index 680a2f727ae..56321730c12 100644 --- a/src/H5FDros3.c +++ b/src/H5FDros3.c @@ -702,11 +702,12 @@ H5Pset_fapl_ros3_token(hid_t fapl_id, const char *token) static H5FD_t * H5FD__ros3_open(const char *url, unsigned flags, hid_t fapl_id, haddr_t maxaddr) { - H5FD_ros3_t *file = NULL; - s3r_t *handle = NULL; - const H5FD_ros3_fapl_t *fa = NULL; - H5P_genplist_t *plist = NULL; - H5FD_t *ret_value = NULL; + H5FD_ros3_t *file = NULL; + s3r_t *handle = NULL; + const H5FD_ros3_fapl_t *fa = NULL; + H5P_genplist_t *plist = NULL; + char *fapl_token = NULL; + H5FD_t *ret_value = NULL; FUNC_ENTER_PACKAGE @@ -735,45 +736,25 @@ H5FD__ros3_open(const char *url, unsigned flags, hid_t fapl_id, haddr_t maxaddr) if (NULL == (fa = (const H5FD_ros3_fapl_t *)H5P_peek_driver_info(plist))) HGOTO_ERROR(H5E_VFL, H5E_CANTGET, NULL, "could not get ros3 VFL driver info"); - /* Open file; procedure depends on whether or not the fapl instructs to - * authenticate requests or not. - */ - if (fa->authenticate == true) { - uint8_t signing_key[SHA256_DIGEST_LENGTH]; - char iso8601[ISO8601_SIZE]; /* ISO-8601 time string */ - htri_t token_exists; /* Does the token exist in the fapl? */ - char *fapl_token = NULL; /* Token from fapl */ - const char *token = NULL; /* const pointer passed to s3r_open */ - - /* Get the current time in ISO-8601 format */ - if (H5FD__s3comms_make_iso_8661_string(time(NULL), iso8601) < 0) - HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, NULL, "could not construct ISO-8601 string"); - - /* Compute signing key (part of AWS/S3 REST API). Can be re-used by - * user/key for 7 days after creation. - */ - if (H5FD__s3comms_make_aws_signing_key(signing_key, (const char *)fa->secret_key, - (const char *)fa->aws_region, (const char *)iso8601) < 0) - HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, NULL, "problem while computing signing key"); - - /* Get the token, if it exists */ + /* Get the token, if it exists */ + if (fa->authenticate) { + htri_t token_exists; + + /* Does the token exist in the fapl? */ if ((token_exists = H5P_exist_plist(plist, ROS3_TOKEN_PROP_NAME)) < 0) HGOTO_ERROR(H5E_VFL, H5E_CANTGET, NULL, "failed check for property token in plist"); + + /* If so, get it */ if (token_exists) { if (H5P_get(plist, ROS3_TOKEN_PROP_NAME, &fapl_token) < 0) HGOTO_ERROR(H5E_VFL, H5E_CANTGET, NULL, "unable to get token value"); - token = fapl_token; } - else - token = ""; - - handle = H5FD__s3comms_s3r_open(url, (const char *)fa->aws_region, (const char *)fa->secret_id, - (const uint8_t *)signing_key, token); } - else - handle = H5FD__s3comms_s3r_open(url, NULL, NULL, NULL, NULL); - if (handle == NULL) + /* Open file; procedure depends on whether or not the fapl instructs to + * authenticate requests or not. + */ + if (NULL == (handle = H5FD__s3comms_s3r_open(url, fa, fapl_token))) HGOTO_ERROR(H5E_VFL, H5E_CANTOPENFILE, NULL, "s3r_open failed"); /* Create new file struct */ diff --git a/src/H5FDros3.h b/src/H5FDros3.h index f91f6801ced..31a91ff08bc 100644 --- a/src/H5FDros3.h +++ b/src/H5FDros3.h @@ -72,7 +72,7 @@ #define H5FD_ROS3_MAX_SECRET_TOK_LEN 4096 /** - *\struct H5FD_ros3_fapl_t + * \struct H5FD_ros3_fapl_t * \brief Configuration structure for H5Pset_fapl_ros3() / H5Pget_fapl_ros3(). * * \details H5FD_ros_fapl_t is a public structure that is used to pass @@ -100,7 +100,6 @@ * * \var char H5FD_ros3_fapl_t::secret_key[H5FD_ROS3_MAX_SECRET_KEY_LEN + 1] * A string which specifies the security key. - * */ typedef struct H5FD_ros3_fapl_t { int32_t version; diff --git a/src/H5FDs3comms.c b/src/H5FDs3comms.c index 034b325275c..86e13943445 100644 --- a/src/H5FDs3comms.c +++ b/src/H5FDs3comms.c @@ -97,6 +97,9 @@ static herr_t H5FD__s3comms_bytes_to_hex(char *dest, size_t dest_len, const unsi static herr_t H5FD__s3comms_load_aws_creds_from_file(FILE *file, const char *profile_name, char *key_id, char *access_key, char *aws_region); +static herr_t H5FD__s3comms_make_iso_8661_string(time_t time, char iso8601[ISO8601_SIZE]); +static herr_t H5FD__s3comms_free_purl(parsed_url_t *purl); + /*********************/ /* Package Variables */ /*********************/ @@ -717,16 +720,15 @@ H5FD__s3comms_s3r_getsize(s3r_t *handle) * * Purpose: Logically open a file hosted on S3 * - * To prevent AWS4 authentication, pass NULL to region, - * id, and signing_key. + * fa can be NULL (implies no authentication) + * fapl_token can be NULL * * Return: SUCCESS: Pointer to new request handle. * FAILURE: NULL *---------------------------------------------------------------------------- */ s3r_t * -H5FD__s3comms_s3r_open(const char *url, const char *region, const char *id, const uint8_t *signing_key, - const char *token) +H5FD__s3comms_s3r_open(const char *url, const H5FD_ros3_fapl_t *fa, const char *fapl_token) { CURL *curlh = NULL; s3r_t *handle = NULL; @@ -742,15 +744,20 @@ H5FD__s3comms_s3r_open(const char *url, const char *region, const char *id, cons if (url[0] == '\0') HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, NULL, "url cannot be an empty string"); - /* Parse URL */ + /************* + * PARSE URL * + *************/ if (NULL == (curlurl = curl_url())) HGOTO_ERROR(H5E_VFL, H5E_CANTCREATE, NULL, "unable to get curl url"); + if (NULL == (purl = (parsed_url_t *)H5MM_calloc(sizeof(parsed_url_t)))) + HGOTO_ERROR(H5E_VFL, H5E_CANTALLOC, NULL, "can't allocate space for parsed_url_t"); + + /* Separate the URL into parts using libcurl */ if (CURLUE_OK != curl_url_set(curlurl, CURLUPART_URL, url, 0)) HGOTO_ERROR(H5E_VFL, H5E_CANTCREATE, NULL, "unable to parse url"); - if (NULL == (purl = (parsed_url_t *)H5MM_calloc(sizeof(parsed_url_t)))) - HGOTO_ERROR(H5E_VFL, H5E_CANTALLOC, NULL, "can't allocate space for parsed_url_t"); + /* Extract the URL components using libcurl */ /* scheme */ rc = curl_url_get(curlurl, CURLUPART_SCHEME, &(purl->scheme), 0); @@ -786,34 +793,52 @@ H5FD__s3comms_s3r_open(const char *url, const char *region, const char *id, cons * RECORD AUTHENTICATION INFORMATION * *************************************/ - if ((region != NULL && *region != '\0') || (id != NULL && *id != '\0') || (signing_key != NULL) || - (token != NULL)) { + if (fa && fa->authenticate) { + uint8_t signing_key[SHA256_DIGEST_LENGTH]; + char iso8601[ISO8601_SIZE]; /* ISO-8601 time string */ - size_t tmplen; - - /* If one exists, all three must exist */ - if (region == NULL || region[0] == '\0') + /* These all need to exist to authenticate */ + if (fa->aws_region[0] == '\0') HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, NULL, "region cannot be NULL"); - if (id == NULL || id[0] == '\0') + if (fa->secret_id[0] == '\0') HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, NULL, "secret id cannot be NULL"); - if (signing_key == NULL) + if (fa->secret_key[0] == '\0') HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, NULL, "signing key cannot be NULL"); - if (token == NULL) - HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, NULL, "token cannot be NULL"); - /* Copy strings */ - if (NULL == (handle->aws_region = strdup(region))) - HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, NULL, "could not copy region"); - if (NULL == (handle->secret_id = strdup(id))) - HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, NULL, "could not copy ID"); - if (NULL == (handle->token = strdup(token))) - HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, NULL, "could not copy token"); + /* Copy strings into the s3r_t handle */ + if (NULL == (handle->aws_region = strdup(fa->aws_region))) + HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, NULL, "could not copy AWS region"); + if (NULL == (handle->secret_id = strdup(fa->secret_id))) + HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, NULL, "could not copy secret_id"); + + /* SIGNING KEY */ + + /* Get the current time in ISO-8601 format */ + if (H5FD__s3comms_make_iso_8661_string(time(NULL), iso8601) < 0) + HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, NULL, "could not construct ISO-8601 string"); + + /* Compute signing key (part of AWS/S3 REST API). Can be re-used by + * user/key for 7 days after creation. + */ + if (H5FD__s3comms_make_aws_signing_key(signing_key, (const char *)fa->secret_key, + (const char *)fa->aws_region, (const char *)iso8601) < 0) + HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, NULL, "problem while computing signing key"); /* Copy signing key (not a string) */ - tmplen = SHA256_DIGEST_LENGTH; - if (NULL == (handle->signing_key = (uint8_t *)H5MM_malloc(sizeof(uint8_t) * tmplen))) + if (NULL == (handle->signing_key = (uint8_t *)H5MM_malloc(sizeof(uint8_t) * SHA256_DIGEST_LENGTH))) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, NULL, "could not allocate space for handle key"); - H5MM_memcpy(handle->signing_key, signing_key, tmplen); + H5MM_memcpy(handle->signing_key, signing_key, SHA256_DIGEST_LENGTH); + + /* TOKEN */ + + if (fapl_token) { + if (NULL == (handle->token = strdup(fapl_token))) + HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, NULL, "could not copy token"); + } + else { + if (NULL == (handle->token = strdup(""))) + HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, NULL, "could not copy empty token"); + } } /************************ @@ -1243,7 +1268,7 @@ H5FD__s3comms_s3r_read(s3r_t *handle, haddr_t offset, size_t len, void *dest) * Return: SUCCEED/FAIL *---------------------------------------------------------------------------- */ -herr_t +static herr_t H5FD__s3comms_make_iso_8661_string(time_t time, char iso8601[ISO8601_SIZE]) { herr_t ret_value = SUCCEED; @@ -1416,7 +1441,7 @@ H5FD__s3comms_bytes_to_hex(char *dest, size_t dest_len, const unsigned char *msg * Return: SUCCEED (Can't fail - passing NULL is okay) *---------------------------------------------------------------------------- */ -herr_t +static herr_t H5FD__s3comms_free_purl(parsed_url_t *purl) { herr_t ret_value = SUCCEED; @@ -1748,10 +1773,10 @@ H5FD__s3comms_make_aws_stringtosign(char *dest, const char *req, const char *now size_t d = 0; char day[9]; char hexsum[S3COMMS_SHA256_HEXSTR_LENGTH]; - size_t i = 0; - int ret = 0; /* snprintf return value */ - herr_t ret_value = SUCCEED; + size_t i = 0; + int ret = 0; /* snprintf return value */ char tmp[128]; + herr_t ret_value = SUCCEED; FUNC_ENTER_PACKAGE diff --git a/src/H5FDs3comms.h b/src/H5FDs3comms.h index 820c0589209..93433f17054 100644 --- a/src/H5FDs3comms.h +++ b/src/H5FDs3comms.h @@ -47,6 +47,7 @@ *****************************************************************************/ #include "H5private.h" /* Generic Functions */ +#include "H5FDros3.h" /* ros3 VFD */ #ifdef H5_HAVE_ROS3_VFD @@ -400,8 +401,7 @@ H5_DLL herr_t H5FD__s3comms_hrb_destroy(hrb_t *buf); H5_DLL herr_t H5FD__s3comms_hrb_node_set(hrb_node_t **L, const char *name, const char *value); /* S3 request buffer routines */ -H5_DLL s3r_t *H5FD__s3comms_s3r_open(const char *url, const char *region, const char *id, - const uint8_t *signing_key, const char *token); +H5_DLL s3r_t *H5FD__s3comms_s3r_open(const char *url, const H5FD_ros3_fapl_t *fa, const char *fapl_token); H5_DLL herr_t H5FD__s3comms_s3r_close(s3r_t *handle); H5_DLL size_t H5FD__s3comms_s3r_get_filesize(s3r_t *handle); H5_DLL herr_t H5FD__s3comms_s3r_read(s3r_t *handle, haddr_t offset, size_t len, void *dest); @@ -415,10 +415,6 @@ H5_DLL herr_t H5FD__s3comms_make_aws_signing_key(unsigned char *md, const char * H5_DLL herr_t H5FD__s3comms_make_aws_stringtosign(char *dest, const char *req_str, const char *now, const char *region); -/* Misc routines */ -H5_DLL herr_t H5FD__s3comms_make_iso_8661_string(time_t time, char iso8601[ISO8601_SIZE]); -H5_DLL herr_t H5FD__s3comms_free_purl(parsed_url_t *purl); - /* Testing routines */ #ifdef H5FD_S3COMMS_TESTING H5_DLL herr_t H5FD__s3comms_load_aws_profile(const char *name, char *key_id_out, char *secret_access_key_out, diff --git a/test/s3comms.c b/test/s3comms.c index 72553dbfc3f..806250a21b4 100644 --- a/test/s3comms.c +++ b/test/s3comms.c @@ -894,7 +894,7 @@ test_s3r_get_filesize(void) if (0 != H5FD__s3comms_s3r_get_filesize(NULL)) FAIL_PUTS_ERROR("filesize of the null handle should be 0"); - if (NULL == (handle = H5FD__s3comms_s3r_open(url_raven, NULL, NULL, NULL, NULL))) + if (NULL == (handle = H5FD__s3comms_s3r_open(url_raven, NULL, NULL))) TEST_ERROR; if (6464 != H5FD__s3comms_s3r_get_filesize(handle)) @@ -924,12 +924,11 @@ test_s3r_get_filesize(void) static int test_s3r_open(void) { - char iso8601[ISO8601_SIZE]; /* ISO-8601 time string */ - char url_missing[S3_TEST_MAX_URL_SIZE]; - char url_raven[S3_TEST_MAX_URL_SIZE]; - char url_shakespeare[S3_TEST_MAX_URL_SIZE]; - unsigned char signing_key[SHA256_DIGEST_LENGTH]; - s3r_t *handle = NULL; + char url_missing[S3_TEST_MAX_URL_SIZE]; + char url_raven[S3_TEST_MAX_URL_SIZE]; + char url_shakespeare[S3_TEST_MAX_URL_SIZE]; + H5FD_ros3_fapl_t *fa = NULL; + s3r_t *handle = NULL; TESTING("s3r_open"); @@ -950,6 +949,18 @@ test_s3r_open(void) * PRE-TEST SETUP * ******************/ + /* Create and fill a common fapl + * + * Specific fields will be set (and reset) as needed by tests below + */ + if (NULL == (fa = (H5FD_ros3_fapl_t *)calloc(1, sizeof(H5FD_ros3_fapl_t)))) + TEST_ERROR; + fa->version = H5FD_CURR_ROS3_FAPL_T_VERSION; + fa->authenticate = true; + strcpy(fa->aws_region, s3_test_aws_region); + strcpy(fa->secret_id, s3_test_aws_access_key_id); + strcpy(fa->secret_key, s3_test_aws_secret_access_key); + if (S3_TEST_MAX_URL_SIZE < snprintf(url_shakespeare, S3_TEST_MAX_URL_SIZE, "%s/%s", s3_test_bucket_url, S3_TEST_RESOURCE_TEXT_RESTRICTED)) TEST_ERROR; @@ -962,17 +973,6 @@ test_s3r_open(void) snprintf(url_raven, S3_TEST_MAX_URL_SIZE, "%s/%s", s3_test_bucket_url, S3_TEST_RESOURCE_TEXT_PUBLIC)) TEST_ERROR; - /* Get the current time in ISO-8601 format */ - if (H5FD__s3comms_make_iso_8661_string(time(NULL), iso8601) < 0) - TEST_ERROR; - - /* It is desired to have means available to verify that signing_key - * was set successfully and to an expected value. - */ - if (H5FD__s3comms_make_aws_signing_key(signing_key, (const char *)s3_test_aws_secret_access_key, - (const char *)s3_test_aws_region, (const char *)iso8601) < 0) - TEST_ERROR; - /************************* * OPEN NONEXISTENT FILE * *************************/ @@ -980,7 +980,7 @@ test_s3r_open(void) /* Attempt anonymously */ H5E_BEGIN_TRY { - handle = H5FD__s3comms_s3r_open(url_missing, NULL, NULL, NULL, NULL); + handle = H5FD__s3comms_s3r_open(url_missing, NULL, NULL); } H5E_END_TRY if (handle != NULL) @@ -989,9 +989,7 @@ test_s3r_open(void) /* Attempt with authentication */ H5E_BEGIN_TRY { - handle = H5FD__s3comms_s3r_open( - url_missing, (const char *)s3_test_aws_region, (const char *)s3_test_aws_access_key_id, - (const unsigned char *)signing_key, (const char *)s3_test_aws_security_token); + handle = H5FD__s3comms_s3r_open(url_missing, fa, (const char *)s3_test_aws_security_token); } H5E_END_TRY if (handle != NULL) @@ -1004,40 +1002,40 @@ test_s3r_open(void) /* Anonymous access on restricted file */ H5E_BEGIN_TRY { - handle = H5FD__s3comms_s3r_open(url_shakespeare, NULL, NULL, NULL, NULL); + handle = H5FD__s3comms_s3r_open(url_shakespeare, NULL, NULL); } H5E_END_TRY if (handle != NULL) TEST_ERROR; /* Pass in a bad ID */ + strcpy(fa->secret_id, "I_MADE_UP_MY_ID"); H5E_BEGIN_TRY { - handle = H5FD__s3comms_s3r_open(url_shakespeare, (const char *)s3_test_aws_region, "I_MADE_UP_MY_ID", - (const unsigned char *)signing_key, - (const char *)s3_test_aws_security_token); + handle = H5FD__s3comms_s3r_open(url_shakespeare, fa, (const char *)s3_test_aws_security_token); } H5E_END_TRY if (handle != NULL) TEST_ERROR; + strcpy(fa->secret_id, s3_test_aws_access_key_id); /* Using an invalid signing key */ + strcpy(fa->secret_key, "I_AM_A_FAKE_KEY"); H5E_BEGIN_TRY { - handle = H5FD__s3comms_s3r_open( - url_shakespeare, (const char *)s3_test_aws_region, (const char *)s3_test_aws_access_key_id, - (const unsigned char *)EMPTY_SHA256, (const char *)s3_test_aws_security_token); + handle = H5FD__s3comms_s3r_open(url_shakespeare, fa, (const char *)s3_test_aws_security_token); } H5E_END_TRY if (handle != NULL) TEST_ERROR; + strcpy(fa->secret_key, s3_test_aws_secret_access_key); /******************************* * SUCCESSFUL OPEN (AND CLOSE) * *******************************/ /* Anonymous */ - handle = H5FD__s3comms_s3r_open(url_raven, NULL, NULL, NULL, NULL); + handle = H5FD__s3comms_s3r_open(url_raven, NULL, NULL); if (handle == NULL) TEST_ERROR; if (6464 != H5FD__s3comms_s3r_get_filesize(handle)) @@ -1047,9 +1045,7 @@ test_s3r_open(void) handle = NULL; /* Using authentication on anonymously-accessible file? */ - handle = H5FD__s3comms_s3r_open( - url_raven, (const char *)s3_test_aws_region, (const char *)s3_test_aws_access_key_id, - (const unsigned char *)signing_key, (const char *)s3_test_aws_security_token); + handle = H5FD__s3comms_s3r_open(url_raven, fa, (const char *)s3_test_aws_security_token); if (handle == NULL) TEST_ERROR; if (6464 != H5FD__s3comms_s3r_get_filesize(handle)) @@ -1059,9 +1055,7 @@ test_s3r_open(void) handle = NULL; /* Authenticating */ - handle = H5FD__s3comms_s3r_open( - url_shakespeare, (const char *)s3_test_aws_region, (const char *)s3_test_aws_access_key_id, - (const unsigned char *)signing_key, (const char *)s3_test_aws_security_token); + handle = H5FD__s3comms_s3r_open(url_shakespeare, fa, (const char *)s3_test_aws_security_token); if (handle == NULL) TEST_ERROR; if (5458199 != H5FD__s3comms_s3r_get_filesize(handle)) @@ -1070,11 +1064,14 @@ test_s3r_open(void) TEST_ERROR; handle = NULL; + free(fa); + PASSED(); return 0; error: if (handle != NULL) H5FD__s3comms_s3r_close(handle); + free(fa); return 1; } /* end test_s3r_open() */ @@ -1119,7 +1116,7 @@ test_s3r_read(void) TEST_ERROR; /* Open file */ - handle = H5FD__s3comms_s3r_open(url_raven, NULL, NULL, NULL, NULL); + handle = H5FD__s3comms_s3r_open(url_raven, NULL, NULL); if (handle == NULL) TEST_ERROR; if (6464 != H5FD__s3comms_s3r_get_filesize(handle)) From 96e81452f533c07f3841ab4183c16d0f7e83aeaa Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Wed, 15 Jan 2025 04:12:36 -0800 Subject: [PATCH 09/12] Address misc warnings --- src/H5FDs3comms.c | 4 ++-- test/ros3.c | 1 + test/s3comms.c | 1 + 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/H5FDs3comms.c b/src/H5FDs3comms.c index 86e13943445..53c731f09f7 100644 --- a/src/H5FDs3comms.c +++ b/src/H5FDs3comms.c @@ -633,7 +633,7 @@ H5FD__s3comms_s3r_getsize(s3r_t *handle) /* Set handle and curlhandle to perform an HTTP HEAD request on file */ curlh = handle->curlhandle; - if (CURLE_OK != curl_easy_setopt(curlh, CURLOPT_NOBODY, 1L)) + if (CURLE_OK != curl_easy_setopt(curlh, CURLOPT_NOBODY, 1)) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "error while setting CURL option (CURLOPT_NOBODY)"); if (CURLE_OK != curl_easy_setopt(curlh, CURLOPT_HEADERDATA, &sds)) @@ -701,7 +701,7 @@ H5FD__s3comms_s3r_getsize(s3r_t *handle) * UNDO HEAD SETTINGS * **********************/ - if (CURLE_OK != curl_easy_setopt(curlh, CURLOPT_NOBODY, NULL)) + if (CURLE_OK != curl_easy_setopt(curlh, CURLOPT_NOBODY, 0)) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "error while setting CURL option (CURLOPT_NOBODY)"); /* Unset HTTP HEAD settings from curl handle, returning to initial state */ diff --git a/test/ros3.c b/test/ros3.c index 26610c26000..d503fd672a1 100644 --- a/test/ros3.c +++ b/test/ros3.c @@ -1052,6 +1052,7 @@ main(void) } else { strncpy(s3_test_bucket_url, bucket_url_env, S3_TEST_MAX_URL_SIZE); + s3_test_bucket_url[S3_TEST_MAX_URL_SIZE - 1] = '\0'; s3_test_bucket_defined = true; } diff --git a/test/s3comms.c b/test/s3comms.c index 806250a21b4..860b380f68b 100644 --- a/test/s3comms.c +++ b/test/s3comms.c @@ -1265,6 +1265,7 @@ main(void) } else { strncpy(s3_test_bucket_url, bucket_url_env, S3_TEST_MAX_URL_SIZE); + s3_test_bucket_url[S3_TEST_MAX_URL_SIZE - 1] = '\0'; s3_test_bucket_defined = true; } From b8e8b7f44b2d58cb98f13543e900c986a5a1580f Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 15 Jan 2025 12:15:33 +0000 Subject: [PATCH 10/12] Committing clang-format changes --- test/ros3.c | 2 +- test/s3comms.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/ros3.c b/test/ros3.c index d503fd672a1..9b21828255e 100644 --- a/test/ros3.c +++ b/test/ros3.c @@ -1053,7 +1053,7 @@ main(void) else { strncpy(s3_test_bucket_url, bucket_url_env, S3_TEST_MAX_URL_SIZE); s3_test_bucket_url[S3_TEST_MAX_URL_SIZE - 1] = '\0'; - s3_test_bucket_defined = true; + s3_test_bucket_defined = true; } if (S3_TEST_MAX_URL_SIZE < snprintf(url_text_restricted, (size_t)S3_TEST_MAX_URL_SIZE, "%s/%s", diff --git a/test/s3comms.c b/test/s3comms.c index 860b380f68b..5192bf9bf7a 100644 --- a/test/s3comms.c +++ b/test/s3comms.c @@ -1266,7 +1266,7 @@ main(void) else { strncpy(s3_test_bucket_url, bucket_url_env, S3_TEST_MAX_URL_SIZE); s3_test_bucket_url[S3_TEST_MAX_URL_SIZE - 1] = '\0'; - s3_test_bucket_defined = true; + s3_test_bucket_defined = true; } curl_global_init(CURL_GLOBAL_DEFAULT); From 04a6f61d9915bcda2cfa1d0084dc2c511846a5f0 Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Wed, 15 Jan 2025 04:54:52 -0800 Subject: [PATCH 11/12] Move URL parsing to a new function --- src/H5FDs3comms.c | 136 +++++++++++++++++++++++++++++----------------- 1 file changed, 85 insertions(+), 51 deletions(-) diff --git a/src/H5FDs3comms.c b/src/H5FDs3comms.c index 53c731f09f7..25706e05c52 100644 --- a/src/H5FDs3comms.c +++ b/src/H5FDs3comms.c @@ -98,6 +98,9 @@ static herr_t H5FD__s3comms_load_aws_creds_from_file(FILE *file, const char *pro char *access_key, char *aws_region); static herr_t H5FD__s3comms_make_iso_8661_string(time_t time, char iso8601[ISO8601_SIZE]); + +static parsed_url_t *H5FD__s3comms_parse_url(const char *url); + static herr_t H5FD__s3comms_free_purl(parsed_url_t *purl); /*********************/ @@ -572,7 +575,6 @@ H5FD__s3comms_s3r_close(s3r_t *handle) if (H5FD__s3comms_free_purl(handle->purl) < 0) HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "unable to release parsed url structure"); - H5MM_xfree(handle->purl); H5MM_xfree(handle); @@ -730,12 +732,9 @@ H5FD__s3comms_s3r_getsize(s3r_t *handle) s3r_t * H5FD__s3comms_s3r_open(const char *url, const H5FD_ros3_fapl_t *fa, const char *fapl_token) { - CURL *curlh = NULL; - s3r_t *handle = NULL; - parsed_url_t *purl = NULL; - CURLU *curlurl = NULL; - CURLUcode rc; - s3r_t *ret_value = NULL; + CURL *curlh = NULL; + s3r_t *handle = NULL; + s3r_t *ret_value = NULL; FUNC_ENTER_PACKAGE @@ -744,51 +743,17 @@ H5FD__s3comms_s3r_open(const char *url, const H5FD_ros3_fapl_t *fa, const char * if (url[0] == '\0') HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, NULL, "url cannot be an empty string"); - /************* - * PARSE URL * - *************/ - - if (NULL == (curlurl = curl_url())) - HGOTO_ERROR(H5E_VFL, H5E_CANTCREATE, NULL, "unable to get curl url"); - if (NULL == (purl = (parsed_url_t *)H5MM_calloc(sizeof(parsed_url_t)))) - HGOTO_ERROR(H5E_VFL, H5E_CANTALLOC, NULL, "can't allocate space for parsed_url_t"); - - /* Separate the URL into parts using libcurl */ - if (CURLUE_OK != curl_url_set(curlurl, CURLUPART_URL, url, 0)) - HGOTO_ERROR(H5E_VFL, H5E_CANTCREATE, NULL, "unable to parse url"); - - /* Extract the URL components using libcurl */ - - /* scheme */ - rc = curl_url_get(curlurl, CURLUPART_SCHEME, &(purl->scheme), 0); - if (CURLUE_OK != rc) - HGOTO_ERROR(H5E_VFL, H5E_CANTCREATE, NULL, "unable to get url scheme"); - /* host */ - rc = curl_url_get(curlurl, CURLUPART_HOST, &(purl->host), 0); - if (CURLUE_OK != rc) - HGOTO_ERROR(H5E_VFL, H5E_CANTCREATE, NULL, "unable to get url host"); - /* port - okay to not exist */ - rc = curl_url_get(curlurl, CURLUPART_PORT, &(purl->port), 0); - if (CURLUE_OK != rc && CURLUE_NO_PORT != rc) - HGOTO_ERROR(H5E_VFL, H5E_CANTCREATE, NULL, "unable to get url port"); - /* path */ - rc = curl_url_get(curlurl, CURLUPART_PATH, &(purl->path), 0); - if (CURLUE_OK != rc) - HGOTO_ERROR(H5E_VFL, H5E_CANTCREATE, NULL, "unable to get url path"); - /* query - okay to not exist */ - rc = curl_url_get(curlurl, CURLUPART_QUERY, &(purl->query), 0); - if (CURLUE_OK != rc && CURLUE_NO_QUERY != rc) - HGOTO_ERROR(H5E_VFL, H5E_CANTCREATE, NULL, "unable to get url query"); - /* Create handle and set fields */ if (NULL == (handle = (s3r_t *)H5MM_calloc(sizeof(s3r_t)))) HGOTO_ERROR(H5E_VFL, H5E_CANTALLOC, NULL, "could not allocate space for handle"); - handle->purl = purl; - if (NULL == (handle->http_verb = (char *)H5MM_calloc(sizeof(char) * 16))) HGOTO_ERROR(H5E_VFL, H5E_CANTALLOC, NULL, "unable to allocate space for S3 request HTTP verb"); + /* Parse URL */ + if (NULL == (handle->purl = H5FD__s3comms_parse_url(url))) + HGOTO_ERROR(H5E_VFL, H5E_CANTALLOC, NULL, "could not allocate and create parsed URL"); + /************************************* * RECORD AUTHENTICATION INFORMATION * *************************************/ @@ -879,16 +844,13 @@ H5FD__s3comms_s3r_open(const char *url, const H5FD_ros3_fapl_t *fa, const char * ret_value = handle; done: - curl_url_cleanup(curlurl); - if (ret_value == NULL) { curl_easy_cleanup(curlh); - if (H5FD__s3comms_free_purl(purl) < 0) - HDONE_ERROR(H5E_VFL, H5E_BADVALUE, NULL, "unable to free parsed url structure"); - H5MM_xfree(purl); - if (handle != NULL) { + if (H5FD__s3comms_free_purl(handle->purl) < 0) + HDONE_ERROR(H5E_VFL, H5E_CANTFREE, NULL, "unable to free parsed url structure"); + H5MM_xfree(handle->aws_region); H5MM_xfree(handle->secret_id); H5MM_xfree(handle->signing_key); @@ -1433,6 +1395,76 @@ H5FD__s3comms_bytes_to_hex(char *dest, size_t dest_len, const unsigned char *msg FUNC_LEAVE_NOAPI(ret_value) } /* end H5FD__s3comms_bytes_to_hex() */ +/*---------------------------------------------------------------------------- + * Function: H5FD__s3comms_parse_url + * + * Purpose: Release resources from a parsed_url_t pointer + * + * Return: Success: A pointer to a parsed_url_t + * Failure: NULL + *---------------------------------------------------------------------------- + */ +static parsed_url_t * +H5FD__s3comms_parse_url(const char *url) +{ + CURLUcode rc; + CURLU *curlurl = NULL; + parsed_url_t *purl = NULL; + parsed_url_t *ret_value = NULL; + + FUNC_ENTER_PACKAGE + + assert(url); + + /* Get a curl URL handle */ + if (NULL == (curlurl = curl_url())) + HGOTO_ERROR(H5E_VFL, H5E_CANTCREATE, NULL, "unable to get curl url"); + + /* Separate the URL into parts using libcurl */ + if (CURLUE_OK != curl_url_set(curlurl, CURLUPART_URL, url, 0)) + HGOTO_ERROR(H5E_VFL, H5E_CANTCREATE, NULL, "unable to parse url"); + + /* Allocate memory for the retrned parsed URL */ + if (NULL == (purl = (parsed_url_t *)H5MM_calloc(sizeof(parsed_url_t)))) + HGOTO_ERROR(H5E_VFL, H5E_CANTALLOC, NULL, "can't allocate space for parsed_url_t"); + + /* Extract the URL components using libcurl */ + + /* scheme */ + rc = curl_url_get(curlurl, CURLUPART_SCHEME, &(purl->scheme), 0); + if (CURLUE_OK != rc) + HGOTO_ERROR(H5E_VFL, H5E_CANTCREATE, NULL, "unable to get url scheme"); + /* host */ + rc = curl_url_get(curlurl, CURLUPART_HOST, &(purl->host), 0); + if (CURLUE_OK != rc) + HGOTO_ERROR(H5E_VFL, H5E_CANTCREATE, NULL, "unable to get url host"); + /* port - okay to not exist */ + rc = curl_url_get(curlurl, CURLUPART_PORT, &(purl->port), 0); + if (CURLUE_OK != rc && CURLUE_NO_PORT != rc) + HGOTO_ERROR(H5E_VFL, H5E_CANTCREATE, NULL, "unable to get url port"); + /* path */ + rc = curl_url_get(curlurl, CURLUPART_PATH, &(purl->path), 0); + if (CURLUE_OK != rc) + HGOTO_ERROR(H5E_VFL, H5E_CANTCREATE, NULL, "unable to get url path"); + /* query - okay to not exist */ + rc = curl_url_get(curlurl, CURLUPART_QUERY, &(purl->query), 0); + if (CURLUE_OK != rc && CURLUE_NO_QUERY != rc) + HGOTO_ERROR(H5E_VFL, H5E_CANTCREATE, NULL, "unable to get url query"); + + ret_value = purl; + +done: + curl_url_cleanup(curlurl); + + if (ret_value == NULL) { + if (H5FD__s3comms_free_purl(purl) < 0) + HDONE_ERROR(H5E_VFL, H5E_BADVALUE, NULL, "unable to free parsed url structure"); + H5MM_xfree(purl); + } + + FUNC_LEAVE_NOAPI(ret_value) +} /* end H5FD__s3comms_parse_url() */ + /*---------------------------------------------------------------------------- * Function: H5FD__s3comms_free_purl * @@ -1457,6 +1489,8 @@ H5FD__s3comms_free_purl(parsed_url_t *purl) curl_free(purl->path); curl_free(purl->query); + H5MM_xfree(purl); + done: FUNC_LEAVE_NOAPI(ret_value) } /* end H5FD__s3comms_free_purl() */ From 0db652ea73f9e12f77271da499608f98c3558505 Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Wed, 15 Jan 2025 05:13:33 -0800 Subject: [PATCH 12/12] Fix typo --- src/H5FDs3comms.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/H5FDs3comms.c b/src/H5FDs3comms.c index 25706e05c52..11c94cb68e0 100644 --- a/src/H5FDs3comms.c +++ b/src/H5FDs3comms.c @@ -1424,7 +1424,7 @@ H5FD__s3comms_parse_url(const char *url) if (CURLUE_OK != curl_url_set(curlurl, CURLUPART_URL, url, 0)) HGOTO_ERROR(H5E_VFL, H5E_CANTCREATE, NULL, "unable to parse url"); - /* Allocate memory for the retrned parsed URL */ + /* Allocate memory for the parsed URL to return */ if (NULL == (purl = (parsed_url_t *)H5MM_calloc(sizeof(parsed_url_t)))) HGOTO_ERROR(H5E_VFL, H5E_CANTALLOC, NULL, "can't allocate space for parsed_url_t");