BUG: Backport null checks after ReadHeader in IPLCommonImageIO

[hjmjohnson]

Cherry-pick of 0eb1c42 from main. Adds null checks after ReadHeader calls in IPLCommonImageIO to prevent null pointer dereference.

Backport for #6051 (Tier 1).

[greptile-apps]

Greptile Summary

This PR backports commit 0eb1c42d from main to release-5.4, adding two explicit null checks after ReadHeader calls in IPLCommonImageIO::ReadImageInformation() to prevent null pointer dereferences. Previously the code relied on an incorrect assumption (noted in a comment) that exceptions would propagate naturally, but if a child class's ReadHeader returned nullptr without throwing, the code would crash with undefined behaviour.

Confidence Score: 5/5

Safe to merge — the fix eliminates a confirmed null-pointer crash with well-defined exception handling; remaining P2 note is a style/philosophy point that doesn't affect correctness in practice.

Both null checks are strictly safer than the original code. The first check (primary file) is unambiguously correct. The second check (directory loop) trades a crash for an exception; the suggestion to use continue instead is a minor philosophy concern since all known child classes already throw on failure anyway. No regressions, no security concerns, and the fix aligns with the cherry-pick from main.

Modules/IO/IPL/src/itkIPLCommonImageIO.cxx — specifically the second null check at line 228 and whether throw vs continue is the right policy for the directory-scan loop.

Important Files Changed

Filename	Overview
Modules/IO/IPL/src/itkIPLCommonImageIO.cxx	Adds two null checks after ReadHeader calls; first check (primary file) is straightforward and correct; second check (directory loop) throws an exception that propagates outside the surrounding try-catch, which may be overly strict compared to just continuing the loop.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[ReadImageInformation called] --> B["ReadHeader(primaryFile)"]
    B --> C{nullptr?}
    C -- yes --> D["itkExceptionMacro NEW\n(was: null-deref crash)"]
    C -- no --> E[Read modality, add to list\nloop through directory]
    E --> F["ReadHeader(curFile) in try-catch"]
    F -- exception --> G[catch: continue to next file]
    F -- nullptr --> H{nullptr? NEW}
    H -- yes --> I["itkExceptionMacro\n(propagates OUTSIDE try-catch)"]
    H -- no --> J[Compare series/echo keys]
    J --> K[AddElementToList or skip]
    K --> F

Loading

_{Reviews (1): Last reviewed commit: "BUG: Add null checks after ReadHeader in..." | Re-trigger Greptile}

[greptile-apps]

Loop null-check aborts scan on one unexpected file

The null check correctly prevents a crash, but the itkExceptionMacro throw sits outside the enclosing try-catch block (lines 217-227). If any file in the directory has a ReadHeader implementation that returns nullptr without throwing — the base class IPLCommonImageIO::ReadHeader does exactly this — the exception will propagate all the way up and abort the entire ReadImageInformation() call, rather than skipping that file as the surrounding try-catch intends.

All three known child-class implementations (GE4, GE5, GEAdw) always throw on failure, so this is unlikely to trigger in practice. But using continue here would be consistent with the loop's error-handling philosophy and safe against future subclasses:

Suggested change

	if (curImageHeader == nullptr)
	{
	itkExceptionMacro("ReadHeader failed for " << fullPath);
	}
	if (curImageHeader == nullptr)
	{
	continue;
	}

[hjmjohnson]

This is a backport from main -> release-5.4; if another developer thinks it needs to be fixed, then we need to fix it upstream as well.