HDFGroup / HDFGroup/hdf5

Onion VFD: page_size on open -- 0 is documented but rejected, and revision 0 with a mismatched value asserts

Open
#6,613 0 comments 0 reactions 1 assignee Claimed by @brtnfld View on GitHub
Component - C Library
Dominant language
C
Stars
988
Forks
355
Avg merge
4d 2h
Merged PRs (30d)
12

Description

### Summary

To open an existing onion history you must already know the `page_size` it was written with:

1. `H5Pset_fapl_onion()` **rejects `page_size = 0`**, although `H5FDonion.h` documents 0 as the way to say "whatever the file already uses" when opening an existing file.
2. Opening **revision 0** with a `page_size` other than the file's **aborts on an assertion** rather than failing.

Together these leave no safe way to open a history whose page size is unknown, and the failure mode for guessing wrong is a process abort.

### 1. `page_size = 0` is documented but rejected

From `src/H5FDonion.h`:

> `page_size`: Size of the amended data pages. **If opening an existing file, must equal the existing page size or zero.**

But `H5Pset_fapl_onion()` validates it unconditionally (`src/H5FDonion.c:325-327`):

```
HDF5-DIAG: Error detected in HDF5 (2.3.0):
#000: src/H5FDonion.c line 326 in H5Pset_fapl_onion(): invalid info page size
major: Invalid arguments to routine
minor: Bad value
```

so 0 never reaches the open path at all. Either the check should allow 0 when the file exists, or the documentation should drop "or zero".

### 2. Revision 0 with a mismatched `page_size` aborts

```c
/* Onion VFD: there is no valid page_size to query an existing history with
* unless you already know the one it was written with.
*
* Build: h5cc -o onion_rev0_pagesize onion_rev0_pagesize.c
* Run: ./onion_rev0_pagesize 32 -> opens normally
* ./onion_rev0_pagesize 4096 -> assertion failure, process aborts
*
* Always writes the history at page_size 32; only the page_size used to open
* revision 0 varies. Revisions 1..3 open fine with either value -- it is
* revision 0, the original canonical file, that aborts.
*/
#include
#include
#include
#include

#include "hdf5.h"
#include "H5FDonion.h"

#define FNAME "onion_rev0_pagesize.h5"
#define WRITE_PAGE 32

static hid_t
onion_fapl(uint64_t revision, uint32_t page_size)
{
H5FD_onion_fapl_info_t info;
hid_t fapl;

memset(&info, 0, sizeof(info));
info.version = H5FD_ONION_FAPL_INFO_VERSION_CURR;
info.backing_fapl_id = H5Pcreate(H5P_FILE_ACCESS);
info.page_size = page_size;
info.store_target = H5FD_ONION_STORE_TARGET_ONION;
info.revision_num = revision;
strcpy(info.comment, "repro");

if ((fapl = H5Pcreate(H5P_FILE_ACCESS)) < 0)
return H5I_INVALID_HID;
if (H5Pset_fapl_onion(fapl, &info) < 0) {
H5Pclose(fapl);
return H5I_INVALID_HID;
}
return fapl;
}

int
main(int argc, char **argv)
{
uint32_t query_page = (argc > 1) ? (uint32_t)strtoul(argv[1], NULL, 10) : 4096;
hid_t fid, fapl, sp, ds;
int i;

unlink(FNAME);
unlink(FNAME ".onion");
unlink(FNAME ".onion.recovery");

if ((fid = H5Fcreate(FNAME, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT)) < 0)
return 1;
H5Fclose(fid);

for (i = 0; i < 3; i++) {
int val = 10 + i;

if ((fapl = onion_fapl(H5FD_ONION_FAPL_INFO_REVISION_ID_LATEST, WRITE_PAGE)) < 0)
return 1;
if ((fid = H5Fopen(FNAME, H5F_ACC_RDWR, fapl)) < 0)
return 1;
H5E_BEGIN_TRY
{
H5Ldelete(fid, "/a", H5P_DEFAULT);
}
H5E_END_TRY
sp = H5Screate(H5S_SCALAR);
ds = H5Dcreate2(fid, "/a", H5T_NATIVE_INT, sp, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT);
H5Dwrite(ds, H5T_NATIVE_INT, H5S_ALL, H5S_ALL, H5P_DEFAULT, &val);
H5Dclose(ds);
H5Sclose(sp);
H5Fclose(fid);
H5Pclose(fapl);
}

printf("wrote 3 revisions at page_size %d; opening REVISION 0 with page_size %u\n", WRITE_PAGE,
query_page);
fflush(stdout);

if ((fapl = onion_fapl(0 /* revision 0 == the original canonical file */, query_page)) < 0) {
printf(" H5Pset_fapl_onion() rejected page_size %u\n", query_page);
return 1;
}

H5E_BEGIN_TRY
{
fid = H5Fopen(FNAME, H5F_ACC_RDONLY, fapl);
}
H5E_END_TRY
printf(" revision 0: %s\n", (fid >= 0) ? "opened" : "open failed (an error, which is fine)");
if (fid >= 0)
H5Fclose(fid);

H5Pclose(fapl);
return 0;
}
```

### Observed

```
$ ./onion_rev0_pagesize 32
wrote 3 revisions at page_size 32; opening REVISION 0 with page_size 32
revision 0: opened

$ ./onion_rev0_pagesize 4096
wrote 3 revisions at page_size 32; opening REVISION 0 with page_size 4096
onion_rev0_pagesize: src/H5FDonion.c:1386: H5FD__onion_read: Assertion `0 == bytes_to_read' failed.
Aborted (core dumped)
```

Only revision 0 -- the original canonical file -- is affected. Revisions 1, 2 and 3 open and read correctly with either page size, and `H5FDonion_get_revision_count()` also works with the mismatched value, which is what makes this easy to hit late: everything else appears to tolerate the wrong page size.

### Expected

An unusable `page_size` should produce an error from `H5Fopen()` (or be ignored in favour of the value in the onion header, which the library has already read by then), not an assertion. In a build without assertions this is presumably a bad read rather than a clean abort.

### Environment

- HDF5 `develop` @ d006fb8d3a09c7d09048f25b45336afeddd9d87d (2026-08-06), version 2.3.0
- Linux, gcc 15.2.1, `CMAKE_BUILD_TYPE=Debug` (assertions active)

### Context

Found while building a tool that turns a streaming HDF5 file's steps into addressable onion revisions. Because of (1) the reader cannot pass 0, and because of (2) guessing wrong crashes, the tool has to record the page size it used as an attribute in the canonical file so a reader can recover it before opening any revision. A public accessor for a history's page size would remove the need for that (cf. #6363, which asks for `H5FDonion_get_revision_info()`).

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.