<istream>: basic_istream::get[line](char_type* s, std::streamsize n, char_type delim) do not null-terminate the output buffer correctly

[muellerj2]

Describe the bug

basic_stringstream::get[line]() do not handle null-termination correctly:

• If an output buffer of size 0 is passed in, a null character is still written into the buffer, resulting in a buffer overrun.
• In many cases, the output buffer is not null-terminated when an exception is raised.

The standard says (N4993 [istream.unformatted]/9 and 21):

In any case, if n is greater than zero it then stores a null character into the next successive location of the array.

So the output buffer must always be null-terminated except if the buffer has size 0.

Test case

#include <sstream>
#include <print>
#include <cassert>

using namespace std;

class throwing_buffer : public basic_streambuf<char> {
    virtual int_type underflow() override {
        throw 0;
    }
};

void test_get() {
    // output buffer of size 0 must not be null-terminated
    istringstream stream1("lorem");
    char buf1[] = "meow";
    stream1.get(buf1 + 1, 0, '\n');
    println("{}", buf1);

    // output buffers nust be null-terminated
    // even when exceptions are raised

    // exception raised due to eofbit being set
    istringstream stream2("o");
    char buf2[] = "woof";
    stream2.exceptions(ios_base::eofbit);
    try {
        stream2.get(buf2, sizeof(buf2), '\n');
    } catch (ios_base::failure&) {
    }
    println("{}", buf2);

    // exception raised in sentry constructor
    istringstream stream3("p");
    stream3.exceptions(ios_base::failbit);
    char buf3[] = "neigh";
    stream3.get(buf3, sizeof(buf3), '\n');
    char buf4[] = "bark";
    try {
        stream3.get(buf3 + 1, 1, '\n');
    } catch (ios_base::failure&) {
    }
    println("{}", buf4);

    // exception raised in streambuf
    // when extracting a character
    throwing_buffer strbuf;
    istream stream4(&strbuf);
    stream4.exceptions(ios_base::badbit);
    char buf5[] = "ribbit";
    try {
        stream4.get(buf5 + 1, 1, '\n');
    } catch (int) {
    }
    println("{}", buf5);
}

void test_getline() {
    // output buffer of size 0 must not be null-terminated
    istringstream stream1("lorem");
    char buf1[] = "meow";
    stream1.getline(buf1 + 1, 0, '\n');
    println("{}", buf1);

    // output buffers nust be null-terminated
    // even when exceptions are raised

    // exception raised due to eofbit being set
    istringstream stream2("o");
    char buf2[] = "woof";
    stream2.exceptions(ios_base::eofbit);
    try {
        stream2.getline(buf2, sizeof(buf2), '\n');
    } catch (ios_base::failure&) {
    }
    println("{}", buf2);

    // exception raised in sentry constructor
    istringstream stream3("p");
    stream3.exceptions(ios_base::failbit);
    char buf3[] = "neigh";
    stream3.get(buf3, sizeof(buf3), '\n');
    char buf4[] = "bark";
    try {
        stream3.getline(buf3 + 1, 1, '\n');
    } catch (ios_base::failure&) {
    }
    println("{}", buf4);

    // exception raised in streambuf
    // when extracting a character
    throwing_buffer strbuf;
    istream stream4(&strbuf);
    stream4.exceptions(ios_base::badbit);
    char buf5[] = "ribbit";
    try {
        stream4.getline(buf5 + 1, 1, '\n');
    } catch (int) {
    }
    println("{}", buf5);
}

int main() {
    test_get();
    println("----");
    test_getline();
    return 0;
}

https://godbolt.org/z/zfKqnePGh

Expected behavior

The program should print

meow
o
b
r
----
meow
o
b
r

Actual behavior

The program actually prints

m
ooof
bark
ribbit
----
m
o
bark
ribbit

Additional remarks

I discovered these bugs while analyzing why the libcxx test std/input.output/iostream.format/input.streams/istream.unformatted/get_pointer_size.pass.cpp fails. (An assert fails because get() does not null-terminate when an exception is raised on EOF.)

libc++ and libstdc++ do not null-terminate as well when a streambuf member function or the sentry constructor throw. However, the standard mandates that the output buffer must be null-terminated "in any case". Moreover, there is additional language in [istream.unformatted]/1 that the output buffer must be null-terminated when the sentry constructor throws:

Otherwise, if the sentry constructor exits by throwing an exception or if the sentry object produces false, when converted to a value of type bool, the function returns without attempting to obtain any input. In either case the number of extracted characters is set to 0; unformatted input functions taking a character array of nonzero size as an argument shall also store a null character (using charT()) in the first location of the array.

[muellerj2]

I will draft a fix.

By the way, a strict reading of the quote from [istream.unformatted]/1 suggests that read() and readsome() should null-terminate as well when sentry fails. I think this doesn't make much sense, though, since these functions do not null-terminate their buffers in any other case.