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
28 changes: 26 additions & 2 deletions stl/inc/istream
Original file line number Diff line number Diff line change
Expand Up @@ -365,8 +365,21 @@ public:

basic_istream& __CLR_OR_THIS_CALL get(_Elem* _Str, streamsize _Count, _Elem _Delim) {
// get up to _Count characters into NTCS, stop before _Delim
struct _NODISCARD _Get_guard {
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
_Elem** _StrRef;
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
~_Get_guard() {
if (_StrRef) {
**_StrRef = _Elem(); // add terminating null character
}
}
};

ios_base::iostate _State = ios_base::goodbit;
_Chcount = 0;

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

const sentry _Ok(*this, true);

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

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

Expand Down Expand Up @@ -448,8 +460,21 @@ public:

basic_istream& __CLR_OR_THIS_CALL getline(_Elem* _Str, streamsize _Count, _Elem _Delim) {
// get up to _Count characters into NTCS, discard _Delim
struct _NODISCARD _Getline_guard {
_Elem** _StrRef;
~_Getline_guard() {
if (_StrRef) {
**_StrRef = _Elem(); // add terminating null character
}
}
};

ios_base::iostate _State = ios_base::goodbit;
_Chcount = 0;

// ensure null termination of buffer with nonzero length
const _Getline_guard _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 +502,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
252 changes: 252 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 Down Expand Up @@ -148,6 +149,254 @@ void test_istream_exceptions() {
}
}

template <typename CharT>
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
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_nulltermination_under_exceptions() {
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
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
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
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, 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);
} catch (const ios_base::failure&) {
// Expected case
}
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
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 +730,9 @@ int main() {
test_istream_exceptions<char>();
test_istream_exceptions<wchar_t>();

test_gh5070_istream_get_nulltermination_under_exceptions<char>();
test_gh5070_istream_get_nulltermination_under_exceptions<wchar_t>();

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