Skip to content

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

Closed

Description

@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.

Activity

  1. nlohmann commented on Aug 27, 2017

    @nlohmann
    Owner

    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?

  2. dPavelDev commented on Aug 27, 2017

    @dPavelDev
    Author

    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.

  3. nlohmann commented on Aug 27, 2017

    @nlohmann
    Owner

    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.

  4. dPavelDev commented on Aug 27, 2017

    @dPavelDev
    Author

    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.

  5. self-assigned this
    on Aug 27, 2017
  6. added this to the Release 3.0.0 milestone on Aug 27, 2017
  7. nlohmann commented on Aug 27, 2017

    @nlohmann
    Owner

    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?

  8. gregmarr commented on Aug 27, 2017

    @gregmarr
    Contributor

    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.

  9. dPavelDev commented on Aug 27, 2017

    @dPavelDev
    Author

    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()) );
    }
  10. nlohmann commented on Aug 28, 2017

    @nlohmann
    Owner

    @dPavelDev This works, thanks a lot!

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

  11. added a commit that references this issue on Aug 28, 2017
    5439307
  12. nlohmann commented on Aug 28, 2017

    @nlohmann
    Owner

    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: 
    
  13. added and removed
    solution: proposed fixa fix for the issue has been proposed and waits for confirmation
    on Aug 28, 2017
  14. added a commit that references this issue on Oct 22, 2017
  15. nlohmann commented on Oct 22, 2017

    @nlohmann
    Owner

    #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?

  16. added a commit that references this issue on Oct 23, 2017
  17. abolz commented on Oct 31, 2017

    @abolz
    Contributor

    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).

  18. nlohmann commented on Oct 31, 2017

    @nlohmann
    Owner

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

  19. added
    solution: proposed fixa fix for the issue has been proposed and waits for confirmation
    and removed on Oct 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions