Skip to content

Commit

Permalink
IOProxy read/write error handling improvements
Browse files Browse the repository at this point in the history
IOMemReader::pread catch out-of-range read positions.

Fixes #3711

Along the way, I noticed that the system pread & pwrite return a
ssize_t rather than a size_t as our IOProxy methods do, so I need to
catch the negative value returns that they use to indicate errors.
  • Loading branch information
lgritz committed Dec 17, 2022
1 parent 55fa360 commit 099ac2c
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 4 deletions.
6 changes: 6 additions & 0 deletions src/include/OpenImageIO/filesystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,12 @@ OIIO_UTIL_API std::string filename_to_regex(string_view pattern,
/// Proxy class for I/O. This provides a simplified interface for file I/O
/// that can have custom overrides. All char-based filenames are assumed to be
/// UTF-8 encoded.
///
/// Note: We intended for read, write, pread, pwrite to closely mimic the
/// standard C functions, but we erred, declaring these as returning size_t
/// instead of ssize_t. We can't change this now without breaking source
/// compatibility, so we have to live with it, but we'll probably change it at
/// the next major compatibility break (OIIO 3.0).
class OIIO_UTIL_API IOProxy {
public:
enum Mode { Closed = 0, Read = 'r', Write = 'w' };
Expand Down
21 changes: 17 additions & 4 deletions src/libutil/filesystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1303,7 +1303,9 @@ Filesystem::IOFile::pread(void* buf, size_t size, int64_t offset)
return r;
#else /* Non-Windows: assume POSIX pread is available */
int fd = fileno(m_file);
return ::pread(fd, buf, size, offset);
auto r = ::pread(fd, buf, size, offset);
// FIXME: the system pread returns ssize_t and is -1 on error.
return r < 0 ? size_t(0) : size_t(r);
#endif
}

Expand Down Expand Up @@ -1332,8 +1334,10 @@ Filesystem::IOFile::pwrite(const void* buf, size_t size, int64_t offset)
seek(origpos);
return r;
#else /* Non-Windows: assume POSIX pwrite is available */
int fd = fileno(m_file);
size_t r = ::pwrite(fd, buf, size, offset);
int fd = fileno(m_file);
auto r = ::pwrite(fd, buf, size, offset);
// FIXME: the system pwrite returns ssize_t and is -1 on error.
return r < 0 ? size_t(0) : size_t(r);
#endif
offset += r;
if (m_pos > int64_t(m_size))
Expand Down Expand Up @@ -1400,8 +1404,17 @@ size_t
Filesystem::IOMemReader::pread(void* buf, size_t size, int64_t offset)
{
// N.B. No lock necessary
if (size + size_t(offset) > size_t(m_buf.size()))
if (!m_buf.size() || !size)
return 0;
if (size + size_t(offset) > size_t(m_buf.size())) {
if (offset < 0 || offset >= m_buf.size()) {
error(Strutil::fmt::format(
"Invalid pread offset {} for an IOMemReader buffer of size {}",
offset, m_buf.size()));
return 0;
}
size = m_buf.size() - size_t(offset);
}
memcpy(buf, m_buf.data() + offset, size);
return size;
}
Expand Down

0 comments on commit 099ac2c

Please sign in to comment.