From b661022ccd03baae65860964305147bafe62ad1f Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Wed, 20 Jul 2022 13:09:36 +0200 Subject: [PATCH 01/13] :boom: throw exception if nullptr is passed to parse function Addresses #3584 --- docs/mkdocs/docs/api/basic_json/parse.md | 2 +- docs/mkdocs/docs/home/exceptions.md | 10 ++++++++++ include/nlohmann/detail/input/input_adapters.hpp | 12 +++++++++--- single_include/nlohmann/json.hpp | 12 +++++++++--- tests/src/unit-deserialization.cpp | 6 ++++++ 5 files changed, 35 insertions(+), 7 deletions(-) diff --git a/docs/mkdocs/docs/api/basic_json/parse.md b/docs/mkdocs/docs/api/basic_json/parse.md index 92808cd6be..0bfae19166 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 (will throw [json.exception.parse_error.116](../../home/exceptions.md#jsonexceptionparse_error116) if passed pointer is `#!cpp nullptr`) - a C-style array of characters - a pointer to a null-terminated string of single byte characters - a `std::string` diff --git a/docs/mkdocs/docs/home/exceptions.md b/docs/mkdocs/docs/home/exceptions.md index c9885ae6fa..8efad510ed 100644 --- a/docs/mkdocs/docs/home/exceptions.md +++ b/docs/mkdocs/docs/home/exceptions.md @@ -354,6 +354,16 @@ 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: No such file or directory + ``` + ## 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 2bc49aa2e9..802746b064 100644 --- a/include/nlohmann/detail/input/input_adapters.hpp +++ b/include/nlohmann/detail/input/input_adapters.hpp @@ -9,8 +9,9 @@ #pragma once #include // array +#include // errno #include // size_t -#include // strlen +#include // strlen, strerror #include // begin, end, iterator_traits, random_access_iterator_tag, distance, next #include // shared_ptr, make_shared, addressof #include // accumulate @@ -48,9 +49,14 @@ class file_input_adapter using char_type = char; JSON_HEDLEY_NON_NULL(2) - explicit file_input_adapter(std::FILE* f) noexcept + explicit file_input_adapter(std::FILE* f) : m_file(f) - {} + { + if (m_file == nullptr) + { + JSON_THROW(parse_error::create(116, 0, detail::concat("input file is invalid: ", reinterpret_cast(std::strerror(errno))), nullptr)); + } + } // make class move-only file_input_adapter(const file_input_adapter&) = delete; diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index cc5562d062..25908710bb 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -5893,8 +5893,9 @@ std::size_t hash(const BasicJsonType& j) #include // array +#include // errno #include // size_t -#include // strlen +#include // strlen, strerror #include // begin, end, iterator_traits, random_access_iterator_tag, distance, next #include // shared_ptr, make_shared, addressof #include // accumulate @@ -5934,9 +5935,14 @@ class file_input_adapter using char_type = char; JSON_HEDLEY_NON_NULL(2) - explicit file_input_adapter(std::FILE* f) noexcept + explicit file_input_adapter(std::FILE* f) : m_file(f) - {} + { + if (m_file == nullptr) + { + JSON_THROW(parse_error::create(116, 0, detail::concat("input file is invalid: ", reinterpret_cast(std::strerror(errno))), nullptr)); + } + } // make class move-only file_input_adapter(const file_input_adapter&) = delete; diff --git a/tests/src/unit-deserialization.cpp b/tests/src/unit-deserialization.cpp index ef2a67f5c7..7018dc0f2d 100644 --- a/tests/src/unit-deserialization.cpp +++ b/tests/src/unit-deserialization.cpp @@ -332,6 +332,12 @@ 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*") + { + std::FILE* f = std::fopen("nonexisting_file", "r"); // NOTLINT(cppcoreguidelines-owning-memory) + CHECK_THROWS_WITH_AS(json::parse(f), "[json.exception.parse_error.116] parse error: input file is invalid: No such file or directory", json::parse_error&); + } } SECTION("contiguous containers") From cca872f18fce4e9e38296957b7402a4b55c2f6ef Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Wed, 20 Jul 2022 15:43:23 +0200 Subject: [PATCH 02/13] :rotating_light: fix warnings --- include/nlohmann/detail/input/input_adapters.hpp | 1 + single_include/nlohmann/json.hpp | 2 ++ tests/src/unit-deserialization.cpp | 3 ++- 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/include/nlohmann/detail/input/input_adapters.hpp b/include/nlohmann/detail/input/input_adapters.hpp index 802746b064..a3a31d1736 100644 --- a/include/nlohmann/detail/input/input_adapters.hpp +++ b/include/nlohmann/detail/input/input_adapters.hpp @@ -25,6 +25,7 @@ #endif // JSON_NO_IO #include +#include #include namespace nlohmann diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 25908710bb..1ffefec60e 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -5910,6 +5910,8 @@ std::size_t hash(const BasicJsonType& j) // #include +// #include + // #include diff --git a/tests/src/unit-deserialization.cpp b/tests/src/unit-deserialization.cpp index 7018dc0f2d..a6a8fd542f 100644 --- a/tests/src/unit-deserialization.cpp +++ b/tests/src/unit-deserialization.cpp @@ -336,7 +336,8 @@ TEST_CASE("deserialization") SECTION("FILE*") { std::FILE* f = std::fopen("nonexisting_file", "r"); // NOTLINT(cppcoreguidelines-owning-memory) - CHECK_THROWS_WITH_AS(json::parse(f), "[json.exception.parse_error.116] parse error: input file is invalid: No such file or directory", json::parse_error&); + json _; + CHECK_THROWS_WITH_AS(_ = json::parse(f), "[json.exception.parse_error.116] parse error: input file is invalid: No such file or directory", json::parse_error&); } } From 432d78195de421ebf5c638d3e427188473a213c8 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Wed, 20 Jul 2022 17:02:51 +0200 Subject: [PATCH 03/13] :ok_hand: apply review comment --- docs/mkdocs/docs/api/basic_json/accept.md | 4 ++++ docs/mkdocs/docs/api/basic_json/parse.md | 10 +++++++++- docs/mkdocs/docs/home/exceptions.md | 2 +- include/nlohmann/detail/input/input_adapters.hpp | 5 ++--- single_include/nlohmann/json.hpp | 5 ++--- tests/src/unit-deserialization.cpp | 5 +++-- 6 files changed, 21 insertions(+), 10 deletions(-) diff --git a/docs/mkdocs/docs/api/basic_json/accept.md b/docs/mkdocs/docs/api/basic_json/accept.md index bbbac053aa..1e7f002091 100644 --- a/docs/mkdocs/docs/api/basic_json/accept.md +++ b/docs/mkdocs/docs/api/basic_json/accept.md @@ -64,6 +64,10 @@ 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. diff --git a/docs/mkdocs/docs/api/basic_json/parse.md b/docs/mkdocs/docs/api/basic_json/parse.md index 0bfae19166..9d7566b252 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 (will throw [json.exception.parse_error.116](../../home/exceptions.md#jsonexceptionparse_error116) if passed pointer is `#!cpp nullptr`) + - a `FILE` pointer - a C-style array of characters - a pointer to a null-terminated string of single byte characters - a `std::string` @@ -71,6 +71,14 @@ Deserialized JSON value; in case of a parse error and `allow_exceptions` set to Strong guarantee: if an exception is thrown, there are no changes in the JSON value. +## Exceptions + +- Throws [`parse_error.101`](../../home/exceptions.md#jsonexceptionparse_error101) in case of an unexpected token. +- 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 Linear in the length of the input. The parser is a predictive LL(1) parser. The complexity can be higher if the parser diff --git a/docs/mkdocs/docs/home/exceptions.md b/docs/mkdocs/docs/home/exceptions.md index 8efad510ed..7e27d51dac 100644 --- a/docs/mkdocs/docs/home/exceptions.md +++ b/docs/mkdocs/docs/home/exceptions.md @@ -361,7 +361,7 @@ A `#!cpp FILE*` pointer passed to the [parse](../api/basic_json/parse.md) functi !!! failure "Example message" ``` - [json.exception.parse_error.116] parse error: input file is invalid: No such file or directory + [json.exception.parse_error.116] parse error: input file is invalid ``` ## Iterator errors diff --git a/include/nlohmann/detail/input/input_adapters.hpp b/include/nlohmann/detail/input/input_adapters.hpp index a3a31d1736..805a965d08 100644 --- a/include/nlohmann/detail/input/input_adapters.hpp +++ b/include/nlohmann/detail/input/input_adapters.hpp @@ -9,9 +9,8 @@ #pragma once #include // array -#include // errno #include // size_t -#include // strlen, strerror +#include // strlen #include // begin, end, iterator_traits, random_access_iterator_tag, distance, next #include // shared_ptr, make_shared, addressof #include // accumulate @@ -55,7 +54,7 @@ class file_input_adapter { if (m_file == nullptr) { - JSON_THROW(parse_error::create(116, 0, detail::concat("input file is invalid: ", reinterpret_cast(std::strerror(errno))), nullptr)); + JSON_THROW(parse_error::create(116, 0, "input file is invalid", nullptr)); } } diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 1ffefec60e..044189df41 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -5893,9 +5893,8 @@ std::size_t hash(const BasicJsonType& j) #include // array -#include // errno #include // size_t -#include // strlen, strerror +#include // strlen #include // begin, end, iterator_traits, random_access_iterator_tag, distance, next #include // shared_ptr, make_shared, addressof #include // accumulate @@ -5942,7 +5941,7 @@ class file_input_adapter { if (m_file == nullptr) { - JSON_THROW(parse_error::create(116, 0, detail::concat("input file is invalid: ", reinterpret_cast(std::strerror(errno))), nullptr)); + JSON_THROW(parse_error::create(116, 0, "input file is invalid", nullptr)); } } diff --git a/tests/src/unit-deserialization.cpp b/tests/src/unit-deserialization.cpp index a6a8fd542f..06ae503e17 100644 --- a/tests/src/unit-deserialization.cpp +++ b/tests/src/unit-deserialization.cpp @@ -335,9 +335,10 @@ TEST_CASE("deserialization") SECTION("FILE*") { - std::FILE* f = std::fopen("nonexisting_file", "r"); // NOTLINT(cppcoreguidelines-owning-memory) + 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: No such file or directory", json::parse_error&); + 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&); } } From 0f9a2f264e95d196138ea884dee91d1b0fc9bec5 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Wed, 20 Jul 2022 17:07:35 +0200 Subject: [PATCH 04/13] :ok_hand: apply review comment --- tests/src/unit-deserialization.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/src/unit-deserialization.cpp b/tests/src/unit-deserialization.cpp index 06ae503e17..ed502f173c 100644 --- a/tests/src/unit-deserialization.cpp +++ b/tests/src/unit-deserialization.cpp @@ -335,7 +335,7 @@ TEST_CASE("deserialization") SECTION("FILE*") { - std::FILE* f = std::fopen("nonexisting_file", "r"); // NOLINT(cppcoreguidelines-owning-memory) + std::FILE* f = nullptr; 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&); From 5fb2b7ec5578e6fbca319bcb78c6e342c1e93a37 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Wed, 20 Jul 2022 18:58:08 +0200 Subject: [PATCH 05/13] :rotating_light: fix UBSAN warning --- tests/src/unit-deserialization.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/src/unit-deserialization.cpp b/tests/src/unit-deserialization.cpp index ed502f173c..06ae503e17 100644 --- a/tests/src/unit-deserialization.cpp +++ b/tests/src/unit-deserialization.cpp @@ -335,7 +335,7 @@ TEST_CASE("deserialization") SECTION("FILE*") { - std::FILE* f = nullptr; + 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&); From 7d4f2f4ec0dbcb9dc4d11fdae4b1ed0dfa1f5569 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Thu, 21 Jul 2022 14:50:48 +0200 Subject: [PATCH 06/13] :rotating_light: suppress UBSAN warning --- tests/src/unit-deserialization.cpp | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/tests/src/unit-deserialization.cpp b/tests/src/unit-deserialization.cpp index 06ae503e17..f716b1dcdd 100644 --- a/tests/src/unit-deserialization.cpp +++ b/tests/src/unit-deserialization.cpp @@ -166,6 +166,21 @@ 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() +{ + 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") @@ -335,10 +350,7 @@ TEST_CASE("deserialization") SECTION("FILE*") { - 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_file_exception(); } } From 002765c0e06a8514d0baf3bbbf921eb509859393 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Thu, 21 Jul 2022 14:57:56 +0200 Subject: [PATCH 07/13] :rotating_light: suppress UBSAN warning --- tests/src/unit-deserialization.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/src/unit-deserialization.cpp b/tests/src/unit-deserialization.cpp index f716b1dcdd..9454092d95 100644 --- a/tests/src/unit-deserialization.cpp +++ b/tests/src/unit-deserialization.cpp @@ -171,7 +171,7 @@ struct SaxEventLoggerExitAfterStartArray : public SaxEventLogger // 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"))) + __attribute__((no_sanitize("undefined"))) #endif void test_file_exception() { From f5c49c8532e078faf3b049dde2066181d6337bbe Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Thu, 21 Jul 2022 15:09:01 +0200 Subject: [PATCH 08/13] :rotating_light: suppress UBSAN warning --- tests/src/unit-deserialization.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/src/unit-deserialization.cpp b/tests/src/unit-deserialization.cpp index 9454092d95..c61c6c6353 100644 --- a/tests/src/unit-deserialization.cpp +++ b/tests/src/unit-deserialization.cpp @@ -170,6 +170,11 @@ struct SaxEventLoggerExitAfterStartArray : public SaxEventLogger // 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 From 313e8eb0e7c013a61e353da824f1218f66e2d188 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Thu, 21 Jul 2022 15:15:03 +0200 Subject: [PATCH 09/13] :ok_hand: apply review comment --- docs/mkdocs/docs/home/exceptions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/mkdocs/docs/home/exceptions.md b/docs/mkdocs/docs/home/exceptions.md index 7e27d51dac..b8640a0450 100644 --- a/docs/mkdocs/docs/home/exceptions.md +++ b/docs/mkdocs/docs/home/exceptions.md @@ -356,7 +356,7 @@ A UBJSON high-precision number could not be parsed. ### 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. +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" From 5a97913e936c231fbd6235a923ac7316ce9b31c4 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Fri, 22 Jul 2022 15:34:35 +0200 Subject: [PATCH 10/13] :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") From 61409c3516b18868da16dbc3c862022dd2f7af9b Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Fri, 22 Jul 2022 17:50:07 +0200 Subject: [PATCH 11/13] Apply suggestions from code review Co-authored-by: Florian Albrechtskirchinger --- docs/mkdocs/docs/features/assertions.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/mkdocs/docs/features/assertions.md b/docs/mkdocs/docs/features/assertions.md index 6132062562..2bad62e81d 100644 --- a/docs/mkdocs/docs/features/assertions.md +++ b/docs/mkdocs/docs/features/assertions.md @@ -106,7 +106,7 @@ behavior and yields a runtime assertion. ### 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. +calling `#!cpp std::fopen` on a nonexistent file. ??? example "Example 4: Uninitialized iterator" @@ -119,7 +119,7 @@ calling `#!cpp std::fopen` on a nonexisting file. int main() { - std::FILE* f = std::fopen("nonexisting_file.json", "r"); + std::FILE* f = std::fopen("nonexistent_file.json", "r"); json j = json::parse(f); } ``` From d07c4541273383dabe534c6412b34f6d9a74387c Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Fri, 22 Jul 2022 18:56:31 +0200 Subject: [PATCH 12/13] :recycle: change behavior for null FILE* --- include/nlohmann/detail/input/input_adapters.hpp | 6 +----- single_include/nlohmann/json.hpp | 6 +----- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/include/nlohmann/detail/input/input_adapters.hpp b/include/nlohmann/detail/input/input_adapters.hpp index bba61abea3..c4b0979030 100644 --- a/include/nlohmann/detail/input/input_adapters.hpp +++ b/include/nlohmann/detail/input/input_adapters.hpp @@ -64,11 +64,7 @@ class file_input_adapter std::char_traits::int_type get_character() noexcept { - if (JSON_HEDLEY_LIKELY(m_file != nullptr)) - { - return std::fgetc(m_file); - } - return std::char_traits::eof(); // LCOV_EXCL_LINE + return std::fgetc(m_file); } private: diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 9c6dbb6b0e..271ff635e4 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -5951,11 +5951,7 @@ class file_input_adapter std::char_traits::int_type get_character() noexcept { - if (JSON_HEDLEY_LIKELY(m_file != nullptr)) - { - return std::fgetc(m_file); - } - return std::char_traits::eof(); // LCOV_EXCL_LINE + return std::fgetc(m_file); } private: From 043a0d72ee677ff0ec7d99c3db61d8f3c6a46ef1 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Fri, 22 Jul 2022 19:03:27 +0200 Subject: [PATCH 13/13] :fire: remove unused header --- include/nlohmann/detail/input/input_adapters.hpp | 1 - single_include/nlohmann/json.hpp | 2 -- 2 files changed, 3 deletions(-) diff --git a/include/nlohmann/detail/input/input_adapters.hpp b/include/nlohmann/detail/input/input_adapters.hpp index c4b0979030..a034a6df68 100644 --- a/include/nlohmann/detail/input/input_adapters.hpp +++ b/include/nlohmann/detail/input/input_adapters.hpp @@ -24,7 +24,6 @@ #endif // JSON_NO_IO #include -#include #include namespace nlohmann diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 271ff635e4..5cc85b33ef 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -5909,8 +5909,6 @@ std::size_t hash(const BasicJsonType& j) // #include -// #include - // #include