Skip to content

Commit

Permalink
Avoid unsafe isspace() (AcademySoftwareFoundation#3310)
Browse files Browse the repository at this point in the history
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 AcademySoftwareFoundation#3300
  • Loading branch information
lgritz committed Jan 30, 2022
1 parent 2508fd8 commit d2e4c3c
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 10 deletions.
2 changes: 1 addition & 1 deletion src/hdr.imageio/rgbe.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}
Expand Down
8 changes: 8 additions & 0 deletions src/include/OpenImageIO/strutil.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
12 changes: 6 additions & 6 deletions src/libutil/strutil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -597,16 +597,16 @@ split_whitespace(string_view str, std::vector<string_view>& 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;
}
Expand Down Expand Up @@ -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);
}

Expand All @@ -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);
}

Expand Down Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions src/pnm.imageio/pnminput.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down

0 comments on commit d2e4c3c

Please sign in to comment.