Parse throw std::ios_base::failure exception when failbit set to true

[dPavelDev]

In brief:

std::ifstream is;
is.exceptions(
      is.exceptions() 
    | std::ios_base::failbit 
    | std::ios_base::badbit
); // handle different exceptions as 'file not found', 'permission denied'

try { 
    is.open("my_valid_file.json");
    const auto &jsonFile = nlohmann::json::parse(is); // (*)
    // ...
catch (const std::ios_base::failure &e) {
    std::cerr << e.code().message() << std::endl;
}

The line (*) produce std::ios_base::failure exception when parse function reaches end of stream even if json document is valid. Without std::ios_base::failbit function works OK.

[nlohmann]

The problem is that inside the lexer, std::ifstream::read is called which may throw in the described setting, because it tries to fill a buffer which may be larger than the input.

Example code:

#include <iostream>
#include <fstream>
#include <array>

int main() {
    std::ifstream is;
    is.exceptions(
                  is.exceptions()
                  | std::ios_base::failbit
                  | std::ios_base::badbit
                  ); // handle different exceptions as 'file not found', 'permission denied'
    
    try {
        is.open("file.json");
        
        std::array<char, 10000> buffer;
        is.read(buffer.data(), static_cast<std::streamsize>(buffer.size()));
    }
    catch (const std::ios_base::failure &e) {
        std::cerr << "error: " << e.code().message() << std::endl;
    }
}

Any ideas how to implement this without a try/catch?

[dPavelDev]

I don't know, is a fast and right this method or not:

is.get(buffer.data(), static_cast<std::streamsize>(buffer.size()), '\0');

but it works with non-empty input streams.

[nlohmann]

The null character does not help here, because the same code is also used for binary formats like CBOR or MsgPack.

Changing this line to

is.get(buffer.data(), static_cast<std::streamsize>(buffer.size()), std::char_traits<char>::eof());

works, but still has the issue that get has the behavior that

If no characters were extracted, calls setstate(failbit).

This happens if we check that the input actually ended with EOF, so we call get once more and get no characters.

[dPavelDev]

Good news! If is.get() reaches end of stream, is.eof() marks as true.

Also,

is.get(buffer.data(), static_cast<std::streamsize>(5), std::char_traits<char>::eof());

read 5 - 1 = 4 chars and store '\0' at the last element.

[nlohmann]

Yes, this may help.

void fill_buffer()
{
    // fill
    is.get(buffer.data(), static_cast<std::streamsize>(buffer.size()), std::char_traits<char>::eof());
    // store number of bytes in the buffer
    fill_size = static_cast<size_t>(is.gcount());
    // set EOF
    eof = is.eof();
}

This fixes nearly all issues - just two test cases with CBOR/MessagePack are still failing. Is there a difference in get's behavior for files opened with std::ios::binary?

[gregmarr]

is.get's third parameter is of type char_type, but eof() returns an int value that is not equal to any valid value of char_type, so I'm not sure that's what you want.

returns a value e such that X::eq_int_type(e, X::to_int_type(c)) is false for all values c.

However, is.get() says

the next available input character c equals delim, as determined by Traits::eq(c, delim). This character is not extracted (unlike basic_istream::getline())

Note that it uses Traits::eq(c, delim), not Traits::eq_int_type(e, Traits::to_int_type(c)).
If this is used for binary files, this may result in stopping prematurely because it matches a byte in the file.

[dPavelDev]

I see is.read() implementation in GCC 7:

// ....
      _M_gcount = this->rdbuf()->sgetn(__s, __n);
	      if (_M_gcount != __n)
		__err |= (ios_base::eofbit | ios_base::failbit);
// ....

If other compilers have similar code, the best solution is:

void fill_buffer()
{
    // fill
    fill_size = is.rdbuf()->sgetn(buffer.data(), static_cast<std::streamsize>(buffer.size()));
    // set EOF
    eof = ( fill_size != static_cast<std::streamsize>(buffer.size()) );
}

[nlohmann]

@dPavelDev This works, thanks a lot!

@gregmarr I tried to be clever by explicitly passing a non-char value... :-S

[nlohmann]

5439307 does not yet work with MSVC:

1: -------------------------------------------------------------------------------
1: regression tests
1:   issue #714 - throw std::ios_base::failure exception when failbit set to true
1: -------------------------------------------------------------------------------
1: C:\projects\json\test\src\unit-regression.cpp(1237)
1: ...............................................................................
1: 
1: C:\projects\json\test\src\unit-regression.cpp(1260): FAILED:
1:   CHECK_NOTHROW( nlohmann::json::from_cbor(is) )
1: due to unexpected exception with message:
1:   [json.exception.parse_error.110] parse error at 59: unexpected end of input
1: 
1: ===============================================================================
1: test cases:      51 |      50 passed | 1 failed
1: assertions: 2307748 | 2307747 passed | 1 failed
1: 

[nlohmann]

#367 does not fix this issue. With MSVC, I experience the same error as #714 (comment):

-------------------------------------------------------------------------------
62: regression tests
62:   issue #714 - throw std::ios_base::failure exception when failbit set to true
62: -------------------------------------------------------------------------------
62: C:\projects\json\test\src\unit-regression.cpp(1257)
62: ...............................................................................
62: 
62: C:\projects\json\test\src\unit-regression.cpp(1280): FAILED:
62:   CHECK_NOTHROW( nlohmann::json::from_cbor(is) )
62: due to unexpected exception with message:
62:   [json.exception.parse_error.110] parse error at 59: unexpected end of input
62: 
62: ===============================================================================
62: test cases:   1 |   0 passed | 1 failed
62: assertions: 407 | 406 passed | 1 failed

Any ideas on this? Maybe @pjkundert?

[abolz]

This just seems to be due to a missing std::ios::binary in
https://github.com/nlohmann/json/blob/develop/test/src/unit-regression.cpp#L1296.

With this flag the tests pass for me (MSVC).

[nlohmann]

@abolz Thanks so much for noting! (I did not mean to close the issue with the commit, but the CI builds seem to succeed) 👍