Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly null-terminate output buffer in basic_istream::get[line] #5073

Merged
merged 8 commits into from
Nov 14, 2024
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]);
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved

{ // 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>();
}