fill_line_buffer incorrectly tests m_stream for eof but not fail or bad bits

[TestMonkey9000]

If the json object is asked to parse from a stream, the stream must be valid otherwise the json object goes into an infinite loop. The current code only tests for the end of file condition which assumes the incoming stream does not have the bad or fail bits set. I think a more general approach is to test with good() rather than eof().

            // no stream is used or end of file is reached
            if (m_stream == nullptr or m_stream->eof())

Should be:

            // no stream is used or end of file is reached
            if (m_stream == nullptr or !m_stream->good())

This will throw an exception if the stream is not open or has another issue rather than loop.
Test code:

#include "json.hpp"
using json = nlohmann::json;

#include <fstream>

int main(int argc, char** argv)
{

   std::ifstream istr("non_existant_file");
   json j = json::parse(istr);
   return 0;
}

[nlohmann]

Which version did you try? This should be fixed in the last version.

[TestMonkey9000]

Clearly an old version - ended up being v2.0.5. It is fixed in the constructor on the version I just pulled - v2.1.1. Is there an issue if the stream throws fail/bad while reading the stream? For example, std::getline() can set the fail bit when it can't store an the entire buffer up to the delimiter ('\n') or it can't extract any characters from the stream - both highly unlikely scenarios but throwing it out there as a potential strange behavior which would cause an infinite loop.

[nlohmann]

You're right - we do not check the stream once we started parsing. I shall have a look whether I can create a test case for this.

[nlohmann]

I need a test case where this is an issue.

[nlohmann]

@TestMonkey9000 With commit 4e49829 I added a check to make sure the stream is OK before calling std::getline on it. Could you have a look at it?

[TestMonkey9000]

The solution looks fine - curious why use good() and not fail()? Asking because good() checks eofbit, failbit, and badbit and eofbit is checked as the other side of the overall conditional. It seemed like since it was checked above, you would either change that check to "not good()" or the inserted check to "not fail()". From a functionality standpoint, I don't see why it would matter because there isn't an operation on the stream operation between the two points...just being pedantic.

[nlohmann]

That's a good point. I overworked this.