You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Parse throw std::ios_base::failure exception when failbit set to true #714
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.
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.
voidfill_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?
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.
Activity
nlohmann commentedon Aug 27, 2017
The problem is that inside the lexer,
std::ifstream::readis called which may throw in the described setting, because it tries to fill a buffer which may be larger than the input.Example code:
Any ideas how to implement this without a try/catch?
dPavelDev commentedon Aug 27, 2017
I don't know, is a fast and right this method or not:
but it works with non-empty input streams.
nlohmann commentedon Aug 27, 2017
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
works, but still has the issue that
gethas the behavior thatThis happens if we check that the input actually ended with EOF, so we call
getonce more and get no characters.dPavelDev commentedon Aug 27, 2017
Good news! If
is.get()reaches end of stream,is.eof()marks astrue.Also,
read
5 - 1 = 4chars and store'\0'at the last element.nlohmann commentedon Aug 27, 2017
Yes, this may help.
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 withstd::ios::binary?gregmarr commentedon Aug 27, 2017
is.get's third parameter is of typechar_type, buteof()returns anintvalue that is not equal to any valid value ofchar_type, so I'm not sure that's what you want.However,
is.get()saysNote that it uses
Traits::eq(c, delim), notTraits::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 commentedon Aug 27, 2017
I see
is.read()implementation in GCC 7:If other compilers have similar code, the best solution is:
nlohmann commentedon Aug 28, 2017
@dPavelDev This works, thanks a lot!
@gregmarr I tried to be clever by explicitly passing a non-char value... :-S
🐛 fixes #714
nlohmann commentedon Aug 28, 2017
5439307 does not yet work with MSVC:
🚧 checking if #714 is now fixed with MSVC
nlohmann commentedon Oct 22, 2017
#367 does not fix this issue. With MSVC, I experience the same error as #714 (comment):
Any ideas on this? Maybe @pjkundert?
⏪ #714 is still not fixed
abolz commentedon Oct 31, 2017
This just seems to be due to a missing
std::ios::binaryinhttps://github.com/nlohmann/json/blob/develop/test/src/unit-regression.cpp#L1296.
With this flag the tests pass for me (MSVC).
nlohmann commentedon Oct 31, 2017
@abolz Thanks so much for noting! (I did not mean to close the issue with the commit, but the CI builds seem to succeed) 👍