BUG: Fix null metadata cast and unchecked geometry lengths in HDF5ImageIO

[physwkim]

HDF5ImageIO::WriteImageInformation() dereferences a null dynamic_cast result for a MetaDataObject<char *>, and ReadImageInformation() indexes the geometry vectors to numDims without checking the datasets are actually that long. Addresses B73 and B74 of #6575.

Root cause

B73: the C-string branch guards with cstringObj != nullptr || constCstringObj != nullptr and then unconditionally dereferences constCstringObj. MetaDataObject<char *> and MetaDataObject<const char *> are unrelated instantiations, so only one cast can succeed — a char * entry makes the other null, and it is the one that gets dereferenced.

B74: numDims comes from the Directions dataset, but Origin, Spacing and Dimension are read with ReadVector and then indexed to numDims regardless of their own length. A file whose geometry datasets disagree over-reads a heap vector before any pixel is touched.

Directions itself — the dataset numDims is derived from — was equally unchecked: ReadDirections() returns dim[1] rows of dim[0] entries and never compares the two, so a non-square matrix yields direction rows shorter than numDims. All four geometry datasets are now validated for an exact length match.

Exception descriptions only reach the caller because ExceptionObject is now rethrown unchanged: every throw inside ReadImageInformation() was previously caught by the trailing catch (...) and replaced with "Unspecified error occurred during ReadImageInformation".

Local validation

New GoogleTest itkHDF5ImageIOGTest.cxx, fixtures written in the test body, no ExternalData. Assertions match on the exception description, so they cannot pass on an unrelated failure.

Test	Fail-before	Pass-after
ReadImageInformationAcceptsConsistentGeometry	positive control	passes
ReadImageInformationRejectsTruncatedOrigin / ...Spacing / ...Dimension	silent out-of-bounds read, or generic "Unspecified error" description	throws, description names the dataset
ReadImageInformationRejectsNonSquareDirections	loads with silently wrong direction cosines	throws
WriteImageInformationCStringMetaDataRoundTrips	SIGSEGV at -O0; also covers the const char * branch	passes

ctest -R HDF5: 8/8 pass, no result changes.

Note: the char * metadata test is a -O0/sanitizer canary. At -O1+ the optimizer sinks the dead load of the null cast into the not-taken branch, so the unfixed code does not fault in an optimized build.

B75 investigated and withdrawn

The ledger also listed B75 (Read() passes the file datatype as H5Dread's memory type, so a STD_I16BE dataset should read back byte-swapped). The code is as described, but the path is unreachable: ReadImageInformation() runs PredTypeToComponentType(), which requires an exact H5Tequal match against a NATIVE_* predtype and throws otherwise — and it populates m_VoxelDataSet, so Read() cannot run before it succeeds. Writing a STD_I16BE VoxelData and reading it back throws during ReadImageInformation(); with STD_I16LE on this host the file type is bit-identical to NATIVE_SHORT and the line is a no-op. There is no reachable failure to fix, so no change was made.

Known limitations left for follow-up

• WriteString(const std::string &, const char * s) does std::string _s(s) with no null check, so a MetaDataObject<char *> holding nullptr is still UB. Out of scope for B73, which concerns the null cast rather than the null value.
• WriteImageInformation() silently drops metadata whose type is outside the hardcoded cast list (e.g. Array<long long>, which the reader can itself produce).
• No catch handler calls ResetToInitialState(), so a rejected file leaves the ImageIO holding an open HDF5 handle.

AI assistance

• Tool: Claude Code
• Role: implementation and regression-test authoring from the analysis in BUG: Findings collected while porting ~60 filters — reported together in one issue #6575
• All code was reviewed, built, and tested locally before committing

[greptile-apps]

Greptile Summary

This PR fixes HDF5 image metadata and geometry validation. The main changes are:

• Adds length checks for Directions, Origin, Spacing, and Dimension datasets.
• Preserves ITK exception descriptions from ReadImageInformation().
• Splits char * and const char * metadata writes to avoid a null cast dereference.
• Adds GoogleTest coverage and module test dependencies for the HDF5 fixes.

Confidence Score: 5/5

This looks safe to merge.

No blocking issues found in the changed code.

T-Rex Logs

What T-Rex did

• Inspected the HDF5 tool availability log to verify which tools are on the PATH, confirming cmake, ctest, ninja, and pixi are available and C/C++ compilers are present.
• Reviewed the HDF5 validation summary log and confirmed it records the intended configure, build, and ctest commands along with the exact blocker.

_{Ran code and verified through T-Rex}

Important Files Changed

Filename	Overview
Modules/IO/HDF5/src/itkHDF5ImageIO.cxx	Adds geometry dataset validation, preserves ITK exception details, and fixes C-string metadata cast handling.
Modules/IO/HDF5/test/itkHDF5ImageIOGTest.cxx	Adds tests for geometry validation and C-string metadata round-tripping.
Modules/IO/HDF5/test/CMakeLists.txt	Adds the HDF5 GoogleTest driver and output-directory compile definition.
Modules/IO/HDF5/itk-module.cmake	Adds test dependencies needed by the new HDF5 GoogleTest target.

_{Reviews (2): Last reviewed commit: "BUG: Validate HDF5 geometry dataset leng..." | Re-trigger Greptile}

[kwrobot-v1]

Errors:

• Failed to reserve ref 04e4a95 for the merge request: invalid git ref: 'no such commit'.
• Failed to run the checks: mr utilities error: failed to list commits of 04e4a953285281f2a912e2487aa5095367f307ff for https://github.com/InsightSoftwareConsortium/ITK/pull/6626: fatal: bad object 04e4a953285281f2a912e2487aa5095367f307ff .

[hjmjohnson]

Two force-pushes, one concern each:

1. c4d44b5 → d4120bb — plain rebase onto main (ab57477), no content changes. git range-diff shows = for both commits; nothing to review here.
2. d4120bb → 04e4a95 — review fixes only, same base. This is the one to read.

What changed in push 2

• Directions rows are now validated against numDims (ReadDirections() returns dim[1] rows of dim[0] entries and never compared them, so a non-square matrix produced direction rows shorter than numDims — the same unchecked-length class as B74, reachable past the original three checks).
• ExceptionObject is rethrown unchanged ahead of the trailing catch (...). Without this, all of the new diagnostics were swallowed and replaced with "Unspecified error occurred during ReadImageInformation" — the checks threw, but the messages never reached the caller.
• Geometry length checks kept as exact (!=) and applied uniformly to all four datasets.
• Tests assert on the exception description rather than only the type, so they can no longer pass on an unrelated failure; added a positive control, a non-square-Directions case, and coverage of the const char * metadata branch.
• Test output moved to ITK_TEST_OUTPUT_DIR (matching Modules/IO/GDCM/test) instead of fixed names in the shared system temp dir.
• Writer is scoped so the file is closed before the read-back, making the round-trip test actually go through disk.

pre-commit run --all-files exits 0 on 04e4a95; ctest -R HDF5 is 8/8 locally.