Skip to content

Commit

Permalink
Properly null-terminate output buffer in basic_istream::get[line] (#…
Browse files Browse the repository at this point in the history
…5073)

Co-authored-by: Stephan T. Lavavej <[email protected]>
  • Loading branch information
muellerj2 and StephanTLavavej authored Nov 14, 2024
1 parent e87ae37 commit ca1af94
Show file tree
Hide file tree
Showing 3 changed files with 272 additions and 6 deletions.
20 changes: 18 additions & 2 deletions stl/inc/istream
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,16 @@ _STL_DISABLE_CLANG_WARNINGS
#undef new

_STD_BEGIN
template <class _Elem>
struct _NODISCARD _Null_terminator_guard {
_Elem** _Str_ref;
~_Null_terminator_guard() {
if (_Str_ref) {
**_Str_ref = _Elem(); // add terminating null character
}
}
};

#pragma vtordisp(push, 2) // compiler bug workaround

_EXPORT_STD extern "C++" template <class _Elem, class _Traits>
Expand Down Expand Up @@ -367,6 +377,10 @@ public:
// get up to _Count characters into NTCS, stop before _Delim
ios_base::iostate _State = ios_base::goodbit;
_Chcount = 0;

// ensure null termination of buffer with nonzero length
const _Null_terminator_guard<_Elem> _Guard{0 < _Count ? &_Str : nullptr};

const sentry _Ok(*this, true);

if (_Ok && 0 < _Count) { // state okay, extract characters
Expand All @@ -388,7 +402,6 @@ public:
}

_Myios::setstate(_Chcount == 0 ? _State | ios_base::failbit : _State);
*_Str = _Elem(); // add terminating null character
return *this;
}

Expand Down Expand Up @@ -450,6 +463,10 @@ public:
// get up to _Count characters into NTCS, discard _Delim
ios_base::iostate _State = ios_base::goodbit;
_Chcount = 0;

// ensure null termination of buffer with nonzero length
const _Null_terminator_guard<_Elem> _Guard{0 < _Count ? &_Str : nullptr};

const sentry _Ok(*this, true);

if (_Ok && 0 < _Count) { // state okay, use facet to extract
Expand Down Expand Up @@ -477,7 +494,6 @@ public:
_CATCH_IO_END
}

*_Str = _Elem(); // add terminating null character
_Myios::setstate(_Chcount == 0 ? _State | ios_base::failbit : _State);
return *this;
}
Expand Down
4 changes: 0 additions & 4 deletions tests/libcxx/expected_results.txt
Original file line number Diff line number Diff line change
Expand Up @@ -488,10 +488,6 @@ std/diagnostics/syserr/syserr.syserr/syserr.syserr.members/ctor_int_error_catego
std/diagnostics/syserr/syserr.syserr/syserr.syserr.members/ctor_int_error_category_string.pass.cpp FAIL
std/diagnostics/syserr/syserr.syserr/syserr.syserr.members/ctor_int_error_category.pass.cpp FAIL

# libc++ disagrees with libstdc++ and MSVC on whether setstate calls during I/O that throw set failbit; see open issue LWG-2349
std/input.output/iostream.format/input.streams/istream.unformatted/get_pointer_size_chart.pass.cpp FAIL
std/input.output/iostream.format/input.streams/istream.unformatted/get_pointer_size.pass.cpp FAIL

# Sensitive to implementation details. Assertion failed: test_alloc_base::count == expected_num_allocs
std/containers/container.requirements/container.requirements.general/allocator_move.pass.cpp FAIL

Expand Down
254 changes: 254 additions & 0 deletions tests/std/tests/GH_001858_iostream_exception/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#define _SILENCE_CXX17_STRSTREAM_DEPRECATION_WARNING

#include <array>
#include <cassert>
#include <fstream>
#include <ios>
Expand All @@ -11,6 +12,7 @@
#include <ostream>
#include <sstream>
#include <streambuf>
#include <string>
#include <strstream>
#include <type_traits>
#include <utility>
Expand Down Expand Up @@ -148,6 +150,255 @@ void test_istream_exceptions() {
}
}

template <class CharT>
CharT meow_array;
template <>
constexpr array<char, 5> meow_array<char> = {"meow"};
template <>
constexpr array<wchar_t, 5> meow_array<wchar_t> = {L"meow"};

// testing GH-5070: basic_istream::get[line](char_type* s, std::streamsize n, char_type delim)
// do not null-terminate the output buffer correctly
template <class CharT>
void test_gh5070_istream_get_null_termination_under_exceptions() {
throwing_buffer<CharT> buffer;
const basic_string<CharT> stream_content(1U, meow_array<CharT>[2]);

{ // get, exception during input extraction, no exception rethrow
basic_istream<CharT> is(buffer.to_buf());
auto buf = meow_array<CharT>;
assert(!is.bad());
is.get(buf.data(), static_cast<streamsize>(buf.size()));
assert(is.bad());
assert(buf[0] == CharT());
}

{ // get, exception during input extraction, exception rethrow enabled
basic_istream<CharT> is(buffer.to_buf());
auto buf = meow_array<CharT>;
is.exceptions(ios_base::badbit);
assert(!is.bad());
try {
is.get(buf.data(), static_cast<streamsize>(buf.size()));
assert(false);
} catch (const ios_base::failure&) {
assert(false);
} catch (const test_exception&) {
// Expected case
}
assert(is.bad());
assert(buf[0] == CharT());
}

{ // get, empty output buffer, no exception raised
basic_istream<CharT> is(buffer.to_buf());
auto buf = meow_array<CharT>;
assert(!is.bad());
is.get(buf.data(), 0);
assert(is.fail());
assert(buf[0] == meow_array<CharT>[0]);
}

{ // get, empty output buffer, exception raised on failbit
basic_istream<CharT> is(buffer.to_buf());
auto buf = meow_array<CharT>;
is.exceptions(ios_base::failbit);
assert(!is.bad());
try {
is.get(buf.data(), 0);
assert(false);
} catch (const ios_base::failure&) {
// Expected case
}
assert(is.fail());
assert(buf[0] == meow_array<CharT>[0]);
}

{ // get, sentry construction fails, no exception raised
basic_stringbuf<CharT> strbuf{stream_content};
basic_istream<CharT> is(&strbuf);
assert(!is.bad());

// tests null termination on eof and
// sets eofbit, preparing sentry failure
auto buf1 = meow_array<CharT>;
is.get(buf1.data(), static_cast<streamsize>(buf1.size()));
assert(is.eof());
assert(!is.fail());
assert(buf1[0] == meow_array<CharT>[2]);
assert(buf1[1] == CharT());

// actually tests sentry construction failure
auto buf2 = meow_array<CharT>;
is.get(buf2.data(), static_cast<streamsize>(buf2.size()));
assert(is.fail());
assert(buf2[0] == CharT());
}

{ // get, sentry construction fails, exception raised on failbit
basic_stringbuf<CharT> strbuf{stream_content};
basic_istream<CharT> is(&strbuf);
is.exceptions(ios_base::failbit);
assert(!is.bad());

// tests null termination on eof and
// sets eofbit, preparing sentry failure
auto buf1 = meow_array<CharT>;
is.get(buf1.data(), static_cast<streamsize>(buf1.size()));
assert(is.eof());
assert(!is.fail());
assert(buf1[0] == meow_array<CharT>[2]);
assert(buf1[1] == CharT());

// actually tests sentry construction failure
auto buf2 = meow_array<CharT>;
try {
is.get(buf2.data(), static_cast<streamsize>(buf2.size()));
assert(false);
} catch (const ios_base::failure&) {
// Expected case
}
assert(is.fail());
assert(buf2[0] == CharT());
}

{ // get, exception raised on eofbit
basic_stringbuf<CharT> strbuf{stream_content};
basic_istream<CharT> is(&strbuf);
is.exceptions(ios_base::eofbit);
assert(!is.bad());

auto buf = meow_array<CharT>;
try {
is.get(buf.data(), static_cast<streamsize>(buf.size()));
assert(false);
} catch (const ios_base::failure&) {
// Expected case
}
assert(is.eof());
assert(!is.fail());
assert(buf[0] == meow_array<CharT>[2]);
assert(buf[1] == CharT());
}

{ // getline, exception during input extraction, no exception rethrow
basic_istream<CharT> is(buffer.to_buf());
auto buf = meow_array<CharT>;
assert(!is.bad());
is.getline(buf.data(), static_cast<streamsize>(buf.size()));
assert(is.bad());
assert(buf[0] == CharT());
}

{ // getline, exception during input extraction, exception rethrow enabled
basic_istream<CharT> is(buffer.to_buf());
auto buf = meow_array<CharT>;
is.exceptions(ios_base::badbit);
assert(!is.bad());
try {
is.getline(buf.data(), static_cast<streamsize>(buf.size()));
assert(false);
} catch (const ios_base::failure&) {
assert(false);
} catch (const test_exception&) {
// Expected case
}
assert(is.bad());
assert(buf[0] == CharT());
}

{ // getline, empty output buffer, no exception raised
basic_istream<CharT> is(buffer.to_buf());
auto buf = meow_array<CharT>;
assert(!is.bad());
is.getline(buf.data(), 0);
assert(is.fail());
assert(buf[0] == meow_array<CharT>[0]);
}

{ // getline, empty output buffer, exception raised on failbit
basic_istream<CharT> is(buffer.to_buf());
auto buf = meow_array<CharT>;
is.exceptions(ios_base::failbit);
assert(!is.bad());
try {
is.getline(buf.data(), 0);
assert(false);
} catch (const ios_base::failure&) {
// Expected case
}
assert(is.fail());
assert(buf[0] == meow_array<CharT>[0]);
}

{ // getline, sentry construction fails, no exception raised
basic_stringbuf<CharT> strbuf{stream_content};
basic_istream<CharT> is(&strbuf);
assert(!is.bad());

// tests null termination on eof and
// sets eofbit, preparing sentry failure
auto buf1 = meow_array<CharT>;
is.getline(buf1.data(), static_cast<streamsize>(buf1.size()));
assert(is.eof());
assert(!is.fail());
assert(buf1[0] == meow_array<CharT>[2]);
assert(buf1[1] == CharT());

// actually tests sentry construction failure
auto buf2 = meow_array<CharT>;
is.getline(buf2.data(), static_cast<streamsize>(buf2.size()));
assert(is.fail());
assert(buf2[0] == CharT());
}

{ // getline, sentry construction fails, exception raised on failbit
basic_stringbuf<CharT> strbuf{stream_content};
basic_istream<CharT> is(&strbuf);
is.exceptions(ios_base::failbit);
assert(!is.bad());

// tests null termination on eof and
// sets eofbit, preparing sentry failure
auto buf1 = meow_array<CharT>;
is.getline(buf1.data(), static_cast<streamsize>(buf1.size()));
assert(is.eof());
assert(!is.fail());
assert(buf1[0] == meow_array<CharT>[2]);
assert(buf1[1] == CharT());

// actually tests sentry construction failure
auto buf2 = meow_array<CharT>;
try {
is.getline(buf2.data(), static_cast<streamsize>(buf2.size()));
assert(false);
} catch (const ios_base::failure&) {
// Expected case
}
assert(is.fail());
assert(buf2[0] == CharT());
}

{ // getline, exception raised on eofbit
basic_stringbuf<CharT> strbuf{stream_content};
basic_istream<CharT> is(&strbuf);
is.exceptions(ios_base::eofbit);
assert(!is.bad());

auto buf = meow_array<CharT>;
try {
is.getline(buf.data(), static_cast<streamsize>(buf.size()));
assert(false);
} catch (const ios_base::failure&) {
// Expected case
}
assert(is.eof());
assert(!is.fail());
assert(buf[0] == meow_array<CharT>[2]);
assert(buf[1] == CharT());
}
}

template <class CharT>
void test_ostream_exceptions() {
throwing_buffer<CharT> buffer;
Expand Down Expand Up @@ -481,6 +732,9 @@ int main() {
test_istream_exceptions<char>();
test_istream_exceptions<wchar_t>();

test_gh5070_istream_get_null_termination_under_exceptions<char>();
test_gh5070_istream_get_null_termination_under_exceptions<wchar_t>();

test_ostream_exceptions<char>();
test_ostream_exceptions<wchar_t>();
}

0 comments on commit ca1af94

Please sign in to comment.