From 473b6fec26d1f46e07f2ea434f54b5724101e8e1 Mon Sep 17 00:00:00 2001 From: maxwelliverson Date: Sat, 16 May 2026 16:19:04 -0700 Subject: [PATCH 01/13] fix(pnm): Prevent PNM reader from loading arbitrarily large files to memory The PNM reader did not provide an efficient `valid_file` implementation. While `PNMInput::open` would perform basic validation of the PNM header, it would only do this after having read the entire contents of the file into memory. Besides the obvious risks/issues with having `open` unconditionally read arbitrarily large files into memory, this also meant that any call to `valid_file` would necessarily do the same (as the default implementation delegates to `open` when ioproxy support is present). To fix these issues, the following changes were made: - Provide an efficient implementation of `valid_file` for `PNMInput` that only loads the header, and then validates magic number/dimensions. - In `PNMInput::open`, first load only the header to memory, and subsequently, only load the rest of the image data if the header is parsed successfully, and the data contained within is valid. - Added a rough limit to header read size of 1KiB. - Added a rough limit to full file read size of 1GiB. Note that, for now, if the file size exceeds the 1GiB limit, the read is simply truncated to 1GiB, rather than failing alltogether. Signed-off-by: maxwelliverson --- src/pnm.imageio/pnminput.cpp | 194 ++++++++++++++++++++++++++++------- 1 file changed, 156 insertions(+), 38 deletions(-) diff --git a/src/pnm.imageio/pnminput.cpp b/src/pnm.imageio/pnminput.cpp index 396ac83ee8..defa4c6d58 100644 --- a/src/pnm.imageio/pnminput.cpp +++ b/src/pnm.imageio/pnminput.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include @@ -23,6 +24,13 @@ OIIO_PLUGIN_NAMESPACE_BEGIN // http://netpbm.sourceforge.net/doc/pam.html (base format) // +enum PNMType { P1, P2, P3, P4, P5, P6, Pf, PF }; + + + +using PNMBasicInfo = std::tuple; // type, width, height + + class PNMInput final : public ImageInput { public: @@ -40,10 +48,9 @@ class PNMInput final : public ImageInput { int current_subimage(void) const override { return 0; } bool read_native_scanline(int subimage, int miplevel, int y, int z, void* data) override; + bool valid_file (Filesystem::IOProxy* ioproxy) const override; private: - enum PNMType { P1, P2, P3, P4, P5, P6, Pf, PF }; - PNMType m_pnm_type; int m_max_val; float m_scaling_factor; @@ -63,17 +70,7 @@ class PNMInput final : public ImageInput { bool read_file_scanline(void* data, int y); bool read_file_header(); - void skipComments() - { - while (m_remaining.size() && Strutil::parse_char(m_remaining, '#')) - Strutil::parse_line(m_remaining); - } - - template bool nextVal(T& val) - { - skipComments(); - return Strutil::parse_value(m_remaining, val); - } + template bool nextVal(T& val); template bool ascii_to_raw(T* write, imagesize_t nvals, T max, bool invert = false); @@ -104,6 +101,67 @@ OIIO_EXPORT const char* pnm_input_extensions[] = { "ppm", "pgm", "pbm", OIIO_PLUGIN_EXPORTS_END +// 1KiB approximate limit on header size +static const imagesize_t max_pnm_header_size = 1024; + +// 1GiB rough limit on file size to avoid loading arbitrarily +// large files into memory +static const imagesize_t max_pnm_file_size = 1024*1024*1024; + + + +static string_view +read_header_to_buffer(std::vector& buffer, Filesystem::IOProxy* io) +{ + // couch std::min in parentheses to work around annoying windows.h defs + imagesize_t header_size = (std::min)(io->size(), max_pnm_header_size); + buffer.resize(header_size); + io->pread(buffer.data(), header_size, 0); + return string_view(buffer.data(), buffer.size()); +} + + + +static string_view +read_image_data_to_buffer(std::vector& buffer, Filesystem::IOProxy* io, + string_view remaining) +{ + // Assume we've already read the header into buffer + imagesize_t header_size = buffer.size(); + // couch std::min in parentheses to work around annoying windows.h defs + imagesize_t full_image_size = (std::min)(io->size(), max_pnm_file_size); + ptrdiff_t remaining_offset = remaining.data() - buffer.data(); + + buffer.resize(full_image_size); + io->pread(buffer.data() + header_size, full_image_size - header_size, + header_size); + + string_view result {buffer.data(), buffer.size()}; + result.remove_prefix(remaining_offset); + return result; +} + + + +inline static void +skip_header_comments(string_view& header) +{ + while (header.size() && Strutil::parse_char(header, '#')) + Strutil::parse_line(header); +} + + + +template +inline static bool +parse_next_header_value(string_view& header, T& val) +{ + skip_header_comments(header); + return Strutil::parse_value(header, val); +} + + + template inline void invert(const T* read, T* write, imagesize_t nvals) @@ -114,6 +172,15 @@ invert(const T* read, T* write, imagesize_t nvals) +template +bool +PNMInput::nextVal(T& val) +{ + return parse_next_header_value(m_remaining, val); +} + + + template bool PNMInput::ascii_to_raw(T* write, imagesize_t nvals, T max, bool invert) @@ -189,6 +256,39 @@ unpack_floats(const unsigned char* read, float* write, imagesize_t numsamples, +static std::optional +read_type_and_resolution(string_view& header) +{ + PNMType type; + + if (!Strutil::parse_char(header, 'P') || header.empty()) + return std::nullopt; + + switch (header.front()) { + case '1': type = P1; break; + case '2': type = P2; break; + case '3': type = P3; break; + case '4': type = P4; break; + case '5': type = P5; break; + case '6': type = P6; break; + case 'f': type = Pf; break; + case 'F': type = PF; break; + default: return std::nullopt; + } + header.remove_prefix(1); + + //Size + int width, height; + if (!parse_next_header_value(header, width)) + return std::nullopt; + if (!parse_next_header_value(header, height)) + return std::nullopt; + + return PNMBasicInfo{ type, width, height }; +} + + + bool PNMInput::read_file_scanline(void* data, int y) { @@ -277,28 +377,13 @@ PNMInput::read_file_scanline(void* data, int y) bool PNMInput::read_file_header() { - // MagicNumber - if (!Strutil::parse_char(m_remaining, 'P') || m_remaining.empty()) - return false; - switch (m_remaining.front()) { - case '1': m_pnm_type = P1; break; - case '2': m_pnm_type = P2; break; - case '3': m_pnm_type = P3; break; - case '4': m_pnm_type = P4; break; - case '5': m_pnm_type = P5; break; - case '6': m_pnm_type = P6; break; - case 'f': m_pnm_type = Pf; break; - case 'F': m_pnm_type = PF; break; - default: return false; - } - m_remaining.remove_prefix(1); - - //Size int width, height; - if (!nextVal(width)) - return false; - if (!nextVal(height)) + + if (auto basic_info = read_type_and_resolution(m_remaining)) { + std::tie(m_pnm_type, width, height) = *basic_info; + } else { return false; + } if (m_pnm_type != PF && m_pnm_type != Pf) { // Max Val @@ -312,7 +397,6 @@ PNMInput::read_file_header() if (!(m_remaining.size() && Strutil::isspace(m_remaining.front()))) return false; m_remaining.remove_prefix(1); - m_after_header = m_remaining; m_spec = ImageSpec(width, height, (m_pnm_type == P3 || m_pnm_type == P6) ? 3 : 1, @@ -332,7 +416,6 @@ PNMInput::read_file_header() if (!(m_remaining.size() && Strutil::isspace(m_remaining.front()))) return false; m_remaining.remove_prefix(1); - m_after_header = m_remaining; m_spec = ImageSpec(width, height, m_pnm_type == PF ? 3 : 1, TypeDesc::FLOAT); @@ -371,9 +454,7 @@ PNMInput::open(const std::string& name, ImageSpec& newspec) // Read the whole file's contents into m_file_contents Filesystem::IOProxy* m_io = ioproxy(); - m_file_contents.resize(m_io->size()); - m_io->pread(m_file_contents.data(), m_file_contents.size(), 0); - m_remaining = string_view(m_file_contents.data(), m_file_contents.size()); + m_remaining = read_header_to_buffer(m_file_contents, m_io); m_pfm_flip = false; if (!read_file_header()) @@ -382,7 +463,11 @@ PNMInput::open(const std::string& name, ImageSpec& newspec) if (!check_open(m_spec)) // check for apparently invalid values return false; + m_remaining = read_image_data_to_buffer(m_file_contents, m_io, + m_remaining); + m_after_header = m_remaining; newspec = m_spec; + return true; } @@ -412,4 +497,37 @@ PNMInput::read_native_scanline(int subimage, int miplevel, int y, int z, return true; } + + +bool +PNMInput::valid_file(Filesystem::IOProxy* ioproxy) const +{ + DBG std::cout << "PNMInput::valid_file()\n"; + + if (!ioproxy || ioproxy->mode() != Filesystem::IOProxy::Mode::Read) + return false; + + std::vector buffer; + string_view header = read_header_to_buffer(buffer, ioproxy); + + int width, height; + if (auto basic_info = read_type_and_resolution(header)) { + std::tie(std::ignore, width, height) = *basic_info; + } else { + return false; + } + + // Per spec, width and height must both be positive integers. + // No formal upper limit is placed on width/height, but for sanity, + // assume dimensions should be no greater than 2^12 + if (width < 0 || 4096 < width) + return false; + if (height < 0 || 4096 < height) + return false; + + DBG std::cout << "PNMInput::valid_file returned true\n"; + + return true; +} + OIIO_PLUGIN_NAMESPACE_END From 6ce299719bd02c182421305564422796e254614b Mon Sep 17 00:00:00 2001 From: maxwelliverson Date: Sat, 16 May 2026 16:24:13 -0700 Subject: [PATCH 02/13] fix(jxl): Prevent JXL reader from loading arbitrarily large files to memory The JXL reader already provided an efficient `valid_file` override, implemented by validating the 128 byte JXL signature. The signature, however, was not being validated in `JxlInput::open` before attempting to decode the file. The process of JXL decoding appears to read the entire contents of the file to memory (at least when provided non-JXL input). Taken in combination, this means that when `JXLInput::open` was called on arbitrarily large non-JXL inputs, the entire file was being read into memory before open would fail. As a simple fix for this, `JxlInput::open` now checks the result of `valid_file` before attempting any decoding. Signed-off-by: maxwelliverson --- src/jpegxl.imageio/jxlinput.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/jpegxl.imageio/jxlinput.cpp b/src/jpegxl.imageio/jxlinput.cpp index 88f2ccea06..da2293352f 100644 --- a/src/jpegxl.imageio/jxlinput.cpp +++ b/src/jpegxl.imageio/jxlinput.cpp @@ -164,6 +164,13 @@ JxlInput::open(const std::string& name, ImageSpec& newspec) return false; } + if (!valid_file(m_io)) { + DBG std::cout << "JxlInput::valid_file() return false\n"; + errorfmt("Possible corrupt file, " + "JPEG XL signature verification failed\n"); + return false; + } + m_decoder = JxlDecoderMake(nullptr); if (m_decoder == nullptr) { DBG std::cout << "JxlDecoderMake failed\n"; From 4b9adb6e8862b914457bdb87d77927a51b5cc891 Mon Sep 17 00:00:00 2001 From: maxwelliverson Date: Sat, 16 May 2026 17:33:43 -0700 Subject: [PATCH 03/13] fix: Fix failed std::min template type deduction on MacOS Signed-off-by: maxwelliverson --- src/pnm.imageio/pnminput.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/pnm.imageio/pnminput.cpp b/src/pnm.imageio/pnminput.cpp index defa4c6d58..2f0865ee0a 100644 --- a/src/pnm.imageio/pnminput.cpp +++ b/src/pnm.imageio/pnminput.cpp @@ -114,7 +114,8 @@ static string_view read_header_to_buffer(std::vector& buffer, Filesystem::IOProxy* io) { // couch std::min in parentheses to work around annoying windows.h defs - imagesize_t header_size = (std::min)(io->size(), max_pnm_header_size); + imagesize_t header_size = (std::min)(static_cast(io->size()), + max_pnm_header_size); buffer.resize(header_size); io->pread(buffer.data(), header_size, 0); return string_view(buffer.data(), buffer.size()); @@ -129,11 +130,12 @@ read_image_data_to_buffer(std::vector& buffer, Filesystem::IOProxy* io, // Assume we've already read the header into buffer imagesize_t header_size = buffer.size(); // couch std::min in parentheses to work around annoying windows.h defs - imagesize_t full_image_size = (std::min)(io->size(), max_pnm_file_size); + imagesize_t full_size = (std::min)(static_cast(io->size()), + max_pnm_file_size); ptrdiff_t remaining_offset = remaining.data() - buffer.data(); - buffer.resize(full_image_size); - io->pread(buffer.data() + header_size, full_image_size - header_size, + buffer.resize(full_size); + io->pread(buffer.data() + header_size, full_size - header_size, header_size); string_view result {buffer.data(), buffer.size()}; From 0e6281da3612d39c95a134429330ac4a46dfb30d Mon Sep 17 00:00:00 2001 From: maxwelliverson Date: Sat, 16 May 2026 17:47:56 -0700 Subject: [PATCH 04/13] style: Adjusted PNMInput changes to better adhere to clang-format rules Signed-off-by: maxwelliverson --- src/pnm.imageio/pnminput.cpp | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/src/pnm.imageio/pnminput.cpp b/src/pnm.imageio/pnminput.cpp index 2f0865ee0a..d88fa005e5 100644 --- a/src/pnm.imageio/pnminput.cpp +++ b/src/pnm.imageio/pnminput.cpp @@ -28,7 +28,7 @@ enum PNMType { P1, P2, P3, P4, P5, P6, Pf, PF }; -using PNMBasicInfo = std::tuple; // type, width, height +using PNMBasicInfo = std::tuple; // type, width, height @@ -48,7 +48,7 @@ class PNMInput final : public ImageInput { int current_subimage(void) const override { return 0; } bool read_native_scanline(int subimage, int miplevel, int y, int z, void* data) override; - bool valid_file (Filesystem::IOProxy* ioproxy) const override; + bool valid_file(Filesystem::IOProxy* ioproxy) const override; private: PNMType m_pnm_type; @@ -106,7 +106,7 @@ static const imagesize_t max_pnm_header_size = 1024; // 1GiB rough limit on file size to avoid loading arbitrarily // large files into memory -static const imagesize_t max_pnm_file_size = 1024*1024*1024; +static const imagesize_t max_pnm_file_size = 1024 * 1024 * 1024; @@ -130,15 +130,15 @@ read_image_data_to_buffer(std::vector& buffer, Filesystem::IOProxy* io, // Assume we've already read the header into buffer imagesize_t header_size = buffer.size(); // couch std::min in parentheses to work around annoying windows.h defs - imagesize_t full_size = (std::min)(static_cast(io->size()), - max_pnm_file_size); + imagesize_t full_size = (std::min)(static_cast(io->size()), + max_pnm_file_size); ptrdiff_t remaining_offset = remaining.data() - buffer.data(); buffer.resize(full_size); io->pread(buffer.data() + header_size, full_size - header_size, - header_size); + header_size); - string_view result {buffer.data(), buffer.size()}; + string_view result { buffer.data(), buffer.size() }; result.remove_prefix(remaining_offset); return result; } @@ -154,7 +154,7 @@ skip_header_comments(string_view& header) -template +template inline static bool parse_next_header_value(string_view& header, T& val) { @@ -286,7 +286,7 @@ read_type_and_resolution(string_view& header) if (!parse_next_header_value(header, height)) return std::nullopt; - return PNMBasicInfo{ type, width, height }; + return PNMBasicInfo { type, width, height }; } @@ -456,8 +456,8 @@ PNMInput::open(const std::string& name, ImageSpec& newspec) // Read the whole file's contents into m_file_contents Filesystem::IOProxy* m_io = ioproxy(); - m_remaining = read_header_to_buffer(m_file_contents, m_io); - m_pfm_flip = false; + m_remaining = read_header_to_buffer(m_file_contents, m_io); + m_pfm_flip = false; if (!read_file_header()) return false; @@ -465,10 +465,9 @@ PNMInput::open(const std::string& name, ImageSpec& newspec) if (!check_open(m_spec)) // check for apparently invalid values return false; - m_remaining = read_image_data_to_buffer(m_file_contents, m_io, - m_remaining); + m_remaining = read_image_data_to_buffer(m_file_contents, m_io, m_remaining); m_after_header = m_remaining; - newspec = m_spec; + newspec = m_spec; return true; } @@ -501,7 +500,7 @@ PNMInput::read_native_scanline(int subimage, int miplevel, int y, int z, -bool +bool PNMInput::valid_file(Filesystem::IOProxy* ioproxy) const { DBG std::cout << "PNMInput::valid_file()\n"; From 5eb60166baa60d48dbd82c856126f42efba2454a Mon Sep 17 00:00:00 2001 From: maxwelliverson Date: Sat, 16 May 2026 18:05:17 -0700 Subject: [PATCH 05/13] style: Removed unnecessary protection against windows.h min/max macro defs Signed-off-by: maxwelliverson --- src/pnm.imageio/pnminput.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/pnm.imageio/pnminput.cpp b/src/pnm.imageio/pnminput.cpp index d88fa005e5..be69c24fff 100644 --- a/src/pnm.imageio/pnminput.cpp +++ b/src/pnm.imageio/pnminput.cpp @@ -113,8 +113,7 @@ static const imagesize_t max_pnm_file_size = 1024 * 1024 * 1024; static string_view read_header_to_buffer(std::vector& buffer, Filesystem::IOProxy* io) { - // couch std::min in parentheses to work around annoying windows.h defs - imagesize_t header_size = (std::min)(static_cast(io->size()), + imagesize_t header_size = std::min(static_cast(io->size()), max_pnm_header_size); buffer.resize(header_size); io->pread(buffer.data(), header_size, 0); @@ -129,9 +128,8 @@ read_image_data_to_buffer(std::vector& buffer, Filesystem::IOProxy* io, { // Assume we've already read the header into buffer imagesize_t header_size = buffer.size(); - // couch std::min in parentheses to work around annoying windows.h defs - imagesize_t full_size = (std::min)(static_cast(io->size()), - max_pnm_file_size); + imagesize_t full_size = std::min(static_cast(io->size()), + max_pnm_file_size); ptrdiff_t remaining_offset = remaining.data() - buffer.data(); buffer.resize(full_size); From df4869c996e87da66ed11ad2734d090a2ae6359f Mon Sep 17 00:00:00 2001 From: maxwelliverson Date: Sat, 16 May 2026 18:39:22 -0700 Subject: [PATCH 06/13] style: Fixed a couple clang-format issues that were previously missed Signed-off-by: maxwelliverson --- src/pnm.imageio/pnminput.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/pnm.imageio/pnminput.cpp b/src/pnm.imageio/pnminput.cpp index be69c24fff..afc0593f46 100644 --- a/src/pnm.imageio/pnminput.cpp +++ b/src/pnm.imageio/pnminput.cpp @@ -127,13 +127,13 @@ read_image_data_to_buffer(std::vector& buffer, Filesystem::IOProxy* io, string_view remaining) { // Assume we've already read the header into buffer - imagesize_t header_size = buffer.size(); - imagesize_t full_size = std::min(static_cast(io->size()), - max_pnm_file_size); + imagesize_t header_size = buffer.size(); + imagesize_t full_size = std::min(static_cast(io->size()), + max_pnm_file_size); ptrdiff_t remaining_offset = remaining.data() - buffer.data(); buffer.resize(full_size); - io->pread(buffer.data() + header_size, full_size - header_size, + io->pread(buffer.data() + header_size, full_size - header_size, header_size); string_view result { buffer.data(), buffer.size() }; From 22bd8de0ba6888b9857726566b11abc94327a796 Mon Sep 17 00:00:00 2001 From: maxwelliverson Date: Sat, 16 May 2026 19:04:35 -0700 Subject: [PATCH 07/13] style: Added comments/symbol rename to better document new PNM read functions Signed-off-by: maxwelliverson --- src/pnm.imageio/pnminput.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/pnm.imageio/pnminput.cpp b/src/pnm.imageio/pnminput.cpp index afc0593f46..6c3f0c7033 100644 --- a/src/pnm.imageio/pnminput.cpp +++ b/src/pnm.imageio/pnminput.cpp @@ -110,6 +110,7 @@ static const imagesize_t max_pnm_file_size = 1024 * 1024 * 1024; +// Read only enough of the file to contain the header (at momst 1KB) into buffer static string_view read_header_to_buffer(std::vector& buffer, Filesystem::IOProxy* io) { @@ -122,9 +123,12 @@ read_header_to_buffer(std::vector& buffer, Filesystem::IOProxy* io) +// buffer contains at most the first 1K of the file. At this point, we know +// the file seems valid. Read the rest in, appending to what we have, and +// return the adjusted string_view of the contents. static string_view -read_image_data_to_buffer(std::vector& buffer, Filesystem::IOProxy* io, - string_view remaining) +append_remainder_to_buffer(std::vector& buffer, Filesystem::IOProxy* io, + string_view remaining) { // Assume we've already read the header into buffer imagesize_t header_size = buffer.size(); From 9b1a08bcbcb929874d697a9934123dccd365174e Mon Sep 17 00:00:00 2001 From: maxwelliverson Date: Sat, 16 May 2026 19:07:51 -0700 Subject: [PATCH 08/13] fix: Fixed missed reference to renamed append_remainder_to_buffer symbol Signed-off-by: maxwelliverson --- src/pnm.imageio/pnminput.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pnm.imageio/pnminput.cpp b/src/pnm.imageio/pnminput.cpp index 6c3f0c7033..a12921656e 100644 --- a/src/pnm.imageio/pnminput.cpp +++ b/src/pnm.imageio/pnminput.cpp @@ -467,7 +467,7 @@ PNMInput::open(const std::string& name, ImageSpec& newspec) if (!check_open(m_spec)) // check for apparently invalid values return false; - m_remaining = read_image_data_to_buffer(m_file_contents, m_io, m_remaining); + m_remaining = append_remainder_to_buffer(m_file_contents, m_io, m_remaining); m_after_header = m_remaining; newspec = m_spec; From 9812245587d63d058a5b11f43cc9abda630de92b Mon Sep 17 00:00:00 2001 From: maxwelliverson Date: Sat, 16 May 2026 19:09:47 -0700 Subject: [PATCH 09/13] refactor: Move definition of PNMInput::valid_file to before PNMInput::open Signed-off-by: maxwelliverson --- src/pnm.imageio/pnminput.cpp | 65 ++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 32 deletions(-) diff --git a/src/pnm.imageio/pnminput.cpp b/src/pnm.imageio/pnminput.cpp index a12921656e..f27e84ffcc 100644 --- a/src/pnm.imageio/pnminput.cpp +++ b/src/pnm.imageio/pnminput.cpp @@ -432,6 +432,39 @@ PNMInput::read_file_header() +bool +PNMInput::valid_file(Filesystem::IOProxy* ioproxy) const +{ + DBG std::cout << "PNMInput::valid_file()\n"; + + if (!ioproxy || ioproxy->mode() != Filesystem::IOProxy::Mode::Read) + return false; + + std::vector buffer; + string_view header = read_header_to_buffer(buffer, ioproxy); + + int width, height; + if (auto basic_info = read_type_and_resolution(header)) { + std::tie(std::ignore, width, height) = *basic_info; + } else { + return false; + } + + // Per spec, width and height must both be positive integers. + // No formal upper limit is placed on width/height, but for sanity, + // assume dimensions should be no greater than 2^12 + if (width < 0 || 4096 < width) + return false; + if (height < 0 || 4096 < height) + return false; + + DBG std::cout << "PNMInput::valid_file returned true\n"; + + return true; +} + + + bool PNMInput::open(const std::string& name, ImageSpec& newspec, const ImageSpec& config) @@ -501,36 +534,4 @@ PNMInput::read_native_scanline(int subimage, int miplevel, int y, int z, } - -bool -PNMInput::valid_file(Filesystem::IOProxy* ioproxy) const -{ - DBG std::cout << "PNMInput::valid_file()\n"; - - if (!ioproxy || ioproxy->mode() != Filesystem::IOProxy::Mode::Read) - return false; - - std::vector buffer; - string_view header = read_header_to_buffer(buffer, ioproxy); - - int width, height; - if (auto basic_info = read_type_and_resolution(header)) { - std::tie(std::ignore, width, height) = *basic_info; - } else { - return false; - } - - // Per spec, width and height must both be positive integers. - // No formal upper limit is placed on width/height, but for sanity, - // assume dimensions should be no greater than 2^12 - if (width < 0 || 4096 < width) - return false; - if (height < 0 || 4096 < height) - return false; - - DBG std::cout << "PNMInput::valid_file returned true\n"; - - return true; -} - OIIO_PLUGIN_NAMESPACE_END From 6f318c7d476168348fd638302840d4944105252d Mon Sep 17 00:00:00 2001 From: maxwelliverson Date: Sat, 16 May 2026 20:39:57 -0700 Subject: [PATCH 10/13] refactor: Change PNMBasicInfo from std::tuple alias to POD struct Signed-off-by: maxwelliverson --- src/pnm.imageio/pnminput.cpp | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/pnm.imageio/pnminput.cpp b/src/pnm.imageio/pnminput.cpp index f27e84ffcc..6f91e37398 100644 --- a/src/pnm.imageio/pnminput.cpp +++ b/src/pnm.imageio/pnminput.cpp @@ -28,7 +28,11 @@ enum PNMType { P1, P2, P3, P4, P5, P6, Pf, PF }; -using PNMBasicInfo = std::tuple; // type, width, height +struct PNMBasicInfo { + PNMType type; + int width; + int height; +}; @@ -384,7 +388,9 @@ PNMInput::read_file_header() int width, height; if (auto basic_info = read_type_and_resolution(m_remaining)) { - std::tie(m_pnm_type, width, height) = *basic_info; + m_pnm_type = basic_info->type; + width = basic_info->width; + height = basic_info->height; } else { return false; } @@ -445,7 +451,8 @@ PNMInput::valid_file(Filesystem::IOProxy* ioproxy) const int width, height; if (auto basic_info = read_type_and_resolution(header)) { - std::tie(std::ignore, width, height) = *basic_info; + width = basic_info->width; + height = basic_info->height; } else { return false; } @@ -500,7 +507,8 @@ PNMInput::open(const std::string& name, ImageSpec& newspec) if (!check_open(m_spec)) // check for apparently invalid values return false; - m_remaining = append_remainder_to_buffer(m_file_contents, m_io, m_remaining); + m_remaining = append_remainder_to_buffer(m_file_contents, m_io, + m_remaining); m_after_header = m_remaining; newspec = m_spec; From 246780f4eae2eea1e0106537ebfeb210623a50ed Mon Sep 17 00:00:00 2001 From: maxwelliverson Date: Sat, 16 May 2026 21:37:59 -0700 Subject: [PATCH 11/13] style: Conform to clang-format once and for all Signed-off-by: maxwelliverson --- src/pnm.imageio/pnminput.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pnm.imageio/pnminput.cpp b/src/pnm.imageio/pnminput.cpp index 6f91e37398..1cdfbb81f7 100644 --- a/src/pnm.imageio/pnminput.cpp +++ b/src/pnm.imageio/pnminput.cpp @@ -507,8 +507,8 @@ PNMInput::open(const std::string& name, ImageSpec& newspec) if (!check_open(m_spec)) // check for apparently invalid values return false; - m_remaining = append_remainder_to_buffer(m_file_contents, m_io, - m_remaining); + m_remaining = append_remainder_to_buffer(m_file_contents, m_io, + m_remaining); m_after_header = m_remaining; newspec = m_spec; From 0fc67b11a1904422e665e41b388837be29c1fc67 Mon Sep 17 00:00:00 2001 From: maxwelliverson Date: Sat, 16 May 2026 22:12:43 -0700 Subject: [PATCH 12/13] refactor: Move PNM reading functions into PNMInput class as static methods Signed-off-by: maxwelliverson --- src/pnm.imageio/pnminput.cpp | 82 ++++++++++++++++++++---------------- 1 file changed, 45 insertions(+), 37 deletions(-) diff --git a/src/pnm.imageio/pnminput.cpp b/src/pnm.imageio/pnminput.cpp index 1cdfbb81f7..93250029e8 100644 --- a/src/pnm.imageio/pnminput.cpp +++ b/src/pnm.imageio/pnminput.cpp @@ -74,6 +74,12 @@ class PNMInput final : public ImageInput { bool read_file_scanline(void* data, int y); bool read_file_header(); + static string_view read_header_to_buffer(std::vector& buffer, + Filesystem::IOProxy* io); + static string_view append_remainder_to_buffer(std::vector& buffer, + Filesystem::IOProxy* io, + string_view remaining); + template bool nextVal(T& val); template @@ -114,43 +120,6 @@ static const imagesize_t max_pnm_file_size = 1024 * 1024 * 1024; -// Read only enough of the file to contain the header (at momst 1KB) into buffer -static string_view -read_header_to_buffer(std::vector& buffer, Filesystem::IOProxy* io) -{ - imagesize_t header_size = std::min(static_cast(io->size()), - max_pnm_header_size); - buffer.resize(header_size); - io->pread(buffer.data(), header_size, 0); - return string_view(buffer.data(), buffer.size()); -} - - - -// buffer contains at most the first 1K of the file. At this point, we know -// the file seems valid. Read the rest in, appending to what we have, and -// return the adjusted string_view of the contents. -static string_view -append_remainder_to_buffer(std::vector& buffer, Filesystem::IOProxy* io, - string_view remaining) -{ - // Assume we've already read the header into buffer - imagesize_t header_size = buffer.size(); - imagesize_t full_size = std::min(static_cast(io->size()), - max_pnm_file_size); - ptrdiff_t remaining_offset = remaining.data() - buffer.data(); - - buffer.resize(full_size); - io->pread(buffer.data() + header_size, full_size - header_size, - header_size); - - string_view result { buffer.data(), buffer.size() }; - result.remove_prefix(remaining_offset); - return result; -} - - - inline static void skip_header_comments(string_view& header) { @@ -438,6 +407,45 @@ PNMInput::read_file_header() +// Read only enough of the file to contain the header (at momst 1KB) into buffer +string_view +PNMInput::read_header_to_buffer(std::vector& buffer, + Filesystem::IOProxy* io) +{ + imagesize_t header_size = std::min(static_cast(io->size()), + max_pnm_header_size); + buffer.resize(header_size); + io->pread(buffer.data(), header_size, 0); + return string_view(buffer.data(), buffer.size()); +} + + + +// buffer contains at most the first 1K of the file. At this point, we know +// the file seems valid. Read the rest in, appending to what we have, and +// return the adjusted string_view of the contents. +string_view +PNMInput::append_remainder_to_buffer(std::vector& buffer, + Filesystem::IOProxy* io, + string_view remaining) +{ + // Assume we've already read the header into buffer + imagesize_t header_size = buffer.size(); + imagesize_t full_size = std::min(static_cast(io->size()), + max_pnm_file_size); + ptrdiff_t remaining_offset = remaining.data() - buffer.data(); + + buffer.resize(full_size); + io->pread(buffer.data() + header_size, full_size - header_size, + header_size); + + string_view result { buffer.data(), buffer.size() }; + result.remove_prefix(remaining_offset); + return result; +} + + + bool PNMInput::valid_file(Filesystem::IOProxy* ioproxy) const { From 31260c568b5dc59a9fb4da25f55bcc3a45f70caf Mon Sep 17 00:00:00 2001 From: maxwelliverson Date: Mon, 18 May 2026 19:55:32 -0700 Subject: [PATCH 13/13] style: Remove trailing spaces Signed-off-by: maxwelliverson --- src/pnm.imageio/pnminput.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pnm.imageio/pnminput.cpp b/src/pnm.imageio/pnminput.cpp index 93250029e8..959d3a749e 100644 --- a/src/pnm.imageio/pnminput.cpp +++ b/src/pnm.imageio/pnminput.cpp @@ -260,7 +260,7 @@ read_type_and_resolution(string_view& header) return std::nullopt; if (!parse_next_header_value(header, height)) return std::nullopt; - + return PNMBasicInfo { type, width, height }; } @@ -438,7 +438,7 @@ PNMInput::append_remainder_to_buffer(std::vector& buffer, buffer.resize(full_size); io->pread(buffer.data() + header_size, full_size - header_size, header_size); - + string_view result { buffer.data(), buffer.size() }; result.remove_prefix(remaining_offset); return result; @@ -456,7 +456,7 @@ PNMInput::valid_file(Filesystem::IOProxy* ioproxy) const std::vector buffer; string_view header = read_header_to_buffer(buffer, ioproxy); - + int width, height; if (auto basic_info = read_type_and_resolution(header)) { width = basic_info->width;