json::from_cbor does not respect allow_exceptions = false when input is string literal

[misos1]

• What is the issue you have?

Function json::from_cbor throws exception even with allow_exceptions = false when input is string literal.

• Please describe the steps to reproduce the issue. Can you provide a small but working code example?

#include <stdio.h>
#include "json.hpp"

using namespace nlohmann;

int main()
{
	json cbor = json::from_cbor("A", true, false);
	printf("is_discarded: %i\n", cbor.is_discarded());
	return 0;
}

Note json::from_cbor:

    static basic_json from_cbor(detail::input_adapter&& i,
                                const bool strict = true,
                                const bool allow_exceptions = true)

• What is the expected behavior?

It should print is_discarded: 1.

• And what is the actual behavior instead?

Throws exception:

terminate called after throwing an instance of 'nlohmann::detail::parse_error'
  what():  [json.exception.parse_error.112] parse error at byte 1: syntax error
while parsing CBOR value: invalid byte: 0x41
exited, aborted

• Which compiler and operating system are you using? Is it a supported compiler?

Clang 7.

[nlohmann]

This is strange:

First off, changing the literal to a std::string does the job:

    json cbor = json::from_cbor(std::string("A"), true, false);
    printf("is_discarded: %i\n", cbor.is_discarded());

output:

is_discarded: 1

This code correctly selects this function:

static basic_json from_cbor(detail::input_adapter&& i,
                            const bool strict = true,
                            const bool allow_exceptions = true)

The original code, however, uses this function:

template<typename A1, typename A2,
         detail::enable_if_t<std::is_constructible<detail::input_adapter, A1, A2>::value, int> = 0>
static basic_json from_cbor(A1 && a1, A2 && a2,
                            const bool strict = true,
                            const bool allow_exceptions = true)

Binding a1 to char const (&)[2] and a2 to true and strict to false. As no fourth parameter is given, allow_exceptions is true. Hence the exception.

That version of from_cbor is chosen, because we can construct an input_adapter with a char pointer and std::size_t using this constructor:

template<typename CharT,
         typename std::enable_if<
             std::is_pointer<CharT>::value and
             std::is_integral<typename std::remove_pointer<CharT>::type>::value and
             sizeof(typename std::remove_pointer<CharT>::type) == 1,
             int>::type = 0>
input_adapter(CharT b, std::size_t l)
    : ia(std::make_shared<input_buffer_adapter>(reinterpret_cast<const char*>(b), l)) {}

Unfortunately, I do not know how to avoid the conversion from bool to std::size_t in this setting.

[t-b]

@nlohmann

At least the example here works with just

$git diff .
diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp
index 2a32a829..d2d3ccf3 100644
--- a/single_include/nlohmann/json.hpp
+++ b/single_include/nlohmann/json.hpp
@@ -4107,13 +4107,14 @@ class input_adapter
         : ia(std::make_shared<wide_string_input_adapter<std::u32string>>(ws)) {}
 
     /// input adapter for buffer
-    template<typename CharT,
+    template<typename CharT, typename SizeT,
              typename std::enable_if<
                  std::is_pointer<CharT>::value and
                  std::is_integral<typename std::remove_pointer<CharT>::type>::value and
+                 not std::is_same<SizeT, bool>::value and
                  sizeof(typename std::remove_pointer<CharT>::type) == 1,
                  int>::type = 0>
-    input_adapter(CharT b, std::size_t l)
+    input_adapter(CharT b, SizeT l)
         : ia(std::make_shared<input_buffer_adapter>(reinterpret_cast<const char*>(b), l)) {}
 
     // derived support

but I'm probably missing a billion things.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[dota17]

Hi there @nlohmann , t-b's patch does work, we should consider applying it into the develop branch(of course, some test cases need to added too), but one thing I don't understand is why this code didn't work, instead, it need to modify this code to make it work?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[nlohmann]

I debugged the issue and found some remarkable coincidence I would like to share:

• The example used A as input for CBOR. Thats 0x41, a byte string with 1 byte.
• Binary formats were unsupported so far, so this would immediately raise a parse error.
• Now the binary formats are supported, and even with the fix from json::from_cbor does not respect allow_exceptions = false when input is string literal #1715 (comment) the behavior between a string literal and a std::string differ:
  • A string literal is passed as iterator range using std::begin/std::end. This uses the length including the terminating null byte. As a result, we can read two bytes from the input "A": 0x41 0x00. This is a proper CBOR byte string with a single byte.
  • A std::string only returns a single byte: 0x41, and the EOF is then unexpected which yields a parse error.

In order to fix this, we need to make sure the the terminating null byte introduced by string literals is already treated as EOF and not returned as 0x00. One quick fix for this is to add an overload

template<std::size_t N>
input_buffer_adapter input_adapter(const char (&array)[N])
{
    return input_adapter(std::begin(array), N-1);
}

that returns N-1 as length, but I do not like this as it would break the very edge case passing

const char input[] = { 'A', 0x00 };

which would then silently remove the null byte and making a valid example fail.

Any ideas on this?

[misos1]

I think better is to have "A" as 0x41 0x00 rather than to break cases like { 'A', 0x00 }. String literal in C++ implicitly appends \0 at end so we should expect that "A" is in fact { 'A', 0x00 } rather than { 'A' }.