Skip to content

Commit 6b2694c

Browse files
brtnfldbrtnfld
committed
HL: Fix prefix-match bugs in H5TB field lookup and H5DS/H5IM CLASS checks
- H5TB: strcmp replaces strncmp in H5TBfind_field so that field names that are a prefix of a requested name (or vice-versa) are no longer matched. HLTB_MAX_FIELD_LEN (255) is now public in H5TBpublic.h and exposed to Fortran as HLTB_MAX_FIELD_LEN_F in H5TBff.F90. H5TBget_field_info documents the buffer-size requirement and truncation behaviour. H5TBget_field_info guards against overflow on long names. - H5IM: H5IMis_image and H5IMis_palette refactored into a shared helper (H5IM__class_attr_equals). The helper now reads both fixed-length and variable-length CLASS string attributes, using H5Treclaim for VL memory. strcmp replaces strncmp for exact-match semantics. - H5DS: H5DSis_scale and H5DS_is_reserved both support variable-length CLASS string attributes via H5Aget_space/H5Aread/H5Treclaim. The fixed-length path retains the 16-byte size guard. strcmp is used throughout for exact comparison. - Tests: new test functions test_is_scale_class_prefix, test_is_reserved_class_prefix, and test_class_prefix cover fixed-length prefix/exact/wrong-value cases and variable-length string cases. test_table.c adds write and read field-name prefix rejection cases and boundary-length truncation verification. All malloc calls are NULL-checked. - CHANGELOG updated with a summary of all fixes.
1 parent 013e9cc commit 6b2694c

11 files changed

Lines changed: 887 additions & 203 deletions

File tree

‎hl/fortran/src/H5TBff.F90‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,12 @@ MODULE H5TB_CONST
4242
USE h5fortran_types
4343
USE hdf5
4444

45+
!> Maximum length of a field name buffer returned by h5tbget_field_info_f.
46+
!! Matches HLTB_MAX_FIELD_LEN defined in H5TBpublic.h.
47+
!! Declare field_names character arrays with at least this length:
48+
!! CHARACTER(LEN=HLTB_MAX_FIELD_LEN_F) :: field_names(nfields)
49+
INTEGER, PARAMETER :: HLTB_MAX_FIELD_LEN_F = 255
50+
4551
INTERFACE h5tbwrite_field_name_f
4652
#ifdef H5_DOXYGEN
4753
MODULE PROCEDURE h5tbwrite_field_name_f
@@ -209,6 +215,8 @@ END FUNCTION h5tbinsert_field_c
209215
!! \param nrecords The number of records.
210216
!! \param type_size The size in bytes of the structure associated with the table. Obtained with sizeof or storage_size.
211217
!! \param field_names An array containing the names of the fields.
218+
!! Names longer than HLTB_MAX_FIELD_LEN_F - 1 characters
219+
!! are silently truncated when read back by h5tbget_field_info_f().
212220
!! \param field_offset An array containing the offsets of the fields.
213221
!! \param field_types An array containing the type of the fields.
214222
!! \param chunk_size The chunk size.
@@ -325,7 +333,9 @@ END SUBROUTINE h5tbmake_table_f90
325333
!! \param nfields The number of fields
326334
!! \param nrecords The number of records
327335
!! \param type_size The size in bytes of the structure associated with the table; This value is obtained with sizeof().
328-
!! \param field_names An array containing the names of the fields
336+
!! \param field_names An array containing the names of the fields.
337+
!! Names longer than HLTB_MAX_FIELD_LEN_F - 1 characters
338+
!! are silently truncated when read back by h5tbget_field_info_f().
329339
!! \param field_offset An array containing the offsets of the fields
330340
!! \param field_types An array containing the type of the fields
331341
!! \param chunk_size The chunk size
@@ -1057,10 +1067,15 @@ END SUBROUTINE h5tbget_table_info_f
10571067
!! \param loc_id Location identifier. The identifier may be that of a file or group.
10581068
!! \param dset_name The name of the dataset to read.
10591069
!! \param nfields The number of fields.
1060-
!! \param field_names An array containing the names of the fields.
1070+
!! \param field_names An array of character buffers to receive the field names.
1071+
!! Each element must be declared with at least HLTB_MAX_FIELD_LEN_F
1072+
!! characters. Field names longer than HLTB_MAX_FIELD_LEN_F - 1
1073+
!! characters are silently truncated. A truncated name may
1074+
!! inadvertently match a different, shorter field when subsequently
1075+
!! passed to h5tbread_fields_name_f() or h5tbwrite_fields_name_f().
10611076
!! \param field_sizes An array containing the size of the fields.
1062-
!! \param field_offsets An array containing the offsets of the fields.
1063-
!! \param type_size The size of the HDF5 datatype associated with the table
1077+
!! \param field_offsets An array containing the offsets of the fields.
1078+
!! \param type_size The size of the HDF5 datatype associated with the table
10641079
!! (i.e., the size in bytes of the HDF5 compound datatype used to define a row, or record, in the table).
10651080
!! \param errcode \fortran_error
10661081
!! \param maxlen_out Maximum character length of the field names.

‎hl/src/H5DS.c‎

Lines changed: 99 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -2221,6 +2221,7 @@ H5DSis_scale(hid_t did)
22212221
hid_t aid = H5I_INVALID_HID; /* attribute ID */
22222222
htri_t attr_class; /* has the "CLASS" attribute */
22232223
htri_t is_ds = -1; /* set to "not a dimension scale" */
2224+
htri_t isvl; /* result of H5Tis_variable_str */
22242225
H5I_type_t it; /* type of identifier */
22252226
char *buf = NULL; /* buffer to read name of attribute */
22262227
size_t string_size; /* size of storage for the attribute */
@@ -2261,54 +2262,71 @@ H5DSis_scale(hid_t did)
22612262
is_ds = 0;
22622263
goto out;
22632264
}
2264-
/* check to make sure string is null-terminated;
2265-
if not, then it is not dimension scale */
2266-
if ((strpad = H5Tget_strpad(tid)) < 0)
2265+
if ((isvl = H5Tis_variable_str(tid)) < 0)
22672266
goto out;
2268-
if (H5T_STR_NULLTERM != strpad) {
2269-
is_ds = 0;
2270-
goto out;
2271-
}
22722267

2273-
/* According to Spec string is ASCII and its size should be 16 to hold
2274-
"DIMENSION_SCALE" string */
2275-
if ((string_size = H5Tget_size(tid)) == 0)
2276-
goto out;
2277-
if (string_size != 16) {
2278-
is_ds = 0;
2279-
goto out;
2280-
}
2268+
if (isvl > 0) {
2269+
/* Variable-length string: H5Aread allocates the string and writes
2270+
* its address into the buffer; use H5Treclaim to free it after use. */
2271+
char *vl_str = NULL;
2272+
hid_t asid;
22812273

2282-
buf = (char *)malloc((size_t)string_size * sizeof(char));
2283-
if (buf == NULL)
2284-
goto out;
2285-
2286-
/* Read the attribute */
2287-
if (H5Aread(aid, tid, buf) < 0)
2288-
goto out;
2274+
if ((asid = H5Aget_space(aid)) < 0)
2275+
goto out;
2276+
if (H5Aread(aid, tid, &vl_str) < 0) {
2277+
H5Sclose(asid);
2278+
goto out;
2279+
}
2280+
is_ds = (vl_str && strcmp(vl_str, DIMENSION_SCALE_CLASS) == 0) ? 1 : 0;
2281+
H5Treclaim(tid, asid, H5P_DEFAULT, &vl_str);
2282+
H5Sclose(asid);
2283+
}
2284+
else {
2285+
/* check to make sure string is null-terminated;
2286+
if not, then it is not dimension scale */
2287+
if ((strpad = H5Tget_strpad(tid)) < 0)
2288+
goto out;
2289+
if (H5T_STR_NULLTERM != strpad) {
2290+
is_ds = 0;
2291+
goto out;
2292+
}
22892293

2290-
/* compare strings */
2291-
if (strncmp(buf, DIMENSION_SCALE_CLASS, MIN(strlen(DIMENSION_SCALE_CLASS), strlen(buf))) == 0)
2292-
is_ds = 1;
2294+
/* According to Spec string is ASCII and its size should be 16 to hold
2295+
"DIMENSION_SCALE" string */
2296+
if ((string_size = H5Tget_size(tid)) == 0)
2297+
goto out;
2298+
if (string_size != 16) {
2299+
is_ds = 0;
2300+
goto out;
2301+
}
22932302

2294-
free(buf);
2303+
/* Allocate one extra byte and force null termination so strcmp never
2304+
* reads past the buffer even if some tool wrote the attribute without
2305+
* honoring the H5T_STR_NULLTERM invariant. */
2306+
buf = (char *)malloc((size_t)string_size * sizeof(char) + 1);
2307+
if (buf == NULL)
2308+
goto out;
22952309

2296-
if (H5Tclose(tid) < 0)
2297-
goto out;
2310+
/* Read the attribute */
2311+
if (H5Aread(aid, tid, buf) < 0)
2312+
goto out;
2313+
buf[string_size] = '\0';
22982314

2299-
if (H5Aclose(aid) < 0)
2300-
goto out;
2315+
/* compare strings */
2316+
if (strcmp(buf, DIMENSION_SCALE_CLASS) == 0)
2317+
is_ds = 1;
2318+
else
2319+
is_ds = 0;
2320+
}
23012321
}
23022322
out:
2303-
if (is_ds < 0) {
2304-
free(buf);
2305-
H5E_BEGIN_TRY
2306-
{
2307-
H5Aclose(aid);
2308-
H5Tclose(tid);
2309-
}
2310-
H5E_END_TRY
2323+
free(buf);
2324+
H5E_BEGIN_TRY
2325+
{
2326+
H5Aclose(aid);
2327+
H5Tclose(tid);
23112328
}
2329+
H5E_END_TRY
23122330
return is_ds;
23132331
}
23142332

@@ -2441,6 +2459,7 @@ static herr_t
24412459
H5DS_is_reserved(hid_t did, bool *is_reserved)
24422460
{
24432461
htri_t has_class;
2462+
htri_t isvl;
24442463
hid_t tid = H5I_INVALID_HID;
24452464
hid_t aid = H5I_INVALID_HID;
24462465
char *buf = NULL; /* Name of attribute */
@@ -2463,28 +2482,53 @@ H5DS_is_reserved(hid_t did, bool *is_reserved)
24632482
if (H5T_STRING != H5Tget_class(tid))
24642483
goto error;
24652484

2466-
/* Check to make sure string is null-terminated */
2467-
if (H5T_STR_NULLTERM != H5Tget_strpad(tid))
2485+
if ((isvl = H5Tis_variable_str(tid)) < 0)
24682486
goto error;
24692487

2470-
/* Allocate buffer large enough to hold string */
2471-
if ((string_size = H5Tget_size(tid)) == 0)
2472-
goto error;
2473-
if (NULL == (buf = malloc(string_size * sizeof(char))))
2474-
goto error;
2488+
if (isvl > 0) {
2489+
/* Variable-length string: H5Aread allocates the string and writes
2490+
* its address into the buffer; use H5Treclaim to free it after use. */
2491+
char *vl_str = NULL;
2492+
hid_t asid;
2493+
2494+
if ((asid = H5Aget_space(aid)) < 0)
2495+
goto error;
2496+
if (H5Aread(aid, tid, &vl_str) < 0) {
2497+
H5Sclose(asid);
2498+
goto error;
2499+
}
2500+
*is_reserved = vl_str && (strcmp(vl_str, IMAGE_CLASS) == 0 || strcmp(vl_str, PALETTE_CLASS) == 0 ||
2501+
strcmp(vl_str, TABLE_CLASS) == 0);
2502+
H5Treclaim(tid, asid, H5P_DEFAULT, &vl_str);
2503+
H5Sclose(asid);
2504+
}
2505+
else {
2506+
/* Check to make sure string is null-terminated */
2507+
if (H5T_STR_NULLTERM != H5Tget_strpad(tid))
2508+
goto error;
24752509

2476-
/* Read the attribute */
2477-
if (H5Aread(aid, tid, buf) < 0)
2478-
goto error;
2510+
/* Allocate buffer large enough to hold string */
2511+
if ((string_size = H5Tget_size(tid)) == 0)
2512+
goto error;
2513+
/* Allocate one extra byte and force null termination so strcmp never
2514+
* reads past the buffer even if some tool wrote the attribute without
2515+
* honoring the H5T_STR_NULLTERM invariant. */
2516+
if (NULL == (buf = malloc(string_size * sizeof(char) + 1)))
2517+
goto error;
24792518

2480-
if (strncmp(buf, IMAGE_CLASS, MIN(strlen(IMAGE_CLASS), strlen(buf))) == 0 ||
2481-
strncmp(buf, PALETTE_CLASS, MIN(strlen(PALETTE_CLASS), strlen(buf))) == 0 ||
2482-
strncmp(buf, TABLE_CLASS, MIN(strlen(TABLE_CLASS), strlen(buf))) == 0)
2483-
*is_reserved = true;
2484-
else
2485-
*is_reserved = false;
2519+
/* Read the attribute */
2520+
if (H5Aread(aid, tid, buf) < 0)
2521+
goto error;
2522+
buf[string_size] = '\0';
24862523

2487-
free(buf);
2524+
if (strcmp(buf, IMAGE_CLASS) == 0 || strcmp(buf, PALETTE_CLASS) == 0 || strcmp(buf, TABLE_CLASS) == 0)
2525+
*is_reserved = true;
2526+
else
2527+
*is_reserved = false;
2528+
2529+
free(buf);
2530+
buf = NULL;
2531+
}
24882532

24892533
if (H5Tclose(tid) < 0)
24902534
goto error;

0 commit comments

Comments
 (0)