From b630f5e9c7cf1dc24797d253f37e526c00054144 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Fri, 10 Jul 2026 13:56:06 +0200 Subject: [PATCH] Fix container input_adapter SFINAE for lvalue-only ADL begin/end (#5260) * Fix container input_adapter SFINAE for lvalue-only ADL begin/end (#111) The container overload of json::parse(c) / accept(c) / sax_parse(c, ...) silently dropped from overload resolution for user types whose ADL begin(T&) / end(T&) accepted only non-const lvalue references (a legitimate pattern matching std::begin semantics). This was because the detection code used std::declval() which synthesized an rvalue, and the rvalue failed to bind to lvalue-only ADL functions. Fix by making both the outer input_adapter(ContainerType&&) and the factory's create(ContainerType&&) forwarding references, preserving the caller's value category and constness via reference collapsing. This ensures detection (std::declval) and actual use (std::forward) always match without needing decay/remove_reference. - Rewrite input_adapters.hpp container overload with forwarding refs - Add regression tests for lvalue-only non-const ADL begin/end - Add regression test for rvalue containers (no breakage) - Update API docs (parse, accept, sax_parse, from_*) to clarify that begin/end must match std::begin/std::end semantics - Add version history notes for 3.13.0 - Regenerate amalgamation Second-order effect: binary_reader.hpp's internal call to input_adapter(number_vector) now deduces iterator vs const_iterator based on the lvalue; functionally harmless (iterator_input_adapter is iterator-type-agnostic), verified via unit-ubjson/unit-bjdata tests. Closes remaining limitation from #4354 / PR #5218 (todo 106). Signed-off-by: Niels Lohmann * Avoid strlen() in test container to fix Codacy CWE-126 flag Suppressing the strlen()-based CWE-126 warning with NOLINT/nosec comments only silenced clang-tidy and the standalone Flawfinder Action; Codacy's own analysis (which also flags this pattern and doesn't honor those suppression comments) still reported it as a new issue, plus flagged the near-duplicate begin/end pair as cloned code. Store the buffer's size explicitly in MyContainerNonConstADL instead of computing it via strlen() in end(), which removes the flagged pattern outright and also de-duplicates the struct from the existing MyContainer's char*-based begin/end pair. Signed-off-by: Niels Lohmann * Avoid trailing return type to satisfy clang-tidy fuchsia-trailing-return The forwarding-reference input_adapter(ContainerType&&) entry point was written with an auto/trailing-decltype return type, but this project's ci_clang_tidy job enables the fuchsia-trailing-return check as an error, which rejects it. The return type only depends on the template parameter ContainerType, not on the runtime parameter, so it can be written as an ordinary leading return type instead - no functional change. Signed-off-by: Niels Lohmann * Avoid C-style array in test to satisfy clang-tidy avoid-c-arrays clang-tidy's cppcoreguidelines/hicpp/modernize-avoid-c-arrays checks flagged the char raw_data[] declaration used to reproduce the lvalue-only non-const ADL begin/end scenario. Use std::string instead and take a mutable pointer via &raw_data[0], which is the standard way to get a non-const char* into a string's buffer under C++11 (std::string::data() only returns non-const in C++17 and later). Signed-off-by: Niels Lohmann --------- Signed-off-by: Niels Lohmann --- docs/mkdocs/docs/api/basic_json/accept.md | 4 +- .../mkdocs/docs/api/basic_json/from_bjdata.md | 4 +- docs/mkdocs/docs/api/basic_json/from_bson.md | 4 +- docs/mkdocs/docs/api/basic_json/from_cbor.md | 4 +- .../docs/api/basic_json/from_msgpack.md | 4 +- .../mkdocs/docs/api/basic_json/from_ubjson.md | 4 +- docs/mkdocs/docs/api/basic_json/parse.md | 4 +- docs/mkdocs/docs/api/basic_json/sax_parse.md | 5 ++- .../nlohmann/detail/input/input_adapters.hpp | 8 ++-- single_include/nlohmann/json.hpp | 8 ++-- tests/src/unit-user_defined_input.cpp | 41 +++++++++++++++++++ 11 files changed, 73 insertions(+), 17 deletions(-) diff --git a/docs/mkdocs/docs/api/basic_json/accept.md b/docs/mkdocs/docs/api/basic_json/accept.md index 74c006665..df239713f 100644 --- a/docs/mkdocs/docs/api/basic_json/accept.md +++ b/docs/mkdocs/docs/api/basic_json/accept.md @@ -35,7 +35,8 @@ Unlike the [`parse()`](parse.md) function, this function neither throws an excep - a C-style array of characters - a pointer to a null-terminated string of single byte characters (throws if null) - a `std::string` - - an object `obj` for which `begin(obj)` and `end(obj)` produces a valid pair of iterators. + - a container `obj` for which `begin(obj)` and `end(obj)` produce a valid pair of iterators + (as found via ADL or member functions, with semantics compatible to `std::begin` and `std::end`) `IteratorType` : a compatible iterator type, for instance. @@ -110,6 +111,7 @@ A UTF-8 byte order mark is silently ignored. - Ignoring comments via `ignore_comments` added in version 3.9.0. - Changed [runtime assertion](../../features/assertions.md) in case of `FILE*` null pointers to exception in version 3.12.0. - Added `ignore_trailing_commas` in version 3.13.0. +- Extended container support (1) to include types with lvalue-only ADL `begin`/`end` (matching `std::begin`/`std::end` semantics) in version 3.13.0. !!! warning "Deprecation" diff --git a/docs/mkdocs/docs/api/basic_json/from_bjdata.md b/docs/mkdocs/docs/api/basic_json/from_bjdata.md index 274444aab..8558e36ec 100644 --- a/docs/mkdocs/docs/api/basic_json/from_bjdata.md +++ b/docs/mkdocs/docs/api/basic_json/from_bjdata.md @@ -29,7 +29,8 @@ The exact mapping and its limitations are described on a [dedicated page](../../ - a `FILE` pointer - a C-style array of characters - a pointer to a null-terminated string of single byte characters - - an object `obj` for which `begin(obj)` and `end(obj)` produces a valid pair of iterators. + - a container `obj` for which `begin(obj)` and `end(obj)` produce a valid pair of iterators + (as found via ADL or member functions, with semantics compatible to `std::begin` and `std::end`) `IteratorType` : a compatible iterator type @@ -101,3 +102,4 @@ Linear in the size of the input. ## Version history - Added in version 3.11.0. +- Extended container support (1) to include types with lvalue-only ADL `begin`/`end` (matching `std::begin`/`std::end` semantics) in version 3.13.0. diff --git a/docs/mkdocs/docs/api/basic_json/from_bson.md b/docs/mkdocs/docs/api/basic_json/from_bson.md index 41389c151..7f08ca0bd 100644 --- a/docs/mkdocs/docs/api/basic_json/from_bson.md +++ b/docs/mkdocs/docs/api/basic_json/from_bson.md @@ -29,7 +29,8 @@ The exact mapping and its limitations are described on a [dedicated page](../../ - a `FILE` pointer - a C-style array of characters - a pointer to a null-terminated string of single byte characters - - an object `obj` for which `begin(obj)` and `end(obj)` produces a valid pair of iterators. + - a container `obj` for which `begin(obj)` and `end(obj)` produce a valid pair of iterators + (as found via ADL or member functions, with semantics compatible to `std::begin` and `std::end`) `IteratorType` : a compatible iterator type @@ -101,6 +102,7 @@ Linear in the size of the input. ## Version history - Added in version 3.4.0. +- Extended container support (1) to include types with lvalue-only ADL `begin`/`end` (matching `std::begin`/`std::end` semantics) in version 3.13.0. !!! warning "Deprecation" diff --git a/docs/mkdocs/docs/api/basic_json/from_cbor.md b/docs/mkdocs/docs/api/basic_json/from_cbor.md index d29c7bf7b..b736b8581 100644 --- a/docs/mkdocs/docs/api/basic_json/from_cbor.md +++ b/docs/mkdocs/docs/api/basic_json/from_cbor.md @@ -32,7 +32,8 @@ The exact mapping and its limitations are described on a [dedicated page](../../ - a `FILE` pointer - a C-style array of characters - a pointer to a null-terminated string of single byte characters - - an object `obj` for which `begin(obj)` and `end(obj)` produces a valid pair of iterators. + - a container `obj` for which `begin(obj)` and `end(obj)` produce a valid pair of iterators + (as found via ADL or member functions, with semantics compatible to `std::begin` and `std::end`) `IteratorType` : a compatible iterator type @@ -111,6 +112,7 @@ Linear in the size of the input. - Changed to consume input adapters, removed `start_index` parameter, and added `strict` parameter in version 3.0.0. - Added `allow_exceptions` parameter in version 3.2.0. - Added `tag_handler` parameter in version 3.9.0. +- Extended container support (1) to include types with lvalue-only ADL `begin`/`end` (matching `std::begin`/`std::end` semantics) in version 3.13.0. !!! warning "Deprecation" diff --git a/docs/mkdocs/docs/api/basic_json/from_msgpack.md b/docs/mkdocs/docs/api/basic_json/from_msgpack.md index 12469bca0..69f45605f 100644 --- a/docs/mkdocs/docs/api/basic_json/from_msgpack.md +++ b/docs/mkdocs/docs/api/basic_json/from_msgpack.md @@ -29,7 +29,8 @@ The exact mapping and its limitations are described on a [dedicated page](../../ - a `FILE` pointer - a C-style array of characters - a pointer to a null-terminated string of single byte characters - - an object `obj` for which `begin(obj)` and `end(obj)` produces a valid pair of iterators. + - a container `obj` for which `begin(obj)` and `end(obj)` produce a valid pair of iterators + (as found via ADL or member functions, with semantics compatible to `std::begin` and `std::end`) `IteratorType` : a compatible iterator type @@ -103,6 +104,7 @@ Linear in the size of the input. - Parameter `start_index` since version 2.1.1. - Changed to consume input adapters, removed `start_index` parameter, and added `strict` parameter in version 3.0.0. - Added `allow_exceptions` parameter in version 3.2.0. +- Extended container support (1) to include types with lvalue-only ADL `begin`/`end` (matching `std::begin`/`std::end` semantics) in version 3.13.0. !!! warning "Deprecation" diff --git a/docs/mkdocs/docs/api/basic_json/from_ubjson.md b/docs/mkdocs/docs/api/basic_json/from_ubjson.md index a6621473d..82eb2d4de 100644 --- a/docs/mkdocs/docs/api/basic_json/from_ubjson.md +++ b/docs/mkdocs/docs/api/basic_json/from_ubjson.md @@ -29,7 +29,8 @@ The exact mapping and its limitations are described on a [dedicated page](../../ - a `FILE` pointer - a C-style array of characters - a pointer to a null-terminated string of single byte characters - - an object `obj` for which `begin(obj)` and `end(obj)` produces a valid pair of iterators. + - a container `obj` for which `begin(obj)` and `end(obj)` produce a valid pair of iterators + (as found via ADL or member functions, with semantics compatible to `std::begin` and `std::end`) `IteratorType` : a compatible iterator type @@ -102,6 +103,7 @@ Linear in the size of the input. - Added in version 3.1.0. - Added `allow_exceptions` parameter in version 3.2.0. +- Extended container support (1) to include types with lvalue-only ADL `begin`/`end` (matching `std::begin`/`std::end` semantics) in version 3.13.0. !!! warning "Deprecation" diff --git a/docs/mkdocs/docs/api/basic_json/parse.md b/docs/mkdocs/docs/api/basic_json/parse.md index 4291d6931..4d202e8a9 100644 --- a/docs/mkdocs/docs/api/basic_json/parse.md +++ b/docs/mkdocs/docs/api/basic_json/parse.md @@ -34,7 +34,8 @@ static basic_json parse(IteratorType first, IteratorType last, - a C-style array of characters - a pointer to a null-terminated string of single byte characters (throws if null) - a `std::string` - - an object `obj` for which `begin(obj)` and `end(obj)` produces a valid pair of iterators. + - a container `obj` for which `begin(obj)` and `end(obj)` produce a valid pair of iterators + (as found via ADL or member functions, with semantics compatible to `std::begin` and `std::end`) `IteratorType` : a compatible iterator type, for instance. @@ -236,6 +237,7 @@ Invalid Unicode escapes and unpaired surrogates in the input are reported as - Ignoring comments via `ignore_comments` added in version 3.9.0. - Changed [runtime assertion](../../features/assertions.md) in case of `FILE*` null pointers to exception in version 3.12.0. - Added `ignore_trailing_commas` in version 3.13.0. +- Extended container support (1) to include types with lvalue-only ADL `begin`/`end` (matching `std::begin`/`std::end` semantics) in version 3.13.0. !!! warning "Deprecation" diff --git a/docs/mkdocs/docs/api/basic_json/sax_parse.md b/docs/mkdocs/docs/api/basic_json/sax_parse.md index e93e850f2..81fa461a0 100644 --- a/docs/mkdocs/docs/api/basic_json/sax_parse.md +++ b/docs/mkdocs/docs/api/basic_json/sax_parse.md @@ -39,8 +39,8 @@ The SAX event lister must follow the interface of [`json_sax`](../json_sax/index - a `FILE` pointer - a C-style array of characters - a pointer to a null-terminated string of single byte characters - - an object `obj` for which `begin(obj)` and `end(obj)` produces a valid pair of - iterators. + - a container `obj` for which `begin(obj)` and `end(obj)` produce a valid pair of iterators + (as found via ADL or member functions, with semantics compatible to `std::begin` and `std::end`) `IteratorType` : a compatible iterator type for overload (2); a pair of character iterators whose `value_type` is an integral type @@ -127,6 +127,7 @@ A UTF-8 byte order mark is silently ignored. - Added in version 3.2.0. - Ignoring comments via `ignore_comments` added in version 3.9.0. - Added `ignore_trailing_commas` in version 3.13.0. +- Extended container support (1) to include types with lvalue-only ADL `begin`/`end` (matching `std::begin`/`std::end` semantics) in version 3.13.0. !!! warning "Deprecation" diff --git a/include/nlohmann/detail/input/input_adapters.hpp b/include/nlohmann/detail/input/input_adapters.hpp index 920a26adc..52d221b54 100644 --- a/include/nlohmann/detail/input/input_adapters.hpp +++ b/include/nlohmann/detail/input/input_adapters.hpp @@ -517,18 +517,18 @@ struct container_input_adapter_factory< ContainerType, { using adapter_type = decltype(input_adapter(begin(std::declval()), end(std::declval()))); - static adapter_type create(const ContainerType& container) + static adapter_type create(ContainerType&& container) { - return input_adapter(begin(container), end(container)); + return input_adapter(begin(std::forward(container)), end(std::forward(container))); } }; } // namespace container_input_adapter_factory_impl template -typename container_input_adapter_factory_impl::container_input_adapter_factory::adapter_type input_adapter(const ContainerType& container) +typename container_input_adapter_factory_impl::container_input_adapter_factory::adapter_type input_adapter(ContainerType&& container) { - return container_input_adapter_factory_impl::container_input_adapter_factory::create(container); + return container_input_adapter_factory_impl::container_input_adapter_factory::create(std::forward(container)); } // specialization for std::string diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 711529e9d..5395609f0 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -7354,18 +7354,18 @@ struct container_input_adapter_factory< ContainerType, { using adapter_type = decltype(input_adapter(begin(std::declval()), end(std::declval()))); - static adapter_type create(const ContainerType& container) + static adapter_type create(ContainerType&& container) { - return input_adapter(begin(container), end(container)); + return input_adapter(begin(std::forward(container)), end(std::forward(container))); } }; } // namespace container_input_adapter_factory_impl template -typename container_input_adapter_factory_impl::container_input_adapter_factory::adapter_type input_adapter(const ContainerType& container) +typename container_input_adapter_factory_impl::container_input_adapter_factory::adapter_type input_adapter(ContainerType&& container) { - return container_input_adapter_factory_impl::container_input_adapter_factory::create(container); + return container_input_adapter_factory_impl::container_input_adapter_factory::create(std::forward(container)); } // specialization for std::string diff --git a/tests/src/unit-user_defined_input.cpp b/tests/src/unit-user_defined_input.cpp index 56446ea94..1a9b6394b 100644 --- a/tests/src/unit-user_defined_input.cpp +++ b/tests/src/unit-user_defined_input.cpp @@ -54,6 +54,47 @@ TEST_CASE("Custom container non-member begin/end") } +struct MyContainerNonConstADL +{ + char* data; + std::size_t size; +}; + +char* begin(MyContainerNonConstADL& c) +{ + return c.data; +} + +char* end(MyContainerNonConstADL& c) +{ + return c.data + c.size; // NOLINT(cppcoreguidelines-pro-bounds-pointer-arithmetic) +} + +TEST_CASE("Custom container non-member non-const begin/end") +{ + // Container with lvalue-only non-const ADL begin/end (bug reproduction) + std::string raw_data = "[1,2,3,4]"; + MyContainerNonConstADL data{&raw_data[0], raw_data.size()}; // NOLINT(readability-container-data-pointer) + const json as_json = json::parse(data); + CHECK(as_json.at(0) == 1); + CHECK(as_json.at(1) == 2); + CHECK(as_json.at(2) == 3); + CHECK(as_json.at(3) == 4); + + // Same container with accept() + CHECK(json::accept(data)); +} + +TEST_CASE("Custom container non-member begin/end, rvalue") +{ + // Regression check: rvalue container parsing should still work + const json as_json = json::parse(MyContainer{"[1,2,3,4]"}); + CHECK(as_json.at(0) == 1); + CHECK(as_json.at(1) == 2); + CHECK(as_json.at(2) == 3); + CHECK(as_json.at(3) == 4); +} + TEST_CASE("Custom container member begin/end") { struct MyContainer2