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

muellerj2 · StephanTLavavej · web-flow · commit ca1af94c87a8 · 2024-11-14T12:31:57.000-08:00
Co-authored-by: Stephan T. Lavavej &lt;stl@nuwen.net&gt;
diff --git a/stl/inc/istream b/stl/inc/istream
@@ -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>
@@ -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
@@ -388,7 +402,6 @@ public:
         }
 
         _Myios::setstate(_Chcount == 0 ? _State | ios_base::failbit : _State);
-        *_Str = _Elem(); // add terminating null character
         return *this;
     }
 
@@ -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
@@ -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;
     }
diff --git a/tests/libcxx/expected_results.txt b/tests/libcxx/expected_results.txt
@@ -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
 
diff --git a/tests/std/tests/GH_001858_iostream_exception/test.cpp b/tests/std/tests/GH_001858_iostream_exception/test.cpp
@@ -3,6 +3,7 @@
 
 #define _SILENCE_CXX17_STRSTREAM_DEPRECATION_WARNING
 
+#include <array>
 #include <cassert>
 #include <fstream>
 #include <ios>
@@ -11,6 +12,7 @@
 #include <ostream>
 #include <sstream>
 #include <streambuf>
+#include <string>
 #include <strstream>
 #include <type_traits>
 #include <utility>
@@ -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;
@@ -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>();
 }
