From 5a97913e936c231fbd6235a923ac7316ce9b31c4 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Fri, 22 Jul 2022 15:34:35 +0200 Subject: [PATCH] :recycle: change behavior for null FILE* --- docs/mkdocs/docs/api/basic_json/accept.md | 11 ++++---- docs/mkdocs/docs/api/basic_json/parse.md | 8 ++++-- docs/mkdocs/docs/features/assertions.md | 27 +++++++++++++++++++ docs/mkdocs/docs/home/exceptions.md | 10 ------- .../nlohmann/detail/input/input_adapters.hpp | 13 ++++----- single_include/nlohmann/json.hpp | 13 ++++----- tests/src/unit-deserialization.cpp | 25 ----------------- 7 files changed, 53 insertions(+), 54 deletions(-) diff --git a/docs/mkdocs/docs/api/basic_json/accept.md b/docs/mkdocs/docs/api/basic_json/accept.md index 1e7f002091..097cc8c914 100644 --- a/docs/mkdocs/docs/api/basic_json/accept.md +++ b/docs/mkdocs/docs/api/basic_json/accept.md @@ -29,7 +29,7 @@ Unlike the [`parse`](parse.md) function, this function neither throws an excepti : A compatible input, for instance: - an `std::istream` object - - a `FILE` pointer + - a `FILE` pointer (must not be null) - a C-style array of characters - a pointer to a null-terminated string of single byte characters - a `std::string` @@ -64,10 +64,6 @@ Whether the input is valid JSON. Strong guarantee: if an exception is thrown, there are no changes in the JSON value. -## Exceptions - -Throws [`parse_error.116`](../../home/exceptions.md#jsonexceptionparse_error116) if passed `#!cpp FILE` pointer is `#!cpp nullptr`. - ## Complexity Linear in the length of the input. The parser is a predictive LL(1) parser. @@ -76,6 +72,11 @@ Linear in the length of the input. The parser is a predictive LL(1) parser. (1) A UTF-8 byte order mark is silently ignored. +!!! danger "Runtime assertion" + + The precondition that a passed `#!cpp FILE` pointer must not be null is enforced with a + [runtime assertion](../../features/assertions.md). + ## Examples ??? example diff --git a/docs/mkdocs/docs/api/basic_json/parse.md b/docs/mkdocs/docs/api/basic_json/parse.md index 9d7566b252..e0aaf956ba 100644 --- a/docs/mkdocs/docs/api/basic_json/parse.md +++ b/docs/mkdocs/docs/api/basic_json/parse.md @@ -28,7 +28,7 @@ static basic_json parse(IteratorType first, IteratorType last, : A compatible input, for instance: - an `std::istream` object - - a `FILE` pointer + - a `FILE` pointer (must not be null) - a C-style array of characters - a pointer to a null-terminated string of single byte characters - a `std::string` @@ -77,7 +77,6 @@ Strong guarantee: if an exception is thrown, there are no changes in the JSON va - Throws [`parse_error.102`](../../home/exceptions.md#jsonexceptionparse_error102) if to_unicode fails or surrogate error. - Throws [`parse_error.103`](../../home/exceptions.md#jsonexceptionparse_error103) if to_unicode fails. -- Throws [`parse_error.116`](../../home/exceptions.md#jsonexceptionparse_error116) if passed `#!cpp FILE` pointer is `#!cpp nullptr`. ## Complexity @@ -89,6 +88,11 @@ super-linear complexity. (1) A UTF-8 byte order mark is silently ignored. +!!! danger "Runtime assertion" + + The precondition that a passed `#!cpp FILE` pointer must not be null is enforced with a + [runtime assertion](../../features/assertions.md). + ## Examples ??? example "Parsing from a character array" diff --git a/docs/mkdocs/docs/features/assertions.md b/docs/mkdocs/docs/features/assertions.md index a0b1187224..6132062562 100644 --- a/docs/mkdocs/docs/features/assertions.md +++ b/docs/mkdocs/docs/features/assertions.md @@ -102,3 +102,30 @@ behavior and yields a runtime assertion. ``` Assertion failed: (m_object != nullptr), function operator++, file iter_impl.hpp, line 368. ``` + +### Reading from a null `FILE` pointer + +Reading from a null `#!cpp FILE` pointer is undefined behavior and yields a runtime assertion. This can happen when +calling `#!cpp std::fopen` on a nonexisting file. + +??? example "Example 4: Uninitialized iterator" + + The following code will trigger an assertion at runtime: + + ```cpp + #include + + using json = nlohmann::json; + + int main() + { + std::FILE* f = std::fopen("nonexisting_file.json", "r"); + json j = json::parse(f); + } + ``` + + Output: + + ``` + Assertion failed: (m_file != nullptr), function file_input_adapter, file input_adapters.hpp, line 55. + ``` diff --git a/docs/mkdocs/docs/home/exceptions.md b/docs/mkdocs/docs/home/exceptions.md index b8640a0450..c9885ae6fa 100644 --- a/docs/mkdocs/docs/home/exceptions.md +++ b/docs/mkdocs/docs/home/exceptions.md @@ -354,16 +354,6 @@ A UBJSON high-precision number could not be parsed. [json.exception.parse_error.115] parse error at byte 5: syntax error while parsing UBJSON high-precision number: invalid number text: 1A ``` -### json.exception.parse_error.116 - -A `#!cpp FILE` pointer passed to the [parse](../api/basic_json/parse.md) function is `#!cpp nullptr`; that is, a previous call to `#!cpp std::fopen` failed. - -!!! failure "Example message" - - ``` - [json.exception.parse_error.116] parse error: input file is invalid - ``` - ## Iterator errors This exception is thrown if iterators passed to a library function do not match diff --git a/include/nlohmann/detail/input/input_adapters.hpp b/include/nlohmann/detail/input/input_adapters.hpp index 805a965d08..bba61abea3 100644 --- a/include/nlohmann/detail/input/input_adapters.hpp +++ b/include/nlohmann/detail/input/input_adapters.hpp @@ -49,13 +49,10 @@ class file_input_adapter using char_type = char; JSON_HEDLEY_NON_NULL(2) - explicit file_input_adapter(std::FILE* f) + explicit file_input_adapter(std::FILE* f) noexcept : m_file(f) { - if (m_file == nullptr) - { - JSON_THROW(parse_error::create(116, 0, "input file is invalid", nullptr)); - } + JSON_ASSERT(m_file != nullptr); } // make class move-only @@ -67,7 +64,11 @@ class file_input_adapter std::char_traits::int_type get_character() noexcept { - return std::fgetc(m_file); + if (JSON_HEDLEY_LIKELY(m_file != nullptr)) + { + return std::fgetc(m_file); + } + return std::char_traits::eof(); // LCOV_EXCL_LINE } private: diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 044189df41..9c6dbb6b0e 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -5936,13 +5936,10 @@ class file_input_adapter using char_type = char; JSON_HEDLEY_NON_NULL(2) - explicit file_input_adapter(std::FILE* f) + explicit file_input_adapter(std::FILE* f) noexcept : m_file(f) { - if (m_file == nullptr) - { - JSON_THROW(parse_error::create(116, 0, "input file is invalid", nullptr)); - } + JSON_ASSERT(m_file != nullptr); } // make class move-only @@ -5954,7 +5951,11 @@ class file_input_adapter std::char_traits::int_type get_character() noexcept { - return std::fgetc(m_file); + if (JSON_HEDLEY_LIKELY(m_file != nullptr)) + { + return std::fgetc(m_file); + } + return std::char_traits::eof(); // LCOV_EXCL_LINE } private: diff --git a/tests/src/unit-deserialization.cpp b/tests/src/unit-deserialization.cpp index c61c6c6353..ef2a67f5c7 100644 --- a/tests/src/unit-deserialization.cpp +++ b/tests/src/unit-deserialization.cpp @@ -166,26 +166,6 @@ struct SaxEventLoggerExitAfterStartArray : public SaxEventLogger }; } // namespace -// Passing a NULL pointer to the input adapter violates its NON_NULL attribute which is detected by UBSAN. -// To still test whether exceptions are thrown, we need to exclude these tests from UBSAN which can only -// be done with a function attribute. See -// https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#disabling-instrumentation-with-attribute-no-sanitize-undefined -#if defined(__clang__) - __attribute__((no_sanitize("undefined"))) -#endif -void test_file_exception(); - -#if defined(__clang__) - __attribute__((no_sanitize("undefined"))) -#endif -void test_file_exception() -{ - std::FILE* f = std::fopen("nonexisting_file", "r"); // NOLINT(cppcoreguidelines-owning-memory) - json _; - CHECK_THROWS_WITH_AS(_ = json::parse(f), "[json.exception.parse_error.116] parse error: input file is invalid", json::parse_error&); - CHECK_THROWS_WITH_AS(_ = json::accept(f), "[json.exception.parse_error.116] parse error: input file is invalid", json::parse_error&); -} - TEST_CASE("deserialization") { SECTION("successful deserialization") @@ -352,11 +332,6 @@ TEST_CASE("deserialization") { CHECK_THROWS_WITH_AS("[\"foo\",1,2,3,false,{\"one\":1}"_json, "[json.exception.parse_error.101] parse error at line 1, column 29: syntax error while parsing array - unexpected end of input; expected ']'", json::parse_error&); } - - SECTION("FILE*") - { - test_file_exception(); - } } SECTION("contiguous containers")