Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Detect invalid ID to H5Gmove2 #4765

Merged
merged 7 commits into from
Sep 6, 2024
Merged

Detect invalid ID to H5Gmove2 #4765

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
215ce16
Detect invalid ID to H5Gmove2
bmribler Aug 26, 2024
9ce80a7
Committing clang-format changes
github-actions[bot] Aug 26, 2024
0db6cc4
Replace non-VOL calls with VOL calls - part 5
bmribler Aug 29, 2024
50674dd
Merge remote-tracking branch 'origin/fix_GH-4730_part5' into fix_GH-4737
bmribler Aug 29, 2024
5f5a559
Fix how H5L_SAME_LOC is handled.
bmribler Aug 31, 2024
f0c507d
Committing clang-format changes
github-actions[bot] Aug 31, 2024
ab98633
Merge branch 'develop' into fix_GH-4737
bmribler Sep 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 33 additions & 10 deletions src/H5Gdeprec.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -519,10 +519,35 @@ H5Gmove2(hid_t src_loc_id, const char *src_name, hid_t dst_loc_id, const char *d
H5VL_loc_params_t loc_params1;
H5VL_object_t *vol_obj2 = NULL; /* Object of dst_id */
H5VL_loc_params_t loc_params2;
H5I_type_t src_id_type = H5I_BADID, dst_id_type = H5I_BADID;
herr_t ret_value = SUCCEED; /* Return value */

FUNC_ENTER_API(FAIL)

/* Check arguments */
if (!src_name || !*src_name)
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "no current name specified");
if (!dst_name || !*dst_name)
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "no destination name specified");

/* src and dst location IDs cannot both have the value of H5L_SAME_LOC */
if (src_loc_id == H5L_SAME_LOC && dst_loc_id == H5L_SAME_LOC)
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "current and destination should not both be H5L_SAME_LOC");

/* reset an ID in the case of H5L_SAME_LOC */
if (src_loc_id == H5L_SAME_LOC)
src_loc_id = dst_loc_id;
else if (dst_loc_id == H5L_SAME_LOC)
dst_loc_id = src_loc_id;

src_id_type = H5I_get_type(src_loc_id);
if (!(H5I_GROUP == src_id_type || H5I_FILE == src_id_type))
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "invalid group (or file) ID, src_loc_id");

dst_id_type = H5I_get_type(dst_loc_id);
if (!(H5I_GROUP == dst_id_type || H5I_FILE == dst_id_type))
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "invalid group (or file) ID, dst_loc_id");

/* Set up collective metadata if appropriate */
if (H5CX_set_loc(dst_loc_id) < 0)
HGOTO_ERROR(H5E_SYM, H5E_CANTSET, FAIL, "can't set collective metadata read info");
Expand All @@ -531,22 +556,20 @@ H5Gmove2(hid_t src_loc_id, const char *src_name, hid_t dst_loc_id, const char *d
loc_params1.type = H5VL_OBJECT_BY_NAME;
loc_params1.loc_data.loc_by_name.name = src_name;
loc_params1.loc_data.loc_by_name.lapl_id = H5P_LINK_ACCESS_DEFAULT;
loc_params1.obj_type = H5I_get_type(src_loc_id);
loc_params1.obj_type = src_id_type;

/* Set location parameter for destination object */
loc_params2.type = H5VL_OBJECT_BY_NAME;
loc_params2.loc_data.loc_by_name.name = dst_name;
loc_params2.loc_data.loc_by_name.lapl_id = H5P_LINK_ACCESS_DEFAULT;
loc_params2.obj_type = H5I_get_type(dst_loc_id);
loc_params2.obj_type = dst_id_type;

if (H5L_SAME_LOC != src_loc_id)
/* get the location object */
if (NULL == (vol_obj1 = H5VL_vol_object(src_loc_id)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid location identifier");
if (H5L_SAME_LOC != dst_loc_id)
/* get the location object */
if (NULL == (vol_obj2 = H5VL_vol_object(dst_loc_id)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid location identifier");
/* get the location object */
if (NULL == (vol_obj1 = H5VL_vol_object(src_loc_id)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid location identifier");
/* get the location object */
if (NULL == (vol_obj2 = H5VL_vol_object(dst_loc_id)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid location identifier");

/* Move the link */
if (H5VL_link_move(vol_obj1, &loc_params1, vol_obj2, &loc_params2, H5P_LINK_CREATE_DEFAULT,
Expand Down
74 changes: 50 additions & 24 deletions src/H5L.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ H5Lmove(hid_t src_loc_id, const char *src_name, hid_t dst_loc_id, const char *ds
H5VL_object_t *vol_obj2 = NULL; /* Object of dst_id */
H5VL_loc_params_t loc_params1;
H5VL_loc_params_t loc_params2;
H5VL_object_t tmp_vol_obj; /* Temporary object */
H5VL_object_t tmp_vol_obj; /* Temporary object */
H5I_type_t src_id_type = H5I_BADID, dst_id_type = H5I_BADID;
herr_t ret_value = SUCCEED; /* Return value */

FUNC_ENTER_API(FAIL)
Expand All @@ -106,6 +107,21 @@ H5Lmove(hid_t src_loc_id, const char *src_name, hid_t dst_loc_id, const char *ds
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "no current name specified");
if (!dst_name || !*dst_name)
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "no destination name specified");

/* reset an ID in the case of H5L_SAME_LOC */
if (src_loc_id == H5L_SAME_LOC)
src_loc_id = dst_loc_id;
else if (dst_loc_id == H5L_SAME_LOC)
dst_loc_id = src_loc_id;

/* verify that src and dst IDs are either a file or a group ID */
src_id_type = H5I_get_type(src_loc_id);
if (!(H5I_GROUP == src_id_type || H5I_FILE == src_id_type))
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "invalid group (or file) ID, src_loc_id");
dst_id_type = H5I_get_type(dst_loc_id);
if (!(H5I_GROUP == dst_id_type || H5I_FILE == dst_id_type))
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "invalid group (or file) ID, dst_loc_id");

if (lcpl_id != H5P_DEFAULT && (true != H5P_isa_class(lcpl_id, H5P_LINK_CREATE)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "not a link creation property list");

Expand All @@ -117,30 +133,27 @@ H5Lmove(hid_t src_loc_id, const char *src_name, hid_t dst_loc_id, const char *ds
H5CX_set_lcpl(lcpl_id);

/* Verify access property list and set up collective metadata if appropriate */
if (H5CX_set_apl(&lapl_id, H5P_CLS_LACC, ((src_loc_id != H5L_SAME_LOC) ? src_loc_id : dst_loc_id), true) <
0)
if (H5CX_set_apl(&lapl_id, H5P_CLS_LACC, dst_loc_id, true) < 0)
HGOTO_ERROR(H5E_LINK, H5E_CANTSET, FAIL, "can't set access property list info");

/* Set location parameter for source object */
loc_params1.type = H5VL_OBJECT_BY_NAME;
loc_params1.loc_data.loc_by_name.name = src_name;
loc_params1.loc_data.loc_by_name.lapl_id = lapl_id;
loc_params1.obj_type = H5I_get_type(src_loc_id);
loc_params1.obj_type = src_id_type;

/* Set location parameter for destination object */
loc_params2.type = H5VL_OBJECT_BY_NAME;
loc_params2.loc_data.loc_by_name.name = dst_name;
loc_params2.loc_data.loc_by_name.lapl_id = lapl_id;
loc_params2.obj_type = H5I_get_type(dst_loc_id);
loc_params2.obj_type = dst_id_type;

if (H5L_SAME_LOC != src_loc_id)
/* Get the location object */
if (NULL == (vol_obj1 = H5VL_vol_object(src_loc_id)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid location identifier");
if (H5L_SAME_LOC != dst_loc_id)
/* Get the location object */
if (NULL == (vol_obj2 = H5VL_vol_object(dst_loc_id)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid location identifier");
/* Get the location object */
if (NULL == (vol_obj1 = H5VL_vol_object(src_loc_id)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid location identifier");
/* Get the location object */
if (NULL == (vol_obj2 = H5VL_vol_object(dst_loc_id)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid location identifier");

/* Make sure that the VOL connectors are the same */
if (vol_obj1 && vol_obj2) {
Expand Down Expand Up @@ -195,7 +208,8 @@ H5Lcopy(hid_t src_loc_id, const char *src_name, hid_t dst_loc_id, const char *ds
H5VL_loc_params_t loc_params1;
H5VL_object_t *vol_obj2 = NULL; /* Object of dst_id */
H5VL_loc_params_t loc_params2;
H5VL_object_t tmp_vol_obj; /* Temporary object */
H5VL_object_t tmp_vol_obj; /* Temporary object */
H5I_type_t src_id_type = H5I_BADID, dst_id_type = H5I_BADID;
herr_t ret_value = SUCCEED; /* Return value */

FUNC_ENTER_API(FAIL)
Expand All @@ -210,6 +224,20 @@ H5Lcopy(hid_t src_loc_id, const char *src_name, hid_t dst_loc_id, const char *ds
if (lcpl_id != H5P_DEFAULT && (true != H5P_isa_class(lcpl_id, H5P_LINK_CREATE)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "not a link creation property list");

/* reset an ID in the case of H5L_SAME_LOC */
if (src_loc_id == H5L_SAME_LOC)
src_loc_id = dst_loc_id;
else if (dst_loc_id == H5L_SAME_LOC)
dst_loc_id = src_loc_id;

/* verify that src and dst IDs are either a file or a group ID */
src_id_type = H5I_get_type(src_loc_id);
if (!(H5I_GROUP == src_id_type || H5I_FILE == src_id_type) && src_loc_id != H5L_SAME_LOC)
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "invalid group (or file) ID, src_loc_id");
dst_id_type = H5I_get_type(dst_loc_id);
if (!(H5I_GROUP == dst_id_type || H5I_FILE == dst_id_type) && dst_loc_id != H5L_SAME_LOC)
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "invalid group (or file) ID, dst_loc_id");

/* Check the link create property list */
if (H5P_DEFAULT == lcpl_id)
lcpl_id = H5P_LINK_CREATE_DEFAULT;
Expand All @@ -226,22 +254,20 @@ H5Lcopy(hid_t src_loc_id, const char *src_name, hid_t dst_loc_id, const char *ds
loc_params1.type = H5VL_OBJECT_BY_NAME;
loc_params1.loc_data.loc_by_name.name = src_name;
loc_params1.loc_data.loc_by_name.lapl_id = lapl_id;
loc_params1.obj_type = H5I_get_type(src_loc_id);
loc_params1.obj_type = src_id_type;

/* Set location parameter for destination object */
loc_params2.type = H5VL_OBJECT_BY_NAME;
loc_params2.loc_data.loc_by_name.name = dst_name;
loc_params2.loc_data.loc_by_name.lapl_id = lapl_id;
loc_params2.obj_type = H5I_get_type(dst_loc_id);
loc_params2.obj_type = dst_id_type;

if (H5L_SAME_LOC != src_loc_id)
/* Get the location object */
if (NULL == (vol_obj1 = H5VL_vol_object(src_loc_id)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid location identifier");
if (H5L_SAME_LOC != dst_loc_id)
/* Get the location object */
if (NULL == (vol_obj2 = H5VL_vol_object(dst_loc_id)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid location identifier");
/* Get the location object */
if (NULL == (vol_obj1 = H5VL_vol_object(src_loc_id)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid location identifier");
/* Get the location object */
if (NULL == (vol_obj2 = H5VL_vol_object(dst_loc_id)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid location identifier");

/* Make sure that the VOL connectors are the same */
if (vol_obj1 && vol_obj2) {
Expand Down
4 changes: 4 additions & 0 deletions src/H5VLcallback.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5100,6 +5100,10 @@ H5VL_link_move(const H5VL_object_t *src_vol_obj, const H5VL_loc_params_t *loc_pa

FUNC_ENTER_NOAPI(FAIL)

/* Sanity check */
assert(src_vol_obj);
assert(src_vol_obj->data);

/* Set wrapper info in API context */
vol_obj = (src_vol_obj->data ? src_vol_obj : dst_vol_obj);
if (H5VL_set_vol_wrapper(vol_obj) < 0)
Expand Down
53 changes: 48 additions & 5 deletions test/links.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1938,12 +1938,14 @@ test_move_preserves(hid_t fapl_id, bool new_format)
*-------------------------------------------------------------------------
*/
#ifndef H5_NO_DEPRECATED_SYMBOLS
#define NUM_OBJS 3 /* number of groups in FILENAME[0] file */
static int
test_deprec(hid_t fapl, bool new_format)
{
hid_t file_id = H5I_INVALID_HID;
hid_t group1_id = H5I_INVALID_HID;
hid_t group2_id = H5I_INVALID_HID;
hid_t group3_id = H5I_INVALID_HID;
H5G_stat_t sb_hard1, sb_hard2, sb_soft1, sb_soft2;
H5G_obj_t obj_type; /* Object type */
hsize_t num_objs; /* Number of objects in a group */
Expand All @@ -1967,6 +1969,8 @@ test_deprec(hid_t fapl, bool new_format)
FAIL_STACK_ERROR;
if ((group2_id = H5Gcreate2(file_id, "group2", H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT)) < 0)
FAIL_STACK_ERROR;
if ((group3_id = H5Gcreate2(file_id, "group3", H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT)) < 0)
FAIL_STACK_ERROR;

/* Test H5Gset and get comment */

Expand Down Expand Up @@ -2022,7 +2026,7 @@ test_deprec(hid_t fapl, bool new_format)
/* Test getting the number of objects in a group */
if (H5Gget_num_objs(file_id, &num_objs) < 0)
FAIL_STACK_ERROR;
if (num_objs != 2)
if (num_objs != NUM_OBJS)
TEST_ERROR;
if (H5Gget_num_objs(group1_id, &num_objs) < 0)
FAIL_STACK_ERROR;
Expand Down Expand Up @@ -2113,9 +2117,43 @@ test_deprec(hid_t fapl, bool new_format)

/* Test H5Gmove and H5Gmove2 */
if (H5Gmove(file_id, "group1", "moved_group1") < 0)
FAIL_STACK_ERROR;
TEST_ERROR;
if (H5Gmove2(file_id, "group2", group1_id, "moved_group2") < 0)
FAIL_STACK_ERROR;
TEST_ERROR;
if (H5Gmove2(file_id, "group3", H5L_SAME_LOC, "moved_group3") < 0)
TEST_ERROR;
if (H5Gmove2(file_id, "moved_group3", group2_id, "moved_group3_to_group2") < 0)
TEST_ERROR;

/* Test H5Gmove2 with H5L_SAME_LOC */
if (H5Gmove2(group2_id, "moved_group3_to_group2", H5L_SAME_LOC, "group3_same_loc") < 0)
TEST_ERROR;

/* Test H5Gmove2 with H5L_SAME_LOC */
if (H5Gmove2(H5L_SAME_LOC, "moved_group1/moved_group2", file_id, "moved_group2_again") < 0)
TEST_ERROR;

/* Put back moved_group2 for subsequent tests */
if (H5Gmove2(file_id, "moved_group2_again", file_id, "moved_group1/moved_group2") < 0)
TEST_ERROR;

/* Test passing in invalid ID */
H5E_BEGIN_TRY
{
hid_t bad_id = H5I_BADID;
if (H5Gmove2(bad_id, "group2", group1_id, "moved_group2") >= 0)
TEST_ERROR;
}
H5E_END_TRY

/* Test passing in invalid ID */
H5E_BEGIN_TRY
{
hid_t bad_id = H5I_BADID;
if (H5Gmove2(file_id, "group2", bad_id, "moved_group2") >= 0)
TEST_ERROR;
}
H5E_END_TRY

/* Ensure that both groups can be opened */
if (H5Gclose(group2_id) < 0)
Expand All @@ -2129,6 +2167,8 @@ test_deprec(hid_t fapl, bool new_format)
FAIL_STACK_ERROR;

/* Close open IDs */
if (H5Gclose(group3_id) < 0)
FAIL_STACK_ERROR;
if (H5Gclose(group2_id) < 0)
FAIL_STACK_ERROR;
if (H5Gclose(group1_id) < 0)
Expand All @@ -2154,6 +2194,7 @@ test_deprec(hid_t fapl, bool new_format)
error:
H5E_BEGIN_TRY
{
H5Gclose(group3_id);
H5Gclose(group2_id);
H5Gclose(group1_id);
H5Fclose(file_id);
Expand Down Expand Up @@ -3293,7 +3334,8 @@ external_link_closing_deprec(hid_t fapl, bool new_format)
/* Test copy (as of this test, it uses the same code as move) */
if (H5Lcopy(fid1, "elink/elink/elink", fid1, "elink/elink/elink_copied", H5P_DEFAULT, H5P_DEFAULT) < 0)
FAIL_STACK_ERROR;
if (H5Lcopy(fid1, "elink/elink/elink", fid1, "elink/elink/elink/elink_copied2", H5P_DEFAULT,
/* Also exercise H5L_SAME_LOC */
if (H5Lcopy(H5L_SAME_LOC, "elink/elink/elink", fid1, "elink/elink/elink/elink_copied2", H5P_DEFAULT,
H5P_DEFAULT) < 0)
FAIL_STACK_ERROR;

Expand Down Expand Up @@ -4325,7 +4367,8 @@ lapl_nlinks_deprec(hid_t fapl, bool new_format)
*/
if (H5Lcopy(fid, "soft17", fid, "soft17/newer_soft", H5P_DEFAULT, plist) < 0)
TEST_ERROR;
if (H5Lmove(fid, "soft17/newer_soft", fid, "soft17/newest_soft", H5P_DEFAULT, plist) < 0)
/* Also exercise H5L_SAME_LOC */
if (H5Lmove(fid, "soft17/newer_soft", H5L_SAME_LOC, "soft17/newest_soft", H5P_DEFAULT, plist) < 0)
TEST_ERROR;

/* H5Olink */
Expand Down
Loading