to_ubjson()/to_bjdata() with use_type=true and use_size=false aborts in debug builds and emits invalid output in release builds

[nlohmann]

Description

to_ubjson.md documents use_type as "must be combined with use_size = true", but the combination is never enforced — it is only asserted:

json/include/nlohmann/detail/output/binary_writer.hpp

Lines 810 to 816 in 8ec98e2

	oa->write_character(to_char_type('['));
	}
	bool prefix_required = true;
	if (use_type && !j.m_data.m_value.array->empty())
	{
	JSON_ASSERT(use_count);

There are three such JSON_ASSERT(use_count) in write_ubjson — in the array, binary, and object cases. The consequences split by build type:

• Debug builds: JSON_ASSERT is assert, so to_ubjson(j, false, true) terminates the process with Assertion 'use_count' failed. An abort() is the wrong response to a user-supplied argument combination — asserts should guard internal invariants, not public API parameters, and there is no way for a caller to recover.
• NDEBUG builds: the assert vanishes and the writer emits a $ type marker with no # count, which is invalid UBJSON. to_ubjson() returns successfully and the library's own parser then rejects the result.

to_bjdata() shares the code path and behaves identically.

Note that the two parameters are plain bools with defaults (to_ubjson(j, use_size = false, use_type = false)), so to_ubjson(j, false, true) is an easy call to write by accident — and its failure mode is either a crash or silent corruption depending on how the translation unit was compiled.

Reproduction steps

Call to_ubjson with use_size = false, use_type = true on a non-empty array or object, once with asserts enabled and once with -DNDEBUG.

Expected vs. actual results

Expected: a thrown exception naming the invalid argument combination.

Actual, with -DNDEBUG:

input	bytes written	read back by from_ubjson
[1,2,3]	5b 24 69 01 02 03 5d	parse_error.112
{"a":1,"b":2}	7b 24 69 69 01 61 01 69 01 62 02 7d	parse_error.112
[]	2 bytes	[] (fine)
{}	2 bytes	{} (fine)
1	2 bytes	1 (fine)

[json.exception.parse_error.112] parse error at byte 4: syntax error while parsing UBJSON size:
    expected '#' after type information; last byte: 0x01

Empty containers and scalars are unaffected because the guarded branch is only reached for non-empty containers — so the failure is input-dependent as well as build-dependent.

Without -DNDEBUG the same call aborts:

Assertion `use_count' failed.

Minimal code example

#include <nlohmann/json.hpp>
#include <iostream>
using json = nlohmann::json;

int main()
{
    const json j = json::array({1, 2, 3});
    const auto v = json::to_ubjson(j, /*use_size=*/false, /*use_type=*/true);

    std::cout << v.size() << " bytes\n";        // NDEBUG: 7; otherwise: abort
    std::cout << json::from_ubjson(v) << "\n";  // throws parse_error.112
}

Error messages

[json.exception.parse_error.112] parse error at byte 4: syntax error while parsing UBJSON size: expected '#' after type information; last byte: 0x01

Compiler and operating system

g++ 13.3.0, Ubuntu 24.04, -std=c++11, with and without -DNDEBUG

Library version

develop @ 8ec98e2

Would the fix be breaking?

No API/ABI change under any of the options below — no signature, default argument, type, or enumerator changes.

Option A — throw (preferred). Reject the combination up front with a type_error/out_of_range, or add a dedicated id. Behaviour change per build type:

• Debug builds: abort() becomes a catchable exception. Strictly an improvement; nothing can depend on an abort.
• NDEBUG builds: a call that currently returns unparseable bytes starts throwing. This is the only real behaviour change, and it is hard to see it breaking anyone, since the output cannot be read back by this library or any conforming UBJSON parser. Worth a release note precisely because it is build-dependent today — a user who tested only in release mode may not know they are hitting it.
• to_ubjson() documents a strong exception-safety guarantee; throwing before writing anything preserves it.

Option B — silently upgrade use_type = true to imply use_size = true. Produces valid output and never throws, but ignores an explicit argument, which I would argue is worse: the caller asked for no size and silently gets sizes.

Option C — leave it, and only document harder. Keeps a public API whose failure mode is abort() in half of all builds. Not recommended.

Whichever is chosen, the three JSON_ASSERT(use_count) calls should go: an assertion is the wrong mechanism for validating caller-supplied arguments regardless of what replaces it.

One detail worth deciding explicitly: whether the check fires eagerly on entry to to_ubjson (so to_ubjson(json::array(), false, true) also throws) or lazily where it does today (so empty containers and scalars keep working). Eager is more predictable and easier to document; lazy is a smaller behaviour delta. I'd suggest eager, with the release note calling it out.

[Patrick10199]

While I'm here, I'd like to take a look at this.

I think Option A is the right behavior. My proposed implementation is to reject use_type && !use_size eagerly before writing output, covering both to_ubjson() and to_bjdata(). I would remove the three internal JSON_ASSERT(use_count) checks, add regression coverage for both formats and output forms, update the API and exception documentation, add a release note, and regenerate the amalgamated header.

type_error.318 appears to be the next available ID. If that exception category/ID and eager validation are acceptable, I'll proceed.