Add new class: ImageCtorParams

[kevinbackhouse]

This PR adds a new parameter to the constructor of the Image class and refactors the constructors of all the classes that inherit from Image so that they pass that parameter through. This is an initial refactoring step. The end goal is to add a max_recursion_depth limit to all of the image parsers so that we can stop people from sending us security reports about deep recursions. (An alternative solution is #9347, but that only works on Linux.) Rather than adding a max_recursion_depth to every constructor, I thought it would be better to a new class named ImageCtorParams. That way, it will be much easier to add new parameters like this in the future, because we can just add a new field to the class.

There are a few classes that already have a max_recursion_depth_ field. I will move that field into the Image base class in a follow-up PR. I will also follow up with PRs that add recursion depth tracking to each of the image parsers that need it.

This PR changes the public interface, so it should not be backported to 0.28.x.

[Copilot]

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

This PR introduces Exiv2::ImageCtorParams to bundle constructor parameters (including max_recursion_depth) and refactors Image subclasses and factory hooks to pass these parameters through, updating public headers, tests, and a sample accordingly.

Changes:

• Added ImageCtorParams and updated Image / NewInstanceFct APIs to accept it.
• Refactored many image/video format constructors and new*Instance factory functions to forward ImageCtorParams.
• Updated unit tests + build files to construct images with the new parameter object.

Show a summary per file

File	Description
unitTests/unittest_utils.hpp	Adds a small helper declaration for creating default ImageCtorParams in tests.
unitTests/unittest_utils.cpp	Implements the default test params helper.
unitTests/test_riffVideo.cpp	Updates RIFF video tests to pass ImageCtorParams.
unitTests/test_pngimage.cpp	Updates PNG tests to pass ImageCtorParams.
unitTests/test_matroskavideo.cpp	Updates Matroska video tests to pass ImageCtorParams.
unitTests/test_jp2image.cpp	Updates JP2 tests to pass ImageCtorParams.
unitTests/test_bmpimage.cpp	Updates BMP tests to pass ImageCtorParams.
unitTests/test_asfvideo.cpp	Updates ASF video tests to pass ImageCtorParams.
unitTests/meson.build	Adds new test utility source to Meson test target.
unitTests/CMakeLists.txt	Adds new test utility files to the unit test executable.
src/xmpsidecar.cpp	Refactors XMP sidecar constructor + factory to accept ImageCtorParams.
src/webpimage.cpp	Refactors WebP constructor + factory to accept ImageCtorParams.
src/tiffimage.cpp	Refactors TIFF constructor + factory to accept ImageCtorParams.
src/tgaimage.cpp	Refactors TGA constructor + factory to accept ImageCtorParams.
src/rw2image.cpp	Refactors RW2 constructor + factory to accept ImageCtorParams.
src/riffvideo.cpp	Refactors RIFF constructor + factory to accept ImageCtorParams.
src/rafimage.cpp	Refactors RAF constructor/factory and updates embedded JPEG instantiation.
src/quicktimevideo.cpp	Refactors QuickTime constructor/factory to take params and pull recursion depth from it.
src/psdimage.cpp	Refactors PSD constructor + factory to accept ImageCtorParams.
src/pngimage.cpp	Refactors PNG constructor + factory to accept ImageCtorParams.
src/pgfimage.cpp	Refactors PGF constructor + factory to accept ImageCtorParams.
src/orfimage.cpp	Refactors ORF constructor + factory to accept ImageCtorParams.
src/mrwimage.cpp	Refactors MRW constructor + factory to accept ImageCtorParams.
src/matroskavideo.cpp	Refactors Matroska constructor + factory to accept ImageCtorParams.
src/jpgimage.cpp	Refactors JPEG/EXV constructors + factory functions to accept ImageCtorParams.
src/jp2image.cpp	Refactors JP2 constructor + factory to accept ImageCtorParams.
src/image.cpp	Implements ImageCtorParams and updates ImageFactory::{open,create} to pass it.
src/gifimage.cpp	Refactors GIF constructor + factory to accept ImageCtorParams.
src/epsimage.cpp	Refactors EPS constructor + factory to accept ImageCtorParams.
src/crwimage.cpp	Refactors CRW constructor + factory to accept ImageCtorParams.
src/cr2image.cpp	Refactors CR2 constructor + factory to accept ImageCtorParams.
src/bmpimage.cpp	Refactors BMP constructor + factory to accept ImageCtorParams.
src/bmffimage.cpp	Refactors BMFF constructor + factory to accept ImageCtorParams and uses params recursion depth.
src/asfvideo.cpp	Refactors ASF constructor + factory to accept ImageCtorParams and uses params recursion depth.
samples/tiff-test.cpp	Updates sample code to construct TIFF with ImageCtorParams.
include/exiv2/xmpsidecar.hpp	Updates XMP sidecar public ctor/factory signatures and docs for params.
include/exiv2/webpimage.hpp	Updates WebP public ctor/factory signatures and docs for params.
include/exiv2/tiffimage.hpp	Updates TIFF public ctor/factory signatures and docs for params.
include/exiv2/tgaimage.hpp	Updates TGA public ctor/factory signatures and docs for params.
include/exiv2/rw2image.hpp	Updates RW2 public ctor/factory signatures and docs for params.
include/exiv2/riffvideo.hpp	Updates RIFF public ctor/factory signatures and docs for params.
include/exiv2/rafimage.hpp	Updates RAF public ctor/factory signatures and docs for params.
include/exiv2/quicktimevideo.hpp	Updates QuickTime public ctor/factory signatures and docs for params.
include/exiv2/psdimage.hpp	Updates PSD public ctor/factory signatures and docs for params.
include/exiv2/pngimage.hpp	Updates PNG public ctor/factory signatures and docs for params.
include/exiv2/pgfimage.hpp	Updates PGF public ctor/factory signatures and docs for params.
include/exiv2/orfimage.hpp	Updates ORF public ctor/factory signatures and docs for params.
include/exiv2/mrwimage.hpp	Updates MRW public ctor/factory signatures and docs for params.
include/exiv2/matroskavideo.hpp	Updates Matroska public ctor/factory signatures and docs for params.
include/exiv2/jpgimage.hpp	Updates JPEG/EXV public ctor/factory signatures and docs for params.
include/exiv2/jp2image.hpp	Updates JP2 public ctor/factory signatures and docs for params.
include/exiv2/image.hpp	Introduces ImageCtorParams and updates Image constructor + NewInstanceFct signature.
include/exiv2/gifimage.hpp	Updates GIF public ctor/factory signatures and docs for params.
include/exiv2/epsimage.hpp	Updates EPS public ctor/factory signatures and docs for params.
include/exiv2/crwimage.hpp	Updates CRW public ctor/factory signatures and docs for params.
include/exiv2/cr2image.hpp	Updates CR2 public ctor/factory signatures and docs for params.
include/exiv2/bmpimage.hpp	Updates BMP public ctor/factory signatures and docs for params.
include/exiv2/bmffimage.hpp	Updates BMFF public ctor/factory signatures and docs for params.
include/exiv2/asfvideo.hpp	Updates ASF public ctor/factory signatures and docs for params.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comments suppressed due to low confidence (15)

src/image.cpp:1

• The factory now hard-codes a default max_recursion_depth of 1000 for all formats. This appears to relax the default for at least ASF (previously defaulted to 500 via the public constructor), which is a security-relevant behavior change in the opposite direction of the PR’s stated goal. Consider centralizing the default depth in a single named constant (or a ImageCtorParams::forOpen()/forCreate() helper) and choosing a value that does not increase the prior effective defaults for any format (e.g., keep 500 unless there’s a specific justification).
  src/image.cpp:1
• The factory now hard-codes a default max_recursion_depth of 1000 for all formats. This appears to relax the default for at least ASF (previously defaulted to 500 via the public constructor), which is a security-relevant behavior change in the opposite direction of the PR’s stated goal. Consider centralizing the default depth in a single named constant (or a ImageCtorParams::forOpen()/forCreate() helper) and choosing a value that does not increase the prior effective defaults for any format (e.g., keep 500 unless there’s a specific justification).
  src/rafimage.cpp:1
• Passing max_recursion_depth = 0 for the embedded JPEG instance is likely unintended: any recursion-depth enforcement would immediately reject parsing (or behave differently from prior behavior, which effectively used the JPEG defaults). Use a non-zero default (ideally the same recursion limit as the parent RAF parsing context), e.g., propagate the RAF instance’s configured recursion limit into this nested JpegImage construction.
  unitTests/test_pngimage.cpp:1
• Unit tests now duplicate the magic number 500 across many call sites, while other tests use defaultImageCtorParams(). To reduce churn when defaults change, consider extending the helper to accept create (and possibly an override for recursion depth), and updating tests/samples to rely on that shared helper or a single named constant.
  unitTests/test_pngimage.cpp:1
• Unit tests now duplicate the magic number 500 across many call sites, while other tests use defaultImageCtorParams(). To reduce churn when defaults change, consider extending the helper to accept create (and possibly an override for recursion depth), and updating tests/samples to rely on that shared helper or a single named constant.
  unitTests/test_pngimage.cpp:1
• Unit tests now duplicate the magic number 500 across many call sites, while other tests use defaultImageCtorParams(). To reduce churn when defaults change, consider extending the helper to accept create (and possibly an override for recursion depth), and updating tests/samples to rely on that shared helper or a single named constant.
  unitTests/test_pngimage.cpp:1
• Unit tests now duplicate the magic number 500 across many call sites, while other tests use defaultImageCtorParams(). To reduce churn when defaults change, consider extending the helper to accept create (and possibly an override for recursion depth), and updating tests/samples to rely on that shared helper or a single named constant.
  unitTests/test_pngimage.cpp:1
• Unit tests now duplicate the magic number 500 across many call sites, while other tests use defaultImageCtorParams(). To reduce churn when defaults change, consider extending the helper to accept create (and possibly an override for recursion depth), and updating tests/samples to rely on that shared helper or a single named constant.
  unitTests/test_pngimage.cpp:1
• Unit tests now duplicate the magic number 500 across many call sites, while other tests use defaultImageCtorParams(). To reduce churn when defaults change, consider extending the helper to accept create (and possibly an override for recursion depth), and updating tests/samples to rely on that shared helper or a single named constant.
  unitTests/test_pngimage.cpp:1
• Unit tests now duplicate the magic number 500 across many call sites, while other tests use defaultImageCtorParams(). To reduce churn when defaults change, consider extending the helper to accept create (and possibly an override for recursion depth), and updating tests/samples to rely on that shared helper or a single named constant.
  unitTests/test_pngimage.cpp:1
• Unit tests now duplicate the magic number 500 across many call sites, while other tests use defaultImageCtorParams(). To reduce churn when defaults change, consider extending the helper to accept create (and possibly an override for recursion depth), and updating tests/samples to rely on that shared helper or a single named constant.
  unitTests/test_pngimage.cpp:1
• Unit tests now duplicate the magic number 500 across many call sites, while other tests use defaultImageCtorParams(). To reduce churn when defaults change, consider extending the helper to accept create (and possibly an override for recursion depth), and updating tests/samples to rely on that shared helper or a single named constant.
  unitTests/test_pngimage.cpp:1
• Unit tests now duplicate the magic number 500 across many call sites, while other tests use defaultImageCtorParams(). To reduce churn when defaults change, consider extending the helper to accept create (and possibly an override for recursion depth), and updating tests/samples to rely on that shared helper or a single named constant.
  src/image.cpp:1
• The new ImageCtorParams plumbing introduces security-sensitive behavior (recursion limits) and changes defaulting in the factory path, but the updated unit tests shown here only verify compilation/usage rather than enforcement. Add focused tests that (1) exercise ImageFactory::open/create using the new defaults and (2) verify that a low max_recursion_depth reliably causes a throw/early failure on a crafted deeply-nested structure (e.g., for QuickTime/BMFF/ASF), preventing regressions in the intended safety mechanism.
  src/image.cpp:1
• The new ImageCtorParams plumbing introduces security-sensitive behavior (recursion limits) and changes defaulting in the factory path, but the updated unit tests shown here only verify compilation/usage rather than enforcement. Add focused tests that (1) exercise ImageFactory::open/create using the new defaults and (2) verify that a low max_recursion_depth reliably causes a throw/early failure on a crafted deeply-nested structure (e.g., for QuickTime/BMFF/ASF), preventing regressions in the intended safety mechanism.

• Files reviewed: 59/59 changed files
• Comments generated: 0
• Review effort level: Low

[Copilot]

Review details

Comments suppressed due to low confidence (3)

src/rafimage.cpp:1

• RafImage::readMetadata() hard-codes ImageCtorParams(false, 0) when parsing the embedded JPEG preview. This breaks propagation of the caller’s recursion-depth policy and may effectively disable (or overly tighten) recursion limiting depending on how 0 is interpreted. Prefer passing through the RAF instance’s params (or at least reuse params.max_recursion_depth()), so nested parsing can’t bypass the configured safety limit.
  src/image.cpp:1
• 1000 is a magic-number default for max_recursion_depth in ImageFactory::open (and similarly in ImageFactory::create). Consider centralizing this default (e.g., a static constexpr on ImageCtorParams or ImageFactory) so library defaults, samples, and tests don’t drift and so changing the default later is a one-line change.
  unitTests/unittest_utils.cpp:1
• The unit-test default uses max_recursion_depth = 500, which currently differs from the ImageFactory default introduced in this PR (1000). To avoid inconsistent behavior across tests vs. production defaults, it would be better to reference a shared default constant (or explicitly justify why tests should exercise a smaller limit).

• Files reviewed: 59/59 changed files
• Comments generated: 1
• Review effort level: Low

[mergify]

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

• Queue this pull request