From bd7616cf98ae52e77f710a16fb4d334be3cb5ce9 Mon Sep 17 00:00:00 2001 From: jhendersonHDF Date: Wed, 26 Apr 2023 17:57:22 -0500 Subject: Fix v1 object header gap bug in H5Ocopy (#2785) --- release_docs/RELEASE.txt | 12 +++++++++ src/H5Ocopy.c | 9 +++++-- test/objcopy.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 2 deletions(-) diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index 773dbde..ad51ff1 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -160,6 +160,18 @@ Bug Fixes since HDF5-1.14.0 release =================================== Library ------- + - Fixed a bug in H5Ocopy that could generate invalid HDF5 files + + H5Ocopy was missing a check to determine whether the new object's + object header version is greater than version 1. Without this check, + copying of objects with object headers that are smaller than a + certain size would cause H5Ocopy to create an object header for the + new object that has a gap in the header data. According to the + HDF5 File Format Specification, this is not allowed for version + 1 of the object header format. + + Fixes GitHub issue #2653 + - Fixed H5Pget_vol_cap_flags and H5Pget_vol_id to accept H5P_DEFAULT H5Pget_vol_cap_flags and H5Pget_vol_id were updated to correctly diff --git a/src/H5Ocopy.c b/src/H5Ocopy.c index 926d0da..9852d1f 100644 --- a/src/H5Ocopy.c +++ b/src/H5Ocopy.c @@ -532,10 +532,15 @@ H5O__copy_header_real(const H5O_loc_t *oloc_src, H5O_loc_t *oloc_dst /*out*/, H5 HDassert((oh_dst->flags & H5O_HDR_CHUNK0_SIZE) == H5O_HDR_CHUNK0_1); /* Determine whether to create gap or NULL message */ - if (delta < H5O_SIZEOF_MSGHDR_OH(oh_dst)) + if ((oh_dst->version > H5O_VERSION_1) && (delta < H5O_SIZEOF_MSGHDR_OH(oh_dst))) dst_oh_gap = delta; - else + else { + /* NULL message must be at least size of message header */ + if (delta < H5O_SIZEOF_MSGHDR_OH(oh_dst)) + delta = H5O_SIZEOF_MSGHDR_OH(oh_dst); + dst_oh_null = delta; + } /* Increase destination object header size */ dst_oh_size += delta; diff --git a/test/objcopy.c b/test/objcopy.c index 012c81d..eac99e0 100644 --- a/test/objcopy.c +++ b/test/objcopy.c @@ -16397,6 +16397,68 @@ error: return (H5_ITER_ERROR); } /* end test_copy_iterate_cb */ +/* + * Test for a bug with copying of v1 object headers where the + * new object header would end up with a gap in the header data, + * which v1 object header shouldn't have. + */ +static int +test_copy_cdt_v1_header_bug(hid_t fcpl_src, hid_t src_fapl) +{ + hid_t file_id = H5I_INVALID_HID; + hid_t type_id = H5I_INVALID_HID; + hid_t ocpypl_id = H5I_INVALID_HID; + char src_filename[NAME_BUF_SIZE]; + + TESTING("H5Ocopy(): bug with copying v1 object headers"); + + /* Initialize the filenames */ + h5_fixname(FILENAME[0], src_fapl, src_filename, sizeof src_filename); + + if ((file_id = H5Fcreate(src_filename, H5F_ACC_TRUNC, fcpl_src, src_fapl)) < 0) + TEST_ERROR; + + if ((type_id = H5Tcreate(H5T_STRING, 385)) < 0) + TEST_ERROR; + if (H5Tset_strpad(type_id, H5T_STR_NULLPAD) < 0) + TEST_ERROR; + if (H5Tset_cset(type_id, H5T_CSET_ASCII) < 0) + TEST_ERROR; + + if (H5Tcommit2(file_id, "committed_str_type", type_id, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT) < 0) + TEST_ERROR; + + if ((ocpypl_id = H5Pcreate(H5P_OBJECT_COPY)) < 0) + TEST_ERROR; + if (H5Pset_copy_object(ocpypl_id, H5O_COPY_WITHOUT_ATTR_FLAG) < 0) + TEST_ERROR; + + if (H5Ocopy(file_id, "committed_str_type", file_id, "committed_str_type2", ocpypl_id, H5P_DEFAULT) < 0) + TEST_ERROR; + + if (H5Tclose(type_id) < 0) + TEST_ERROR; + if (H5Pclose(ocpypl_id) < 0) + TEST_ERROR; + if (H5Fclose(file_id) < 0) + TEST_ERROR; + + PASSED(); + + return 0; + +error: + H5E_BEGIN_TRY + { + H5Tclose(type_id); + H5Pclose(ocpypl_id); + H5Fclose(file_id); + } + H5E_END_TRY; + + return 1; +} + static int test_copy_iterate(hid_t fcpl_src, hid_t fcpl_dst, hid_t src_fapl, hid_t dst_fapl) { @@ -17577,6 +17639,8 @@ main(void) nerrors += test_copy_null_ref(fcpl_src, fcpl_dst, src_fapl, dst_fapl); nerrors += test_copy_null_ref_open(fcpl_src, fcpl_dst, src_fapl, dst_fapl); + nerrors += test_copy_cdt_v1_header_bug(fcpl_src, src_fapl); + nerrors += test_copy_iterate(fcpl_src, fcpl_dst, src_fapl, dst_fapl); } /* end if */ -- cgit v0.12