From 1fe2ee7de07b75b3dd0e52f38511f2a3a475efd0 Mon Sep 17 00:00:00 2001 From: Scot Breitenfeld Date: Thu, 20 Aug 2026 11:11:25 -0500 Subject: [PATCH] Canonicalize the persisted parameter string in pline v3 H5Pappend_filter previously stored the caller's raw parameter string verbatim. The filter profile's accepted grammar is a superset of TOML v1.0.0 in two respects -- optional outer braces, and C99 hex-float literals -- so a raw payload is not guaranteed to parse with a stock TOML parser. That defeats the point of persisting the string: readers that are not the HDF5 library at all (pure-reimplementation readers such as jHDF or pyfive, decoding the object header directly) are exactly the audience a stored string is meant to serve without a plugin. Add H5Z_canonicalize_params(), which strips optional outer braces and rewrites hex-float literals to %.17e decimal (bit-exact, reusing H5Z__rewrite_hexfloats). H5Pappend_filter now persists this canonical form instead of the raw string, so H5O_PLINE_EXT_CONFIG bytes are always valid TOML v1.0.0 and valid set_config input. Interior spacing, quote style, key case, and key order are unaffected. Also corrects the H5Z__rewrite_hexfloats doc comment, which claimed %.17g (the RFC-forbidden format that drops the decimal point for whole numbers); the implementation already used %.17e correctly. --- src/H5Pocpl.c | 20 +++- src/H5Zconfig.c | 94 ++++++++++++++- src/H5Zprivate.h | 4 + test/tfilter2.c | 295 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 407 insertions(+), 6 deletions(-) diff --git a/src/H5Pocpl.c b/src/H5Pocpl.c index 447c27f5a69..c297157a84c 100644 --- a/src/H5Pocpl.c +++ b/src/H5Pocpl.c @@ -1830,7 +1830,8 @@ H5Pappend_filter(hid_t plist_id, H5Z_filter_t filter, unsigned int flags, const const unsigned *cd_values = NULL; unsigned *allocated_cd_values = NULL; /* owns heap mem for string path */ char *fi_name_heap = NULL; /* non-NULL if fi->name was H5MM_strdup'd here */ - const char *retain_config = NULL; /* verbatim string to persist (STRING path only) */ + const char *retain_config = NULL; /* canonical string to persist (STRING path only) */ + char *canon_config = NULL; /* owns the buffer retain_config points into */ char *fi_config_heap = NULL; /* non-NULL if fi->config was H5MM_strdup'd here */ size_t cd_nelmts = 0; herr_t ret_value = SUCCEED; @@ -1900,11 +1901,19 @@ H5Pappend_filter(hid_t plist_id, H5Z_filter_t filter, unsigned int flags, const "filter does not support string configuration (no set_config callback)"); } - /* Persist the caller's verbatim parameter string so it can be - * recovered losslessly (pipeline v3) without loading the plugin. + /* Persist the caller's parameter string so it can be recovered + * losslessly (pipeline v3) without loading the plugin. The string is + * canonicalised first -- outer braces stripped, hex-float literals + * rewritten to %.17e decimal -- so the persisted bytes are a valid + * TOML v1.0.0 document and can be parsed by readers that are not the + * HDF5 library. Both normalisations preserve the value exactly. * An empty input stores nothing. */ - if (!empty_input) - retain_config = param_str; + if (!empty_input) { + if (NULL == (canon_config = H5Z_canonicalize_params(param_str))) + HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, + "can't canonicalize filter parameter string"); + retain_config = canon_config; + } /* set_config is present: invoke it. Normalise an empty input to * params = NULL so callbacks only need to handle one form. */ @@ -2011,6 +2020,7 @@ done: H5MM_xfree(fi_name_heap); H5MM_xfree(fi_config_heap); } + H5MM_xfree(canon_config); H5MM_xfree(allocated_cd_values); FUNC_LEAVE_API(ret_value) } /* end H5Pappend_filter() */ diff --git a/src/H5Zconfig.c b/src/H5Zconfig.c index cceb3f50e49..a0f7906a5fb 100644 --- a/src/H5Zconfig.c +++ b/src/H5Zconfig.c @@ -66,7 +66,11 @@ H5Z__copy_chars2(char *out, size_t cap, size_t *pos, const char **p) /* * H5Z__rewrite_hexfloats - return a copy of `src` with every C99 hex-float * literal (e.g. "0x1.8p+1", "-0x1p-1") replaced by an equivalent decimal - * string. Uses %.17g which guarantees IEEE 754 double round-trip fidelity. + * string. Uses %.17e, which always carries a decimal point and an exponent + * (so tomlc17 types it TOML_FP64, not TOML_INTEGER) and which guarantees + * IEEE 754 double round-trip fidelity (C99 DBL_DECIMAL_DIG == 17). %.17g + * must not be substituted: it drops the decimal point for whole values + * ("8.0" -> "8"), which a TOML parser reads as an integer. * * This pre-processing step lets callers produce parameter strings with `%a` * for exact float encoding without @@ -266,6 +270,94 @@ H5Z__toml_wrap(const char *params) return buf; } +/*------------------------------------------------------------------------- + * Function: H5Z_canonicalize_params + * + * Purpose: Return a heap copy of PARAMS in the canonical form used for + * on-disk storage (filter pipeline version 3). + * + * Two value-preserving normalisations are applied: + * + * 1. Optional outer braces, and surrounding whitespace, are + * removed: both "{level = 6}" and "level = 6" store as + * "level = 6". This mirrors the acceptance rule in + * H5Z__toml_wrap(). + * 2. C99 hex-float literals are rewritten to %.17e decimal, + * which is bit-exact for IEEE 754 doubles. + * + * The point of both is that the stored bytes are a valid TOML + * v1.0.0 document, which neither the braced form nor a + * hex-float literal is. The stored string is read by tools + * that are not the HDF5 library -- pure-reimplementation + * readers such as jHDF and pyfive parse the object header + * directly and use a stock TOML parser -- and for those a + * braced or hex-float payload is a hard parse error. Keeping + * the canonical form a strict subset of TOML is what makes the + * persisted string useful to them. See RFC-HDFG-2026-001 + * sec:pline-v3. + * + * Everything else is preserved byte-for-byte: interior + * spacing, quote style, key case, and key order. Values are + * never re-serialised, so no decimal-precision rounding is + * introduced (the reason a full parser-normalised + * re-serialisation was rejected). + * + * Return: Success: Heap-allocated NUL-terminated string; the caller + * frees it with H5MM_xfree(). + * Failure: NULL + *------------------------------------------------------------------------- + */ +char * +H5Z_canonicalize_params(const char *params) +{ + char *expanded = NULL; + char *ret_value = NULL; + const char *p; + const char *e; + size_t len; + + FUNC_ENTER_NOAPI_NOINIT_NOERR + + if (params == NULL) + HGOTO_DONE(NULL); + + /* Rewrite hex-float literals first. The rewriter skips quoted strings + * and comments, so it cannot disturb the brace characters examined + * below, nor rewrite hex-float-looking text inside a string value. */ + if (NULL == (expanded = H5Z__rewrite_hexfloats(params))) + HGOTO_DONE(NULL); + + /* Strip optional outer braces, then trim whitespace at both ends. */ + p = expanded; + while (*p == ' ' || *p == '\t') + p++; + if (*p == '{') { + p++; + e = p + strlen(p); + while (e > p && (*(e - 1) == ' ' || *(e - 1) == '\t')) + e--; + if (e > p && *(e - 1) == '}') + e--; + } + else + e = p + strlen(p); + + while (p < e && (*p == ' ' || *p == '\t')) + p++; + while (e > p && (*(e - 1) == ' ' || *(e - 1) == '\t')) + e--; + + len = (size_t)(e - p); + if (NULL != (ret_value = (char *)H5MM_malloc(len + 1))) { + H5MM_memcpy(ret_value, p, len); + ret_value[len] = '\0'; + } + +done: + H5MM_xfree(expanded); + FUNC_LEAVE_NOAPI(ret_value) +} /* end H5Z_canonicalize_params() */ + /* * H5Z__toml_parse_params - wrap params as a TOML document and parse it. * diff --git a/src/H5Zprivate.h b/src/H5Zprivate.h index aa8d2b84b8f..17a8253a229 100644 --- a/src/H5Zprivate.h +++ b/src/H5Zprivate.h @@ -116,6 +116,10 @@ H5_DLL htri_t H5Z_all_filters_avail(const struct H5O_pline_t *pline) H5_DLL htri_t H5Z_filter_avail(H5Z_filter_t id); H5_DLL herr_t H5Z_delete(struct H5O_pline_t *pline, H5Z_filter_t filter); H5_DLL herr_t H5Z_get_filter_info(H5Z_filter_t filter, unsigned int *filter_config_flags); +/* Normalise a parameter string into the form persisted in pipeline v3: + * outer braces stripped and hex-float literals rewritten to %.17e decimal, + * so the stored bytes are valid TOML v1.0.0. Caller frees with H5MM_xfree(). */ +H5_DLL char *H5Z_canonicalize_params(const char *params); /* Data Transform Functions */ typedef struct H5Z_data_xform_t H5Z_data_xform_t; /* Defined in H5Ztrans.c */ diff --git a/test/tfilter2.c b/test/tfilter2.c index fed488aff9f..4fd35c8b3c3 100644 --- a/test/tfilter2.c +++ b/test/tfilter2.c @@ -2380,6 +2380,298 @@ error: return -1; } +/* ----------------------------------------------------------------------- + * Canonicalization of the persisted configuration string + * (RFC-HDFG-2026-001 sec:pline-v3) + * + * The stored string is normalised so the bytes on disk are a valid TOML + * v1.0.0 document: optional outer braces are stripped, and C99 hex-float + * literals are rewritten to %.17e decimal. Neither the braced form nor a + * hex-float literal is accepted by a stock TOML parser, and the persisted + * string is meant to be readable by tools that are not the HDF5 library + * (pure-reimplementation readers such as jHDF and pyfive parse the object + * header directly). Both normalisations preserve the value exactly. + * + * This filter carries a double so bit-exactness can be asserted: the + * value is memcpy'd into cd_values rather than quantised. + * ---------------------------------------------------------------------- */ + +#define CANON_FILTER_ID 532 + +static herr_t +canon_set_config(const char *params, unsigned H5_ATTR_UNUSED *flags, size_t *cd_nelmts, + unsigned cd_values[], size_t cd_values_size) +{ + double rate = 0.0; + + /* A double occupies two unsigned slots; cd_values_size is an element + * count, matching the value H5Pappend_filter passes. */ + *cd_nelmts = 2; + if (cd_values) { + if (cd_values_size < 2) + return FAIL; + if (params && *params) { + if (H5Zconfig_get_double(params, "rate", &rate) < 0) + return FAIL; + } + memcpy(cd_values, &rate, sizeof(rate)); + } + return SUCCEED; +} + +static herr_t +canon_get_config(unsigned H5_ATTR_UNUSED flags, size_t cd_nelmts, const unsigned cd_values[], char *buf, + size_t *buf_size) +{ + double rate = 0.0; + size_t needed; + + if (cd_nelmts >= 2) + memcpy(&rate, cd_values, sizeof(rate)); + needed = (size_t)snprintf(NULL, 0, "rate = %.17e", rate) + 1; + if (buf_size) + *buf_size = needed; + if (buf) + snprintf(buf, needed, "rate = %.17e", rate); + return SUCCEED; +} + +static size_t +canon_filter_func(unsigned int H5_ATTR_UNUSED flags, size_t H5_ATTR_UNUSED cd_nelmts, + const unsigned int H5_ATTR_UNUSED *cd_values, hid_t H5_ATTR_UNUSED dxpl_id, + const hsize_t H5_ATTR_UNUSED *scaled, size_t H5_ATTR_UNUSED ndims, size_t nbytes, + size_t H5_ATTR_UNUSED *buf_size, void H5_ATTR_UNUSED **buf) +{ + return nbytes; /* pass-through */ +} + +static const H5Z_class3_t canon_cls = { + 2, /* version */ + CANON_FILTER_ID, /* id */ + 1, /* encoder_present */ + 1, /* decoder_present */ + "canon_filter", /* name */ + NULL, /* description */ + NULL, /* can_apply */ + NULL, /* set_local */ + canon_filter_func, /* filter */ + canon_set_config, /* set_config */ + canon_get_config, /* get_config */ +}; + +/* Append CANON_FILTER_ID configured with PARAMS and return the DCPL */ +static hid_t +canon_make_dcpl(const char *params) +{ + hid_t dcpl = H5I_INVALID_HID; + hsize_t chunk[2] = {4, 4}; + H5Z_params_t p; + + if ((dcpl = H5Pcreate(H5P_DATASET_CREATE)) < 0) + return H5I_INVALID_HID; + if (H5Pset_chunk(dcpl, 2, chunk) < 0) + goto error; + p.type = H5Z_PARAMS_STRING; + p.u.str = params; + if (H5Pappend_filter(dcpl, CANON_FILTER_ID, 0, &p) < 0) + goto error; + return dcpl; +error: + H5E_BEGIN_TRY + { + H5Pclose(dcpl); + } + H5E_END_TRY + return H5I_INVALID_HID; +} + +/* Assert that appending INPUT stores exactly EXPECT */ +static int +canon_check(const char *input, const char *expect) +{ + hid_t dcpl = H5I_INVALID_HID; + char pbuf[H5Z_CONFIG_STRING_MAX + 1]; + size_t plen = 0; + + if ((dcpl = canon_make_dcpl(input)) < 0) + return -1; + if (H5Pget_filter_params_by_idx(dcpl, 0, pbuf, sizeof(pbuf), &plen) < 0) + goto error; + if (strcmp(pbuf, expect) != 0) { + fprintf(stderr, "\n input \"%s\"\n stored \"%s\"\n expect \"%s\"\n", input, pbuf, expect); + goto error; + } + if (H5Pclose(dcpl) < 0) + return -1; + return 0; +error: + H5E_BEGIN_TRY + { + H5Pclose(dcpl); + } + H5E_END_TRY + return -1; +} + +/* Recover the double that set_config packed into cd_values */ +static int +canon_stored_double(hid_t dcpl, double *out) +{ + unsigned cd[8]; + size_t cd_nelmts = 8; + unsigned flags = 0; + char name[64]; + unsigned cfg = 0; + H5Z_filter_t id; + + id = H5Pget_filter2(dcpl, 0, &flags, &cd_nelmts, cd, sizeof(name), name, &cfg); + if (id < 0 || cd_nelmts < 2) + return -1; + memcpy(out, cd, sizeof(*out)); + return 0; +} + +static int +test_config_canonicalization(hid_t fapl) +{ + hid_t dcpl = H5I_INVALID_HID, sid = H5I_INVALID_HID, file = H5I_INVALID_HID, dset = H5I_INVALID_HID; + hid_t dcpl_out = H5I_INVALID_HID; + hsize_t dims[2] = {8, 8}; + char filename[1024]; + char pbuf[H5Z_CONFIG_STRING_MAX + 1]; + size_t plen = 0; + double got = 0.0; + + if (H5Zregister(&canon_cls) < 0) + TEST_ERROR; + + /* --- canon-01: a plain bare string is stored unchanged --- */ + TESTING("canonicalization: bare string stored unchanged"); + if (canon_check("rate = 1.5", "rate = 1.5") < 0) + TEST_ERROR; + PASSED(); + + /* --- canon-02: outer braces are stripped --- */ + TESTING("canonicalization: outer braces stripped"); + if (canon_check("{rate = 1.5}", "rate = 1.5") < 0) + TEST_ERROR; + if (canon_check("{ rate = 1.5 }", "rate = 1.5") < 0) + TEST_ERROR; + if (canon_check(" {rate = 1.5} ", "rate = 1.5") < 0) + TEST_ERROR; + PASSED(); + + /* --- canon-03: hex-float rewritten to %.17e decimal --- */ + TESTING("canonicalization: hex-float rewritten to decimal"); + if (canon_check("rate = 0x1.8p+1", "rate = 3.00000000000000000e+00") < 0) + TEST_ERROR; + if (canon_check("rate = 0x1.cp+1", "rate = 3.50000000000000000e+00") < 0) + TEST_ERROR; + PASSED(); + + /* --- canon-04: both normalisations at once --- */ + TESTING("canonicalization: braces and hex-float together"); + if (canon_check("{ rate = 0x1.8p+1 }", "rate = 3.00000000000000000e+00") < 0) + TEST_ERROR; + PASSED(); + + /* --- canon-05: hex-float text inside a quoted string is preserved --- */ + TESTING("canonicalization: hex-float inside a string is not rewritten"); + if (canon_check("rate = 1.5, note = \"0x1.8p+1\"", "rate = 1.5, note = \"0x1.8p+1\"") < 0) + TEST_ERROR; + if (canon_check("rate = 1.5, note = '0x1.8p+1'", "rate = 1.5, note = '0x1.8p+1'") < 0) + TEST_ERROR; + PASSED(); + + /* --- canon-06: rewriting a hex-float loses no precision --- */ + TESTING("canonicalization: hex-float value is bit-exact after rewrite"); + if ((dcpl = canon_make_dcpl("rate = 0x1.5555555555555p-2")) < 0) + TEST_ERROR; + if (canon_stored_double(dcpl, &got) < 0) + TEST_ERROR; + if (memcmp(&got, &(double){0x1.5555555555555p-2}, sizeof(got)) != 0) + TEST_ERROR; + if (H5Pclose(dcpl) < 0) + TEST_ERROR; + dcpl = H5I_INVALID_HID; + PASSED(); + + /* --- canon-07: the canonical form is itself valid set_config input --- */ + TESTING("canonicalization: stored form round-trips through set_config"); + if ((dcpl = canon_make_dcpl("{rate = 0x1.cp+1}")) < 0) + TEST_ERROR; + if (H5Pget_filter_params_by_idx(dcpl, 0, pbuf, sizeof(pbuf), &plen) < 0) + TEST_ERROR; + if (H5Pclose(dcpl) < 0) + TEST_ERROR; + /* Feed the stored string back in; it must parse and yield the same value */ + if ((dcpl = canon_make_dcpl(pbuf)) < 0) + TEST_ERROR; + if (canon_stored_double(dcpl, &got) < 0) + TEST_ERROR; + if (memcmp(&got, &(double){0x1.cp+1}, sizeof(got)) != 0) + TEST_ERROR; + if (H5Pclose(dcpl) < 0) + TEST_ERROR; + dcpl = H5I_INVALID_HID; + PASSED(); + + /* --- canon-08: the canonical form survives to disk and back with no + * plugin loaded (the case a non-HDF5 reader faces) --- */ + TESTING("canonicalization: canonical form persists and reads back plugin-free"); + if ((sid = H5Screate_simple(2, dims, NULL)) < 0) + TEST_ERROR; + h5_fixname(FILENAME[1], fapl, filename, sizeof(filename)); + if ((dcpl = canon_make_dcpl("{ rate = 0x1.8p+1 }")) < 0) + TEST_ERROR; + if ((file = H5Fcreate(filename, H5F_ACC_TRUNC, H5P_DEFAULT, fapl)) < 0) + TEST_ERROR; + if ((dset = H5Dcreate2(file, "dset", H5T_NATIVE_INT, sid, H5P_DEFAULT, dcpl, H5P_DEFAULT)) < 0) + TEST_ERROR; + if (H5Dclose(dset) < 0 || H5Pclose(dcpl) < 0 || H5Fclose(file) < 0) + TEST_ERROR; + dset = dcpl = file = H5I_INVALID_HID; + + if (H5Zunregister(CANON_FILTER_ID) < 0) /* only the stored bytes remain */ + TEST_ERROR; + if ((file = H5Fopen(filename, H5F_ACC_RDONLY, fapl)) < 0) + TEST_ERROR; + if ((dset = H5Dopen2(file, "dset", H5P_DEFAULT)) < 0) + TEST_ERROR; + if ((dcpl_out = H5Dget_create_plist(dset)) < 0) + TEST_ERROR; + if (H5Pget_filter_params_by_idx(dcpl_out, 0, pbuf, sizeof(pbuf), &plen) < 0) + TEST_ERROR; + /* Canonical: no outer brace, no hex-float -- parseable as plain TOML */ + if (strcmp(pbuf, "rate = 3.00000000000000000e+00") != 0) + TEST_ERROR; + if (pbuf[0] == '{' || strstr(pbuf, "0x") != NULL) + TEST_ERROR; + if (H5Pclose(dcpl_out) < 0 || H5Dclose(dset) < 0 || H5Fclose(file) < 0 || H5Sclose(sid) < 0) + TEST_ERROR; + dcpl_out = dset = file = sid = H5I_INVALID_HID; + if (H5Zregister(&canon_cls) < 0) + TEST_ERROR; + PASSED(); + + if (H5Zunregister(CANON_FILTER_ID) < 0) + TEST_ERROR; + return 0; + +error: + H5E_BEGIN_TRY + { + H5Pclose(dcpl); + H5Pclose(dcpl_out); + H5Dclose(dset); + H5Fclose(file); + H5Sclose(sid); + H5Zunregister(CANON_FILTER_ID); + } + H5E_END_TRY + return -1; +} + /* ----------------------------------------------------------------------- * main * ---------------------------------------------------------------------- */ @@ -2444,6 +2736,9 @@ main(void) /* On-disk configuration-string storage (pipeline v3) */ nerrors += test_config_string_ondisk(fapl) < 0 ? 1 : 0; + /* Canonicalization of the persisted configuration string */ + nerrors += test_config_canonicalization(fapl) < 0 ? 1 : 0; + if (H5Fclose(file) < 0) goto error;