Description
cbor_tag_handler_t::store is not a superset of cbor_tag_handler_t::ignore. Two separate gaps in the store branch of parse_cbor_internal():
1. A tagged item that is not a byte string is a hard parse error. After reading the tag argument for 0xD8–0xDB, the code unconditionally does get(); return get_cbor_binary(b) && sax->binary(b); (binary_reader.hpp:864-865). If the enclosed data item is anything other than a byte string, parsing fails with parse_error.113 — even though ignore handles the same document fine.
The most visible casualty is the CBOR self-describe tag 55799 (RFC 8949 §3.4.6), encoded 0xD9 0xD9 0xF7, which is widely used as a magic-number prefix and is exactly the case that motivated #1968:
magic 55799 + map, ignore : OK -> {"a":1}
magic 55799 + map, store : THROW: [json.exception.parse_error.113] parse error at byte 4:
syntax error while parsing CBOR binary: expected length specification (0x40-0x5B)
or indefinite binary array type (0x5F); last byte: 0xA1
2. Tag numbers 6–20 are silently discarded. For head bytes 0xC6–0xD4 the store switch hits default: and just recurses (binary_reader.hpp:861-862), so store behaves identically to ignore there and the tag number is lost with no indication:
auto j = json::from_cbor(std::vector<std::uint8_t>{0xCA, 0x41, 0x00}, // tag 10 over h'00'
true, true, json::cbor_tag_handler_t::store);
j.get_binary().has_subtype(); // false
cbor_tag_handler_t.md does scope store to "bytes 0xd8..0xdb", so #2 is documented. Combined with #5315 it means store covers only 4 of the 24 low tag head encodings.
Reproduction steps
Parse tag 0xD8 0x18 over each CBOR content type under both handlers.
Expected vs. actual results
Expected: store records the subtype where it can and otherwise degrades to ignore's behavior; it should never reject a document ignore accepts.
Actual (tag 0xD8 0x18 over various content):
| tagged item |
ignore |
store |
| byte string |
OK |
OK |
| unsigned int |
OK |
THROW |
| text string |
OK |
THROW |
| array |
OK |
THROW |
| map |
OK |
THROW |
| bool |
OK |
THROW |
Minimal code example
#include <nlohmann/json.hpp>
#include <iostream>
using json = nlohmann::json;
int main()
{
// 55799({"a": 1}) -- CBOR self-describe magic followed by a map
const std::vector<std::uint8_t> v{0xD9, 0xD9, 0xF7, 0xA1, 0x61, 0x61, 0x01};
std::cout << json::from_cbor(v, true, true, json::cbor_tag_handler_t::ignore) << "\n"; // {"a":1}
std::cout << json::from_cbor(v, true, true, json::cbor_tag_handler_t::store) << "\n"; // throws
}
Error messages
[json.exception.parse_error.113] parse error at byte 4: syntax error while parsing CBOR binary: expected length specification (0x40-0x5B) or indefinite binary array type (0x5F); last byte: 0xA1
Compiler and operating system
g++ 13.3.0, Ubuntu 24.04, -std=c++11
Library version
develop @ 8ec98e2
Would the fix be breaking?
The two halves differ a lot in risk, and they can be fixed independently.
Part 1 (fall back to ignore when the tagged item is not a byte string) — low risk. No API/ABI change. Input that currently throws parse_error.113 would start parsing, which only affects callers who explicitly opted into store. Depending on store throwing for a tagged map is hard to justify as intentional; the current behavior looks like an oversight rather than a contract. A caller who genuinely wants strictness still has cbor_tag_handler_t::error.
Part 2 (actually store subtypes for tags 6–20) — higher risk, and I would not bundle it. This changes the value produced for input that already parses today: a byte string tagged 10 currently yields a binary with has_subtype() == false, and would start yielding has_subtype() == true, subtype() == 10. That silently changes comparison results (operator== on binary values compares subtypes), dump() output, and any round-trip through to_cbor, which would then emit 0xD8 0x0A where it previously emitted nothing. It is also arguably the wrong shape for the general problem, since a tag over a non-binary item still has nowhere to go. Worth splitting into its own issue, or leaving as documented behavior.
So: part 1 is a bug fix and safe to backport; part 2 is a behavior change that deserves its own discussion and at minimum a release note.
Both are somewhat entangled with #5315 — if the missing head bytes 0xC0–0xC5/0xD5–0xD7 are added there, the store branch will need to decide what to do with them too, and "fall back to ignore" is the natural answer for both.
Description
cbor_tag_handler_t::storeis not a superset ofcbor_tag_handler_t::ignore. Two separate gaps in thestorebranch ofparse_cbor_internal():1. A tagged item that is not a byte string is a hard parse error. After reading the tag argument for
0xD8–0xDB, the code unconditionally doesget(); return get_cbor_binary(b) && sax->binary(b);(binary_reader.hpp:864-865). If the enclosed data item is anything other than a byte string, parsing fails withparse_error.113— even thoughignorehandles the same document fine.The most visible casualty is the CBOR self-describe tag 55799 (RFC 8949 §3.4.6), encoded
0xD9 0xD9 0xF7, which is widely used as a magic-number prefix and is exactly the case that motivated #1968:2. Tag numbers 6–20 are silently discarded. For head bytes
0xC6–0xD4thestoreswitch hitsdefault:and just recurses (binary_reader.hpp:861-862), sostorebehaves identically toignorethere and the tag number is lost with no indication:cbor_tag_handler_t.mddoes scopestoreto "bytes 0xd8..0xdb", so #2 is documented. Combined with #5315 it meansstorecovers only 4 of the 24 low tag head encodings.Reproduction steps
Parse tag
0xD8 0x18over each CBOR content type under both handlers.Expected vs. actual results
Expected:
storerecords the subtype where it can and otherwise degrades toignore's behavior; it should never reject a documentignoreaccepts.Actual (tag
0xD8 0x18over various content):ignorestoreMinimal code example
Error messages
Compiler and operating system
g++ 13.3.0, Ubuntu 24.04,
-std=c++11Library version
develop@ 8ec98e2Would the fix be breaking?
The two halves differ a lot in risk, and they can be fixed independently.
Part 1 (fall back to
ignorewhen the tagged item is not a byte string) — low risk. No API/ABI change. Input that currently throwsparse_error.113would start parsing, which only affects callers who explicitly opted intostore. Depending onstorethrowing for a tagged map is hard to justify as intentional; the current behavior looks like an oversight rather than a contract. A caller who genuinely wants strictness still hascbor_tag_handler_t::error.Part 2 (actually store subtypes for tags 6–20) — higher risk, and I would not bundle it. This changes the value produced for input that already parses today: a byte string tagged 10 currently yields a
binarywithhas_subtype() == false, and would start yieldinghas_subtype() == true, subtype() == 10. That silently changes comparison results (operator==on binary values compares subtypes),dump()output, and any round-trip throughto_cbor, which would then emit0xD8 0x0Awhere it previously emitted nothing. It is also arguably the wrong shape for the general problem, since a tag over a non-binary item still has nowhere to go. Worth splitting into its own issue, or leaving as documented behavior.So: part 1 is a bug fix and safe to backport; part 2 is a behavior change that deserves its own discussion and at minimum a release note.
Both are somewhat entangled with #5315 — if the missing head bytes
0xC0–0xC5/0xD5–0xD7are added there, thestorebranch will need to decide what to do with them too, and "fall back toignore" is the natural answer for both.