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.
This commit is contained in:
Scot Breitenfeld
2026-08-20 11:11:25 -05:00
parent fe96544389
commit 1fe2ee7de0
4 changed files with 407 additions and 6 deletions
+15 -5
View File
@@ -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() */
+93 -1
View File
@@ -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.
*
+4
View File
@@ -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 */
+295
View File
@@ -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;