From 43dd03c00f752e360aac283a0416b8b89f49f980 Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Tue, 11 Aug 2026 17:22:57 -0700 Subject: [PATCH 1/2] fix(ImageInput): guarantee close()-on-failure for all open() methods If ImageInput::open() encounters an error and returns false, is the object expected to be left in the same state as a newly constructed, never-opened instance (other than having an error that can be retrieved), as if close() had been called? Or is it up to the caller to do the close()? We were never quite clear on this, and some plugins have any open() failures do a close() before returning, but others did not. Clean this up once and for all by documenting ImageInput::open() to make clear that the contract is that a failed open() will close the file and needs no further cleanup action on the part of the caller. Audit all readers and ensure that in every instance that an open() method returns false to indicate an error, close() is also called. Note that: - The error message is stil retrievable from the closed ImageInput. - If an IOProxy was passed to the ImageInput to use, that proxy is still open and able to be used for further reads to the same file. Assisted-by: Claude Code / Claude Sonnet 5 Signed-off-by: Larry Gritz Co-authored by: Brecht Van Lommel --- src/cineon.imageio/cineoninput.cpp | 6 +++++- src/dds.imageio/ddsinput.cpp | 17 ++++++++++++++--- src/dicom.imageio/dicominput.cpp | 6 +++++- src/dpx.imageio/dpxinput.cpp | 1 + src/ffmpeg.imageio/ffmpeginput.cpp | 9 +++++++++ src/fits.imageio/fitsinput.cpp | 4 +++- src/heif.imageio/heifinput.cpp | 4 ++++ src/ico.imageio/icoinput.cpp | 5 ++++- src/iff.imageio/iffinput.cpp | 4 +++- src/include/OpenImageIO/imageio.h | 8 ++++++++ src/jpeg2000.imageio/jpeg2000input.cpp | 1 + src/jpegxl.imageio/jxlinput.cpp | 21 +++++++++++++++++++++ src/openexr.imageio/exrinput.cpp | 6 ++++++ src/openvdb.imageio/openvdbinput.cpp | 4 +++- src/png.imageio/pnginput.cpp | 1 + src/pnm.imageio/pnminput.cpp | 13 ++++++++++--- src/psd.imageio/psdinput.cpp | 7 +++++++ src/ptex.imageio/ptexinput.cpp | 8 ++++++-- src/r3d.imageio/r3dinput.cpp | 1 + src/raw.imageio/rawinput.cpp | 9 ++++++--- src/rla.imageio/rlainput.cpp | 6 +++++- src/sgi.imageio/sgiinput.cpp | 8 ++++++-- src/targa.imageio/targainput.cpp | 24 +++++++++++++++++++++--- src/tiff.imageio/tiffinput.cpp | 2 ++ src/webp.imageio/webpinput.cpp | 7 ++++++- 25 files changed, 158 insertions(+), 24 deletions(-) diff --git a/src/cineon.imageio/cineoninput.cpp b/src/cineon.imageio/cineoninput.cpp index f7704e4b42..ad23052b45 100644 --- a/src/cineon.imageio/cineoninput.cpp +++ b/src/cineon.imageio/cineoninput.cpp @@ -88,6 +88,7 @@ CineonInput::open(const std::string& name, ImageSpec& newspec) m_cin.SetInStream(m_stream); if (!m_cin.ReadHeader()) { errorfmt("Could not read header"); + close(); return false; } @@ -126,7 +127,10 @@ CineonInput::open(const std::string& name, ImageSpec& newspec) case 2: typedesc = TypeDesc::UINT16; break; case 3: case 4: typedesc = TypeDesc::UINT32; break; - default: errorfmt("Unsupported bit depth {}", maxbits); return false; + default: + errorfmt("Unsupported bit depth {}", maxbits); + close(); + return false; } m_spec = ImageSpec(m_cin.header.Width(), m_cin.header.Height(), nchannels, typedesc); diff --git a/src/dds.imageio/ddsinput.cpp b/src/dds.imageio/ddsinput.cpp index 355cddd3b2..2f6248dfdb 100644 --- a/src/dds.imageio/ddsinput.cpp +++ b/src/dds.imageio/ddsinput.cpp @@ -425,8 +425,10 @@ DDSInput::open(const std::string& name, ImageSpec& newspec) ioseek(0); static_assert(sizeof(dds_header) == 128, "dds header size does not match"); - if (!ioread(&m_dds, sizeof(m_dds), 1)) + if (!ioread(&m_dds, sizeof(m_dds), 1)) { + close(); return false; + } if (bigendian()) { // DDS files are little-endian @@ -481,6 +483,7 @@ DDSInput::open(const std::string& name, ImageSpec& newspec) || (m_dds.caps.flags2 & DDS_CAPS2_CUBEMAP && !(m_dds.caps.flags1 & DDS_CAPS1_COMPLEX))) { errorfmt("Invalid DDS header, possibly corrupt file"); + close(); return false; } @@ -494,13 +497,16 @@ DDSInput::open(const std::string& name, ImageSpec& newspec) & (DDS_PF_RGB | DDS_PF_LUMINANCE | DDS_PF_ALPHA | DDS_PF_ALPHAONLY)))) { errorfmt("Image with no data"); + close(); return false; } // read optional DX10 header if (m_dds.fmt.fourCC == DDS_4CC_DX10) { - if (!ioread(&m_dx10, sizeof(m_dx10), 1)) + if (!ioread(&m_dx10, sizeof(m_dx10), 1)) { + close(); return false; + } /*std::cerr << "[dds:dx10] dxgiFormat: " << m_dx10.dxgiFormat << "\n"; std::cerr << "[dds:dx10] resourceDimension: " << m_dx10.resourceDimension << "\n"; @@ -557,6 +563,7 @@ DDSInput::open(const std::string& name, ImageSpec& newspec) if (!GetDxgiFormatChannelMasks(m_dx10.dxgiFormat, m_dds.fmt.masks)) { errorfmt("Unsupported DXGI format: {}", m_dx10.dxgiFormat); + close(); return false; } break; @@ -564,6 +571,7 @@ DDSInput::open(const std::string& name, ImageSpec& newspec) } break; default: errorfmt("Unsupported compression type: {}", m_dds.fmt.fourCC); + close(); return false; } } @@ -627,6 +635,7 @@ DDSInput::open(const std::string& name, ImageSpec& newspec) if (m_compression == Compression::None && (m_Bpp < 1 || m_Bpp > 16)) { errorfmt("Invalid DDS bytes-per-pixel ({}). Possible corrupt input?", m_Bpp); + close(); return false; } @@ -648,8 +657,10 @@ DDSInput::open(const std::string& name, ImageSpec& newspec) } else m_nfaces = 1; - if (!seek_subimage(0, 0)) + if (!seek_subimage(0, 0)) { + close(); return false; + } newspec = spec(); return true; } diff --git a/src/dicom.imageio/dicominput.cpp b/src/dicom.imageio/dicominput.cpp index 381d171c31..bfef821921 100644 --- a/src/dicom.imageio/dicominput.cpp +++ b/src/dicom.imageio/dicominput.cpp @@ -109,7 +109,11 @@ DICOMInput::open(const std::string& name, ImageSpec& newspec, bool ok = seek_subimage(0, 0); newspec = spec(); - return ok; + if (!ok) { + close(); + return false; + } + return true; } diff --git a/src/dpx.imageio/dpxinput.cpp b/src/dpx.imageio/dpxinput.cpp index 878552fad8..c576d8633e 100644 --- a/src/dpx.imageio/dpxinput.cpp +++ b/src/dpx.imageio/dpxinput.cpp @@ -145,6 +145,7 @@ DPXInput::open(const std::string& name, ImageSpec& newspec) m_stream = new InStream(ioproxy()); if (!m_stream) { errorfmt("Could not open file \"{}\"", name); + close(); return false; } diff --git a/src/ffmpeg.imageio/ffmpeginput.cpp b/src/ffmpeg.imageio/ffmpeginput.cpp index 59f38f3825..2079d6e2f0 100644 --- a/src/ffmpeg.imageio/ffmpeginput.cpp +++ b/src/ffmpeg.imageio/ffmpeginput.cpp @@ -233,6 +233,7 @@ FFmpegInput::open(const std::string& name, ImageSpec& spec) } if (!valid_extension) { errorfmt("\"{}\" could not open input", name); + close(); return false; } @@ -245,6 +246,7 @@ FFmpegInput::open(const std::string& name, ImageSpec& spec) } if (avformat_find_stream_info(m_format_context, NULL) < 0) { errorfmt("\"{}\" could not find stream info", file_name); + close(); return false; } m_video_stream = -1; @@ -259,6 +261,7 @@ FFmpegInput::open(const std::string& name, ImageSpec& spec) } if (m_video_stream == -1) { errorfmt("\"{}\" could not find a valid videostream", file_name); + close(); return false; } for (unsigned int i = 0; i < m_format_context->nb_streams; i++) { @@ -276,12 +279,14 @@ FFmpegInput::open(const std::string& name, ImageSpec& spec) m_codec = avcodec_find_decoder(par->codec_id); if (!m_codec) { errorfmt("\"{}\" can't find decoder", file_name); + close(); return false; } m_codec_context = avcodec_alloc_context3(m_codec); if (!m_codec_context) { errorfmt("\"{}\" can't allocate decoder context", file_name); + close(); return false; } @@ -290,11 +295,13 @@ FFmpegInput::open(const std::string& name, ImageSpec& spec) ret = avcodec_parameters_to_context(m_codec_context, par); if (ret < 0) { errorfmt("\"{}\" unsupported codec", file_name); + close(); return false; } if (avcodec_open2(m_codec_context, m_codec, NULL) < 0) { errorfmt("\"{}\" could not open codec", file_name); + close(); return false; } if (!strcmp(m_codec_context->codec->name, "mjpeg") @@ -497,6 +504,7 @@ FFmpegInput::open(const std::string& name, ImageSpec& spec) m_codec_context->height, ffmpeg_image_align); if (rgb_buffer_size <= 0) { errorfmt("\"{}\" invalid FFmpeg RGB buffer size", file_name); + close(); return false; } m_rgb_buffer.resize(static_cast(rgb_buffer_size), 0); @@ -508,6 +516,7 @@ FFmpegInput::open(const std::string& name, ImageSpec& spec) NULL, NULL, NULL); if (!m_sws_rgb_context) { errorfmt("\"{}\" could not create FFmpeg scaling context", file_name); + close(); return false; } diff --git a/src/fits.imageio/fitsinput.cpp b/src/fits.imageio/fitsinput.cpp index 8ab0304fc3..d3f7925d9b 100644 --- a/src/fits.imageio/fitsinput.cpp +++ b/src/fits.imageio/fitsinput.cpp @@ -85,8 +85,10 @@ FitsInput::open(const std::string& name, ImageSpec& spec) if (!subimage_search()) return false; - if (!set_spec_info()) + if (!set_spec_info()) { + close(); return false; + } spec = m_spec; return true; diff --git a/src/heif.imageio/heifinput.cpp b/src/heif.imageio/heifinput.cpp index b135f4b8fe..ebe3e2981f 100644 --- a/src/heif.imageio/heifinput.cpp +++ b/src/heif.imageio/heifinput.cpp @@ -205,15 +205,19 @@ HeifInput::open(const std::string& name, ImageSpec& newspec, } catch (const heif::Error& err) { std::string e = err.get_message(); errorfmt("{}", e.empty() ? "unknown exception" : e.c_str()); + close(); return false; } catch (const std::exception& err) { std::string e = err.what(); errorfmt("{}", e.empty() ? "unknown exception" : e.c_str()); + close(); return false; } bool ok = seek_subimage(0, 0); newspec = spec(); + if (!ok) + close(); return ok; } diff --git a/src/ico.imageio/icoinput.cpp b/src/ico.imageio/icoinput.cpp index 946ceee911..b211c30cbd 100644 --- a/src/ico.imageio/icoinput.cpp +++ b/src/ico.imageio/icoinput.cpp @@ -137,8 +137,10 @@ ICOInput::open(const std::string& name, ImageSpec& newspec, ioseek(0); - if (!ioread(&m_ico, 1, sizeof(m_ico))) + if (!ioread(&m_ico, 1, sizeof(m_ico))) { + close(); return false; + } if (bigendian()) { // ICOs are little endian @@ -148,6 +150,7 @@ ICOInput::open(const std::string& name, ImageSpec& newspec, } if (m_ico.reserved != 0 || m_ico.type != 1) { errorfmt("File failed ICO header check"); + close(); return false; } diff --git a/src/iff.imageio/iffinput.cpp b/src/iff.imageio/iffinput.cpp index 602a4cf7b1..949fba043f 100644 --- a/src/iff.imageio/iffinput.cpp +++ b/src/iff.imageio/iffinput.cpp @@ -285,8 +285,10 @@ IffInput::open(const std::string& name, ImageSpec& spec) // Validity check resolutions and check for decompression bombs. if (!check_open(m_spec, { 0, 1 << 16, 0, 1 << 16, 0, 1, 0, 5 }) - || !check_compression_ratio(m_spec, ioproxy()->size())) + || !check_compression_ratio(m_spec, ioproxy()->size())) { + close(); return false; + } spec = m_spec; return true; diff --git a/src/include/OpenImageIO/imageio.h b/src/include/OpenImageIO/imageio.h index a20af2284c..39fb4c518e 100644 --- a/src/include/OpenImageIO/imageio.h +++ b/src/include/OpenImageIO/imageio.h @@ -1180,6 +1180,11 @@ class OIIO_API ImageInput { /// /// @returns /// `true` if the file was found and opened successfully. + /// + /// If `open()` returns `false`, the ImageInput will be left in the same + /// state as a newly constructed, unopened ImageInput (as if `close()` had + /// been called). Callers may assume this and are not responsible for + /// calling `close()` after a failed `open()`. OIIO_NODISCARD_ERROR virtual bool open (const std::string& name, ImageSpec &newspec) = 0; @@ -1208,6 +1213,9 @@ class OIIO_API ImageInput { /// /// @returns /// `true` if the file was found and opened successfully. + /// + /// Same close-on-failure contract as `open(name,newspec)`: a `false` + /// return must leave the ImageInput as if `close()` had been called. OIIO_NODISCARD_ERROR virtual bool open (const std::string& name, ImageSpec &newspec, const ImageSpec& config OIIO_MAYBE_UNUSED) { diff --git a/src/jpeg2000.imageio/jpeg2000input.cpp b/src/jpeg2000.imageio/jpeg2000input.cpp index 6913ef3e0a..1f7f9b8406 100644 --- a/src/jpeg2000.imageio/jpeg2000input.cpp +++ b/src/jpeg2000.imageio/jpeg2000input.cpp @@ -703,6 +703,7 @@ Jpeg2000Input::open(const std::string& name, ImageSpec& p_spec) if (!ok && OIIO::get_int_attribute("imageinput:strict")) { errorfmt("Possible corrupt file, could not decode ICC profile: {}\n", errormsg); + close(); return false; } } diff --git a/src/jpegxl.imageio/jxlinput.cpp b/src/jpegxl.imageio/jxlinput.cpp index 9e268ca74f..c03e794ce7 100644 --- a/src/jpegxl.imageio/jxlinput.cpp +++ b/src/jpegxl.imageio/jxlinput.cpp @@ -161,6 +161,7 @@ JxlInput::open(const std::string& name, ImageSpec& newspec) std::string proxytype = m_io->proxytype(); if (proxytype != "file" && proxytype != "memreader") { errorfmt("JPEG XL reader can't handle proxy type {}", proxytype); + close(); return false; } @@ -168,18 +169,21 @@ JxlInput::open(const std::string& name, ImageSpec& newspec) DBG std::cout << "JxlInput::valid_file() return false\n"; errorfmt("Possible corrupt file, " "JPEG XL signature verification failed\n"); + close(); return false; } m_decoder = JxlDecoderMake(nullptr); if (m_decoder == nullptr) { DBG std::cout << "JxlDecoderMake failed\n"; + close(); return false; } m_runner = JxlResizableParallelRunnerMake(nullptr); if (m_runner == nullptr) { DBG std::cout << "JxlThreadParallelRunnerMake failed\n"; + close(); return false; } @@ -187,6 +191,7 @@ JxlInput::open(const std::string& name, ImageSpec& newspec) m_decoder.get(), JxlResizableParallelRunner, m_runner.get()); if (status != JXL_DEC_SUCCESS) { DBG std::cout << "JxlDecoderSetParallelRunner failed\n"; + close(); return false; } @@ -196,6 +201,7 @@ JxlInput::open(const std::string& name, ImageSpec& newspec) | JXL_DEC_FRAME | JXL_DEC_FULL_IMAGE); if (status != JXL_DEC_SUCCESS) { DBG std::cout << "JxlDecoderSubscribeEvents failed\n"; + close(); return false; } @@ -216,6 +222,7 @@ JxlInput::open(const std::string& name, ImageSpec& newspec) status = JxlDecoderSetInput(m_decoder.get(), jxl.get(), size); if (status != JXL_DEC_SUCCESS) { DBG std::cout << "JxlDecoderSetInput() returned " << status << "\n"; + close(); return false; } JxlDecoderCloseInput(m_decoder.get()); @@ -226,6 +233,7 @@ JxlInput::open(const std::string& name, ImageSpec& newspec) const_cast(buffer.data()), buffer.size()); if (status != JXL_DEC_SUCCESS) { + close(); return false; } } @@ -247,11 +255,13 @@ JxlInput::open(const std::string& name, ImageSpec& newspec) DBG std::cout << "JXL_DEC_ERROR\n"; errorfmt("JPEG XL decoder error"); + close(); return false; } else if (status == JXL_DEC_NEED_MORE_INPUT) { DBG std::cout << "JXL_DEC_NEED_MORE_INPUT\n"; errorfmt("JPEG XL decoder error, already provided all input\n"); + close(); return false; } else if (status == JXL_DEC_BASIC_INFO) { DBG std::cout << "JXL_DEC_BASIC_INFO\n"; @@ -260,6 +270,7 @@ JxlInput::open(const std::string& name, ImageSpec& newspec) if (JXL_DEC_SUCCESS != JxlDecoderGetBasicInfo(m_decoder.get(), &info)) { errorfmt("JxlDecoderGetBasicInfo failed\n"); + close(); return false; } @@ -280,6 +291,7 @@ JxlInput::open(const std::string& name, ImageSpec& newspec) bits = 32; } else { errorfmt("Unsupported bits per sample\n"); + close(); return false; } @@ -294,6 +306,7 @@ JxlInput::open(const std::string& name, ImageSpec& newspec) m_spec = ImageSpec(info.xsize, info.ysize, m_channels, m_data_type); if (!check_open(m_spec, { 0, (1 << 30) - 1, 0, (1 << 30) - 1, 0, 1, 0, 4099 })) { + close(); return false; } got_basic_info = true; @@ -313,6 +326,7 @@ JxlInput::open(const std::string& name, ImageSpec& newspec) JXL_COLOR_PROFILE_TARGET_DATA, &icc_size)) { errorfmt("JxlDecoderGetICCProfileSize failed\n"); + close(); return false; } m_icc_profile.resize(icc_size); @@ -322,6 +336,7 @@ JxlInput::open(const std::string& name, ImageSpec& newspec) m_icc_profile.data(), m_icc_profile.size())) { errorfmt("JxlDecoderGetColorAsICCProfile failed\n"); + close(); return false; } @@ -342,6 +357,7 @@ JxlInput::open(const std::string& name, ImageSpec& newspec) != JxlDecoderImageOutBufferSize(m_decoder.get(), &format, &buffer_size)) { errorfmt("JxlDecoderImageOutBufferSize failed\n"); + close(); return false; } size_t expected_size = size_t(info.xsize) * info.ysize * m_channels @@ -349,6 +365,7 @@ JxlInput::open(const std::string& name, ImageSpec& newspec) if (buffer_size != expected_size) { errorfmt("Invalid out buffer size {} {}\n", buffer_size, expected_size); + close(); return false; } @@ -358,6 +375,7 @@ JxlInput::open(const std::string& name, ImageSpec& newspec) != JxlDecoderSetImageOutBuffer(m_decoder.get(), &format, m_buffer.get(), buffer_size)) { errorfmt("JxlDecoderSetImageOutBuffer failed\n"); + close(); return false; } } else if (status == JXL_DEC_FULL_IMAGE) { @@ -377,12 +395,14 @@ JxlInput::open(const std::string& name, ImageSpec& newspec) break; } else { errorfmt("Unknown decoder status\n"); + close(); return false; } } if (!got_basic_info) { errorfmt("Possible corrupt file, no JPEG XL basic info found\n"); + close(); return false; } @@ -409,6 +429,7 @@ JxlInput::open(const std::string& name, ImageSpec& newspec) if (!ok && OIIO::get_int_attribute("imageinput:strict")) { errorfmt("Possible corrupt file, could not decode ICC profile: {}\n", errormsg); + close(); return false; } } diff --git a/src/openexr.imageio/exrinput.cpp b/src/openexr.imageio/exrinput.cpp index b830ed54b7..dfd2f3a849 100644 --- a/src/openexr.imageio/exrinput.cpp +++ b/src/openexr.imageio/exrinput.cpp @@ -246,6 +246,7 @@ OpenEXRInput::open(const std::string& name, ImageSpec& newspec, if (!valid_file(m_io)) { errorfmt("\"{}\" is not an OpenEXR file", name); + close(); return false; } @@ -289,6 +290,7 @@ OpenEXRInput::open(const std::string& name, ImageSpec& newspec, std::string e = m_io->error(); errorfmt("Could not open \"{}\" ({})", name, e.size() ? e : std::string("unknown error")); + close(); return false; } m_io->seek(0); @@ -296,10 +298,12 @@ OpenEXRInput::open(const std::string& name, ImageSpec& newspec, } catch (const std::exception& e) { m_input_stream = NULL; errorfmt("OpenEXR exception: {}", e.what()); + close(); return false; } catch (...) { // catch-all for edge cases or compiler bugs m_input_stream = NULL; errorfmt("OpenEXR exception: unknown"); + close(); return false; } @@ -311,10 +315,12 @@ OpenEXRInput::open(const std::string& name, ImageSpec& newspec, delete m_input_stream; m_input_stream = NULL; errorfmt("OpenEXR exception: {}", e.what()); + close(); return false; } catch (...) { // catch-all for edge cases or compiler bugs m_input_stream = NULL; errorfmt("OpenEXR exception: unknown"); + close(); return false; } diff --git a/src/openvdb.imageio/openvdbinput.cpp b/src/openvdb.imageio/openvdbinput.cpp index 58c659269f..68d83245a4 100644 --- a/src/openvdb.imageio/openvdbinput.cpp +++ b/src/openvdb.imageio/openvdbinput.cpp @@ -522,8 +522,8 @@ OpenVDBInput::open(const std::string& filename, ImageSpec& newspec) readMetaData(*layer.grid, layer, layerspec); } } catch (const std::exception& e) { - init(); // Reset to initial state errorfmt("Could not open '{}': {}", filename, e.what()); + close(); // Reset to initial state return false; } m_name = filename; @@ -534,6 +534,8 @@ OpenVDBInput::open(const std::string& filename, ImageSpec& newspec) bool ok = seek_subimage(0, 0); newspec = ImageInput::spec(); + if (!ok) + close(); return ok; } diff --git a/src/png.imageio/pnginput.cpp b/src/png.imageio/pnginput.cpp index 55afc89ee8..a9b53d7d0c 100644 --- a/src/png.imageio/pnginput.cpp +++ b/src/png.imageio/pnginput.cpp @@ -160,6 +160,7 @@ PNGInput::open(const std::string& name, ImageSpec& newspec) || png_sig_cmp(sig, 0, 8)) { if (!has_error()) errorfmt("Not a PNG file"); + close(); return false; // Read failed } diff --git a/src/pnm.imageio/pnminput.cpp b/src/pnm.imageio/pnminput.cpp index 3cd8a55a64..daa965e834 100644 --- a/src/pnm.imageio/pnminput.cpp +++ b/src/pnm.imageio/pnminput.cpp @@ -488,6 +488,7 @@ PNMInput::open(const std::string& name, ImageSpec& newspec, if (!open(name, newspec)) { errorfmt("Could not parse spec for file \"%s\"", name); + close(); return false; } @@ -509,18 +510,24 @@ PNMInput::open(const std::string& name, ImageSpec& newspec) m_remaining = read_header_to_buffer(m_file_contents, m_io); m_pfm_flip = false; - if (!read_file_header()) + if (!read_file_header()) { + close(); return false; + } - if (!check_open(m_spec)) // check for apparently invalid values + if (!check_open(m_spec)) { // check for apparently invalid values + close(); return false; + } // Reject a tiny file that declares a huge image before the caller // allocates the full (declared) pixel buffer. Binary PNM data must be // present in the file and ASCII values cost >=2 bytes each, so a // legitimate file never has an extreme ratio. - if (!check_compression_ratio(m_spec, m_io->size())) + if (!check_compression_ratio(m_spec, m_io->size())) { + close(); return false; + } m_remaining = append_remainder_to_buffer(m_file_contents, m_io, m_remaining); diff --git a/src/psd.imageio/psdinput.cpp b/src/psd.imageio/psdinput.cpp index dd3473cd2f..6da41e395e 100644 --- a/src/psd.imageio/psdinput.cpp +++ b/src/psd.imageio/psdinput.cpp @@ -587,6 +587,7 @@ PSDInput::open(const std::string& name, ImageSpec& newspec) // File Header if (!load_header()) { errorfmt("failed to open \"{}\": failed load_header", name); + close(); return false; } @@ -610,36 +611,42 @@ PSDInput::open(const std::string& name, ImageSpec& newspec) // Color Mode Data if (!load_color_data()) { errorfmt("failed to open \"{}\": failed load_color_data", name); + close(); return false; } // Image Resources if (!load_resources()) { errorfmt("failed to open \"{}\": failed load_resources", name); + close(); return false; } // Layers if (!load_layers()) { errorfmt("failed to open \"{}\": failed load_layers", name); + close(); return false; } // Global Mask Info if (!load_global_mask_info()) { errorfmt("failed to open \"{}\": failed load_global_mask_info", name); + close(); return false; } // Global Additional Layer Info if (!load_global_additional()) { errorfmt("failed to open \"{}\": failed load_global_additional", name); + close(); return false; } // Image Data if (!load_image_data()) { errorfmt("failed to open \"{}\": failed load_image_data", name); + close(); return false; } diff --git a/src/ptex.imageio/ptexinput.cpp b/src/ptex.imageio/ptexinput.cpp index e59f00c3d6..d840f00174 100644 --- a/src/ptex.imageio/ptexinput.cpp +++ b/src/ptex.imageio/ptexinput.cpp @@ -334,15 +334,19 @@ PtexInput::open(const std::string& name, ImageSpec& newspec) m_ptex = NULL; } errorfmt("{}", perr.c_str()); + close(); return false; } m_numFaces = m_ptex->numFaces(); m_hasMipMaps = m_ptex->hasMipMaps(); - bool ok = seek_subimage(0, 0); + if (!seek_subimage(0, 0)) { + close(); + return false; + } newspec = spec(); - return ok; + return true; } diff --git a/src/r3d.imageio/r3dinput.cpp b/src/r3d.imageio/r3dinput.cpp index c5ba3a0c03..0875a884ec 100644 --- a/src/r3d.imageio/r3dinput.cpp +++ b/src/r3d.imageio/r3dinput.cpp @@ -266,6 +266,7 @@ R3dInput::open(const std::string& name, ImageSpec& newspec) DBG("Failed to allocate {} bytes of memory for output image\n", static_cast(memNeeded)); + close(); return false; } diff --git a/src/raw.imageio/rawinput.cpp b/src/raw.imageio/rawinput.cpp index e2539d5c9f..748a79b319 100644 --- a/src/raw.imageio/rawinput.cpp +++ b/src/raw.imageio/rawinput.cpp @@ -393,9 +393,12 @@ RawInput::open(const std::string& name, ImageSpec& newspec, // will need to close and re-open with unpack=true if and when we need // the actual pixel values. bool ok = open_raw(force_load, force_load, m_filename, m_config); - if (ok) - newspec = m_spec; - return ok; + if (!ok) { + close(); + return false; + } + newspec = m_spec; + return true; } diff --git a/src/rla.imageio/rlainput.cpp b/src/rla.imageio/rlainput.cpp index 9f73542f34..f71234c34e 100644 --- a/src/rla.imageio/rlainput.cpp +++ b/src/rla.imageio/rlainput.cpp @@ -169,8 +169,12 @@ RLAInput::open(const std::string& name, ImageSpec& newspec) m_subimage = 1; bool ok = seek_subimage(0, 0); + if (!ok) { + close(); + return false; + } newspec = spec(); - return ok; + return true; } diff --git a/src/sgi.imageio/sgiinput.cpp b/src/sgi.imageio/sgiinput.cpp index 2c1364628e..3f47d7761c 100644 --- a/src/sgi.imageio/sgiinput.cpp +++ b/src/sgi.imageio/sgiinput.cpp @@ -117,8 +117,10 @@ SgiInput::open(const std::string& name, ImageSpec& spec) return false; ioseek(0); - if (!read_header()) + if (!read_header()) { + close(); return false; + } if (m_sgi_header.magic != sgi_pvt::SGI_MAGIC) { errorfmt("\"{}\" is not a SGI file, magic number doesn't match", @@ -171,8 +173,10 @@ SgiInput::open(const std::string& name, ImageSpec& spec) if (m_sgi_header.storage == sgi_pvt::RLE) { m_spec.attribute("compression", "rle"); - if (!read_offset_tables()) + if (!read_offset_tables()) { + close(); return false; + } } spec = m_spec; diff --git a/src/targa.imageio/targainput.cpp b/src/targa.imageio/targainput.cpp index c6e6ccb824..252b592a4b 100644 --- a/src/targa.imageio/targainput.cpp +++ b/src/targa.imageio/targainput.cpp @@ -175,32 +175,38 @@ TGAInput::open(const std::string& name, ImageSpec& newspec) && read(m_tga.y_origin) && read(m_tga.width) && read(m_tga.height) && read(m_tga.bpp) && read(m_tga.attr))) { errorfmt("Could not read full header"); + close(); return false; } if (m_tga.cmap_type != 0 && m_tga.cmap_type != 1) { errorfmt("Illegal cmap_type value {} in header", m_tga.cmap_type); + close(); return false; } if (m_tga.type == TYPE_NODATA) { errorfmt("Image with no data"); + close(); return false; } if (m_tga.type != TYPE_PALETTED && m_tga.type != TYPE_RGB && m_tga.type != TYPE_GRAY && m_tga.type != TYPE_PALETTED_RLE && m_tga.type != TYPE_RGB_RLE && m_tga.type != TYPE_GRAY_RLE) { errorfmt("Illegal image type: {}", m_tga.type); + close(); return false; } if (m_tga.bpp != 8 && m_tga.bpp != 15 && m_tga.bpp != 16 && m_tga.bpp != 24 && m_tga.bpp != 32) { errorfmt("Illegal pixel size: {} bits per pixel", m_tga.bpp); + close(); return false; } if ((m_tga.type == TYPE_PALETTED || m_tga.type == TYPE_PALETTED_RLE) && !is_palette()) { errorfmt("Palette image with no palette"); + close(); return false; } if (is_palette()) { @@ -208,17 +214,20 @@ TGAInput::open(const std::string& name, ImageSpec& newspec) // it should be an error for TYPE_RGB* as well, but apparently some // *very* old TGAs can be this way, so we'll hack around it errorfmt("Palette defined for grayscale image"); + close(); return false; } if (m_tga.cmap_size != 15 && m_tga.cmap_size != 16 && m_tga.cmap_size != 24 && m_tga.cmap_size != 32) { errorfmt("Illegal palette entry size: {} bits", m_tga.cmap_size); + close(); return false; } if (m_tga.cmap_first + m_tga.cmap_length > (uint64_t(1) << m_tga.bpp)) { errorfmt( "Too big a color palette ({}) for {} bpp, assume corruption", m_tga.cmap_first + m_tga.cmap_length, m_tga.bpp); + close(); return false; } } @@ -278,6 +287,7 @@ TGAInput::open(const std::string& name, ImageSpec& newspec) errorfmt( "TGA header claims image size {} bytes, implausible for {} bytes of remaining file data", raw_pixel_bytes, avail); + close(); return false; } } @@ -292,8 +302,10 @@ TGAInput::open(const std::string& name, ImageSpec& newspec) // in case the comment lacks null termination char id[256]; memset(id, 0, sizeof(id)); - if (!ioread(id, m_tga.idlen, 1)) + if (!ioread(id, m_tga.idlen, 1)) { + close(); return false; + } m_spec.attribute("targa:ImageID", id); } @@ -305,6 +317,7 @@ TGAInput::open(const std::string& name, ImageSpec& newspec) bool check_for_tga2 = (ioproxy()->size() > 26 + 18); if (check_for_tga2 && !ioseek(-26, SEEK_END)) { errorfmt("Could not seek to find the TGA 2.0 signature."); + close(); return false; } if (check_for_tga2 && read(m_foot.ofs_ext) && read(m_foot.ofs_dev) @@ -312,15 +325,19 @@ TGAInput::open(const std::string& name, ImageSpec& newspec) && !strncmp(m_foot.signature, "TRUEVISION-XFILE.", 17)) { //std::cerr << "[tga] this is a TGA 2.0 file\n"; m_tga_version = 2; - if (!read_tga2_header()) + if (!read_tga2_header()) { + close(); return false; + } } else { m_tga_version = 1; } m_spec.attribute("targa:version", int(m_tga_version)); - if (!check_open(m_spec)) + if (!check_open(m_spec)) { + close(); return false; + } if (m_spec.alpha_channel != -1 && m_alpha_type == TGA_ALPHA_USEFUL && m_keep_unassociated_alpha) @@ -328,6 +345,7 @@ TGAInput::open(const std::string& name, ImageSpec& newspec) // Reposition back to where the palette starts if (!ioseek(ofs)) { + close(); return false; } diff --git a/src/tiff.imageio/tiffinput.cpp b/src/tiff.imageio/tiffinput.cpp index fdfcd4e77d..c090e2ce30 100644 --- a/src/tiff.imageio/tiffinput.cpp +++ b/src/tiff.imageio/tiffinput.cpp @@ -889,6 +889,8 @@ TIFFInput::open(const std::string& name, ImageSpec& newspec) bool ok = seek_subimage(0, 0); newspec = spec(); + if (!ok) + close(); return ok; } diff --git a/src/webp.imageio/webpinput.cpp b/src/webp.imageio/webpinput.cpp index d30b568f68..fae51b7bd4 100644 --- a/src/webp.imageio/webpinput.cpp +++ b/src/webp.imageio/webpinput.cpp @@ -141,6 +141,7 @@ WebpInput::open(const std::string& name, ImageSpec& spec, m_image_size = io->size(); if (m_image_size == size_t(-1)) { errorfmt("Failed to get size for \"{}\"", m_filename); + close(); return false; } @@ -211,6 +212,7 @@ WebpInput::open(const std::string& name, ImageSpec& spec, bool ok = decode_exif(exif_span, m_spec); if (!ok && OIIO::get_int_attribute("imageinput:strict")) { errorfmt("Could not decode Exif"); + close(); return false; } } @@ -237,6 +239,7 @@ WebpInput::open(const std::string& name, ImageSpec& spec, if (!ok && OIIO::get_int_attribute("imageinput:strict")) { errorfmt("Possible corrupt file, could not decode ICC profile: {}\n", errormsg); + close(); return false; } } @@ -245,8 +248,10 @@ WebpInput::open(const std::string& name, ImageSpec& spec, // BEFORE allocating the decoded-image buffer, so a malformed header cannot // drive a large allocation. if (!check_open(m_spec, { 0, (1 << 14) - 1, 0, (1 << 14) - 1, 0, 1, 0, 4 }) - || !check_compression_ratio(m_spec, m_image_size)) + || !check_compression_ratio(m_spec, m_image_size)) { + close(); return false; + } // Make space for the decoded image m_decoded_image.reset(new uint8_t[m_spec.image_bytes()]); From 0548339a7cf77ce5770661d98f0a09d825ea1395 Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Fri, 14 Aug 2026 16:56:24 -0700 Subject: [PATCH 2/2] Further fixes from @brechtvl Signed-off-by: Larry Gritz --- src/cineon.imageio/cineoninput.cpp | 1 + src/ffmpeg.imageio/ffmpeginput.cpp | 1 + src/fits.imageio/fitsinput.cpp | 5 ++++- src/heif.imageio/heifinput.cpp | 1 + src/jpeg.imageio/jpeginput.cpp | 20 ++++++++++++++++---- src/openexr.imageio/exrinput.cpp | 3 --- src/openexr.imageio/exrinput_c.cpp | 7 +++---- src/pnm.imageio/pnminput.cpp | 3 ++- src/r3d.imageio/r3dinput.cpp | 4 ++-- 9 files changed, 30 insertions(+), 15 deletions(-) diff --git a/src/cineon.imageio/cineoninput.cpp b/src/cineon.imageio/cineoninput.cpp index ad23052b45..6174e91630 100644 --- a/src/cineon.imageio/cineoninput.cpp +++ b/src/cineon.imageio/cineoninput.cpp @@ -82,6 +82,7 @@ CineonInput::open(const std::string& name, ImageSpec& newspec) m_stream = new InStream(); if (!m_stream->Open(name.c_str())) { errorfmt("Could not open file \"{}\"", name); + close(); return false; } diff --git a/src/ffmpeg.imageio/ffmpeginput.cpp b/src/ffmpeg.imageio/ffmpeginput.cpp index 2079d6e2f0..ba6a3a8b12 100644 --- a/src/ffmpeg.imageio/ffmpeginput.cpp +++ b/src/ffmpeg.imageio/ffmpeginput.cpp @@ -242,6 +242,7 @@ FFmpegInput::open(const std::string& name, ImageSpec& spec) if (avformat_open_input(&m_format_context, file_name, NULL, NULL) != 0) { // avformat_open_input allocs format_context errorfmt("\"{}\" could not open input", file_name); + close(); return false; } if (avformat_find_stream_info(m_format_context, NULL) < 0) { diff --git a/src/fits.imageio/fitsinput.cpp b/src/fits.imageio/fitsinput.cpp index d3f7925d9b..5dad013e3e 100644 --- a/src/fits.imageio/fitsinput.cpp +++ b/src/fits.imageio/fitsinput.cpp @@ -79,11 +79,14 @@ FitsInput::open(const std::string& name, ImageSpec& spec) // moving back to the start of the file if (Filesystem::fseek(m_fd, 0, SEEK_SET)) { errorfmt("Seek error"); + close(); return false; } - if (!subimage_search()) + if (!subimage_search()) { + close(); return false; + } if (!set_spec_info()) { close(); diff --git a/src/heif.imageio/heifinput.cpp b/src/heif.imageio/heifinput.cpp index ebe3e2981f..ed52db66ed 100644 --- a/src/heif.imageio/heifinput.cpp +++ b/src/heif.imageio/heifinput.cpp @@ -230,6 +230,7 @@ HeifInput::close() m_ihandle = heif::ImageHandle(); m_ctx.reset(); m_reader.reset(); + m_item_ids.clear(); m_subimage = -1; m_num_subimages = 0; m_associated_alpha = true; diff --git a/src/jpeg.imageio/jpeginput.cpp b/src/jpeg.imageio/jpeginput.cpp index 77c4556ad8..f67a70d1e3 100644 --- a/src/jpeg.imageio/jpeginput.cpp +++ b/src/jpeg.imageio/jpeginput.cpp @@ -213,6 +213,7 @@ JpgInput::open(const std::string& name, ImageSpec& newspec) std::string proxytype = m_io->proxytype(); if (proxytype != "file" && proxytype != "memreader") { errorfmt("JPEG reader can't handle proxy type {}", proxytype); + close(); return false; } @@ -266,6 +267,7 @@ JpgInput::open(const std::string& name, ImageSpec& newspec) // read the file parameters if (jpeg_read_header(&m_cinfo, FALSE) != JPEG_HEADER_OK || m_fatalerr) { errorfmt("Bad JPEG header for \"{}\"", filename()); + close(); return false; } @@ -287,21 +289,27 @@ JpgInput::open(const std::string& name, ImageSpec& newspec) TypeDesc::UINT8); // Validity check resolutions. - if (!check_open(m_spec, { 0, 1 << 16, 0, 1 << 16, 0, 1, 0, 3 })) + if (!check_open(m_spec, { 0, 1 << 16, 0, 1 << 16, 0, 1, 0, 3 })) { + close(); return false; + } // check_open's size cap still admits dimensions that are absurd for a tiny // file, so also bound the declared-vs-compressed ratio. imagesize_t filesize = m_io ? m_io->size() : Filesystem::file_size(name); - if (!check_compression_ratio(m_spec, filesize)) + if (!check_compression_ratio(m_spec, filesize)) { + close(); return false; + } if (m_raw) m_coeffs = jpeg_read_coefficients(&m_cinfo); else jpeg_start_decompress(&m_cinfo); // start working - if (m_fatalerr) + if (m_fatalerr) { + close(); return false; + } m_next_scanline = 0; // next scanline we'll read // The output dimensions ought to match the header's, but re-validate if @@ -311,8 +319,10 @@ JpgInput::open(const std::string& name, ImageSpec& newspec) m_spec = ImageSpec(m_cinfo.output_width, m_cinfo.output_height, nchannels, TypeDesc::UINT8); if (!check_open(m_spec, { 0, 1 << 16, 0, 1 << 16, 0, 1, 0, 3 }) - || !check_compression_ratio(m_spec, filesize)) + || !check_compression_ratio(m_spec, filesize)) { + close(); return false; + } } // Assume JPEG is in sRGB unless the Exif or XMP tags say otherwise. @@ -342,6 +352,7 @@ JpgInput::open(const std::string& name, ImageSpec& newspec) bool ok = decode_exif(exif, m_spec); if (!ok && OIIO::get_int_attribute("imageinput:strict")) { errorfmt("Could not decode Exif"); + close(); return false; } scan_for_thumbnail(exif); @@ -356,6 +367,7 @@ JpgInput::open(const std::string& name, ImageSpec& newspec) string_view((const char*)m->data, m->data_length)); if (!ok && OIIO::get_int_attribute("imageinput:strict")) { errorfmt("Corrupted IPTC data"); + close(); return false; } } else if (m->marker == JPEG_COM) { diff --git a/src/openexr.imageio/exrinput.cpp b/src/openexr.imageio/exrinput.cpp index dfd2f3a849..ac770dafa2 100644 --- a/src/openexr.imageio/exrinput.cpp +++ b/src/openexr.imageio/exrinput.cpp @@ -312,13 +312,10 @@ OpenEXRInput::open(const std::string& name, ImageSpec& newspec, try { m_input_multipart = new Imf::MultiPartInputFile(*m_input_stream); } catch (const std::exception& e) { - delete m_input_stream; - m_input_stream = NULL; errorfmt("OpenEXR exception: {}", e.what()); close(); return false; } catch (...) { // catch-all for edge cases or compiler bugs - m_input_stream = NULL; errorfmt("OpenEXR exception: unknown"); close(); return false; diff --git a/src/openexr.imageio/exrinput_c.cpp b/src/openexr.imageio/exrinput_c.cpp index 1b2c56847a..3c13b07f7c 100644 --- a/src/openexr.imageio/exrinput_c.cpp +++ b/src/openexr.imageio/exrinput_c.cpp @@ -404,6 +404,7 @@ OpenEXRCoreInput::open(const std::string& name, ImageSpec& newspec, std::string e = m_userdata.m_io->error(); errorfmt("Could not open \"{}\" ({})", name, e.size() ? e : std::string("unknown error")); + close(); return false; } m_userdata.m_io->seek(0); @@ -421,8 +422,7 @@ OpenEXRCoreInput::open(const std::string& name, ImageSpec& newspec, exr_result_t rv = exr_start_read(&m_exr_context, name.c_str(), &cinit); if (rv != EXR_ERR_SUCCESS) { // the error handler would have already reported the error into us - m_local_io.reset(); - m_userdata.m_io = nullptr; + close(); return false; } #if ENABLE_EXR_DEBUG_PRINTS || !defined(NDEBUG) /* allow debugging */ @@ -431,8 +431,7 @@ OpenEXRCoreInput::open(const std::string& name, ImageSpec& newspec, #endif rv = exr_get_count(m_exr_context, &m_nsubimages); if (rv != EXR_ERR_SUCCESS) { - m_local_io.reset(); - m_userdata.m_io = nullptr; + close(); return false; } diff --git a/src/pnm.imageio/pnminput.cpp b/src/pnm.imageio/pnminput.cpp index daa965e834..2a9f75fed0 100644 --- a/src/pnm.imageio/pnminput.cpp +++ b/src/pnm.imageio/pnminput.cpp @@ -66,6 +66,7 @@ class PNMInput final : public ImageInput { void init() { + m_file_contents.clear(); m_file_contents.shrink_to_fit(); ioproxy_clear(); m_y_next = 0; @@ -487,7 +488,7 @@ PNMInput::open(const std::string& name, ImageSpec& newspec, ioproxy_retrieve_from_config(config); if (!open(name, newspec)) { - errorfmt("Could not parse spec for file \"%s\"", name); + errorfmt("Could not parse spec for file \"{}\"", name); close(); return false; } diff --git a/src/r3d.imageio/r3dinput.cpp b/src/r3d.imageio/r3dinput.cpp index 0875a884ec..dfc4063587 100644 --- a/src/r3d.imageio/r3dinput.cpp +++ b/src/r3d.imageio/r3dinput.cpp @@ -234,8 +234,8 @@ R3dInput::open(const std::string& name, ImageSpec& newspec) if (m_clip->Status() != R3DSDK::LSClipLoaded) { DBG("Error loading {}\n", m_filename); - delete m_clip; - m_clip = nullptr; + errorfmt("Could not load R3D clip \"{}\"", m_filename); + close(); return false; }