From d2e4c3cbbf06bb5833d2272a4dcfba8d00113f83 Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Fri, 28 Jan 2022 21:29:11 -0800 Subject: [PATCH] Avoid unsafe isspace() (#3310) C isspace() takes an int, but on Windows when building in debug mode, the implementation of isspace() can hit an assert if the value is < 0, which of course it can be if you're looking at a character in a string whose high bit is set (because char is signed). Easiest solution is to make our own Strutil::isspace that does exactly what we want and is safe. Fixes #3300 --- src/hdr.imageio/rgbe.cpp | 2 +- src/include/OpenImageIO/strutil.h | 8 ++++++++ src/libutil/strutil.cpp | 12 ++++++------ src/pnm.imageio/pnminput.cpp | 6 +++--- 4 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/hdr.imageio/rgbe.cpp b/src/hdr.imageio/rgbe.cpp index cf7d7e113e..b00ef04ec0 100644 --- a/src/hdr.imageio/rgbe.cpp +++ b/src/hdr.imageio/rgbe.cpp @@ -218,7 +218,7 @@ int RGBE_ReadHeader(FILE *fp, int *width, int *height, rgbe_header_info *info, else if (info) { info->valid |= RGBE_VALID_PROGRAMTYPE; for(i=0;i<(int)sizeof(info->programtype)-1;i++) { - if ((buf[i+2] == 0) || isspace(buf[i+2])) + if ((buf[i+2] == 0) || Strutil::isspace(buf[i+2])) break; info->programtype[i] = buf[i+2]; } diff --git a/src/include/OpenImageIO/strutil.h b/src/include/OpenImageIO/strutil.h index fc0dd3d462..bd9ea43e76 100644 --- a/src/include/OpenImageIO/strutil.h +++ b/src/include/OpenImageIO/strutil.h @@ -841,6 +841,14 @@ std::string OIIO_UTIL_API utf16_to_utf8(const std::wstring& utf16str) noexcept; OIIO_UTIL_API char * safe_strcpy (char *dst, string_view src, size_t size) noexcept; +/// Is the character a whitespace character (space, linefeed, tab, carrage +/// return)? Note: this is safer than C isspace(), which has undefined +/// behavior for negative char values. Also note that it differs from C +/// isspace by not detecting form feed or vertical tab, because who cares. +inline bool isspace(char c) { + return c == ' ' || c == '\n' || c == '\t' || c == '\r'; +} + /// Modify str to trim any leading whitespace (space, tab, linefeed, cr) /// from the front. void OIIO_UTIL_API skip_whitespace (string_view &str) noexcept; diff --git a/src/libutil/strutil.cpp b/src/libutil/strutil.cpp index 1c436e31fa..e634f7e4d2 100644 --- a/src/libutil/strutil.cpp +++ b/src/libutil/strutil.cpp @@ -597,16 +597,16 @@ split_whitespace(string_view str, std::vector& result, // Implementation inspired by Pystring string_view::size_type i, j, len = str.size(); for (i = j = 0; i < len;) { - while (i < len && ::isspace(str[i])) + while (i < len && Strutil::isspace(str[i])) i++; j = i; - while (i < len && !::isspace(str[i])) + while (i < len && !Strutil::isspace(str[i])) i++; if (j < i) { if (--maxsplit <= 0) break; result.push_back(str.substr(j, i - j)); - while (i < len && ::isspace(str[i])) + while (i < len && Strutil::isspace(str[i])) i++; j = i; } @@ -797,7 +797,7 @@ Strutil::safe_strcpy(char* dst, string_view src, size_t size) noexcept void Strutil::skip_whitespace(string_view& str) noexcept { - while (str.size() && isspace(str.front())) + while (str.size() && Strutil::isspace(str.front())) str.remove_prefix(1); } @@ -806,7 +806,7 @@ Strutil::skip_whitespace(string_view& str) noexcept void Strutil::remove_trailing_whitespace(string_view& str) noexcept { - while (str.size() && isspace(str.back())) + while (str.size() && Strutil::isspace(str.back())) str.remove_suffix(1); } @@ -911,7 +911,7 @@ Strutil::parse_string(string_view& str, string_view& val, bool eat, const char *begin = p.begin(), *end = p.begin(); bool escaped = false; // was the prior character a backslash while (end != p.end()) { - if (isspace(*end) && !quoted) + if (Strutil::isspace(*end) && !quoted) break; // not quoted and we hit whitespace: we're done if (quoted && *end == lead_char && !escaped) break; // closing quote -- we're done (beware embedded quote) diff --git a/src/pnm.imageio/pnminput.cpp b/src/pnm.imageio/pnminput.cpp index be46da9bc0..3c02206edb 100644 --- a/src/pnm.imageio/pnminput.cpp +++ b/src/pnm.imageio/pnminput.cpp @@ -90,7 +90,7 @@ inline const char* nextToken(std::istream& file, std::string& current_line, const char*& pos) { while (1) { - while (isspace(*pos)) + while (Strutil::isspace(*pos)) pos++; if (*pos) break; @@ -360,7 +360,7 @@ PNMInput::read_file_header() m_max_val = 1; //Space before content - if (!(isspace(m_file.get()) && m_file.good())) + if (!(Strutil::isspace(m_file.get()) && m_file.good())) return false; m_header_end_pos = m_file.tellg(); // remember file pos @@ -393,7 +393,7 @@ PNMInput::read_file_header() } //Space before content - if (!(isspace(m_file.get()) && m_file.good())) + if (!(Strutil::isspace(m_file.get()) && m_file.good())) return false; m_header_end_pos = m_file.tellg(); // remember file pos