Subject: [Cbor] Review of draft-ietf-cbor-packed-02

From: Russ Housley <[email-masked]>
Date: Mon, 15 Mar 2021 12:47:48 -0400

I agreed to review this document during the session at IETF 110.

Russ

= = = = = = = =

Document: draft-ietf-cbor-packed-02
Reviewer: Russ Housley
Review Date: 2021-03-15

Major Concern:

RFC 7049 says that CBOR was designed so that "encoded data should be
self-describing so that a generic decoder can be written."  I di not
expect Packed CBOR to deviate from this design goal.  However, the
feature that allows an application environment, such as a media type,
to define tables does exactly that.  Decode will fail if these table
values are not known to the decoder.

Minor Concern:

In Section 2.1, the text says: "In the abstract, ...".  I am unsure what
this is trying to tell the implementer.  I think it means: "regardless
of the data structure used by an implementer".  I think these words can
be removed.

Nits:

Section 2: Begins with: "Packed CBOR is defined in two parts:
Referencing packing tables (this section) and setting up packing
tables (Section 3)."  I suggest this text be place at the send of
the Introduction.

---

From: Carsten Bormann <[email-masked]>
Date: Mon, 15 Mar 2021 18:35:35 +0100

Hi Russ,

thank you for this review.

> values are not known to the decoder.

Well, decode won’t fail, but unpacking will leave holes.
That is a consequence of using external dictionaries that is hard to avoid.
Would you prefer to rule out external dictionaries?  
They sure were useful in places like RFC 3485 etc.

I think this was supposed to mean:

>> Each of the tables is indexed by an unsigned integer (starting
>> from 0), which may be computed from information in tags and their
>> content as well as from CBOR simple values.

Changed this to:

>> Packed CBOR is defined in two parts: Referencing packing tables
>> ({{sec-packed-cbor}} and setting up packing tables
>> ({{sec-table-setup}}).
(At the end of 1. proper; Terminology section interspersed…)
>> # Packed CBOR
>> 
>> This section describes the packing tables, their structure and how
>> they are referenced.

Both changes are now in https://github.com/cbor-wg/cbor-packed/pull/2

Thank you!

Grüße, Carsten

---

From: Russ Housley <[email-masked]>
Date: Mon, 15 Mar 2021 14:40:11 -0400

> They sure were useful in places like RFC 3485 etc.

 I felt this point of divergence from the original design goal needed explicit discussion.

I'm not sure what the right answer might be.  Maybe some preamble to say that a named table is needed for successfully recovering the entire content. In the current document, this context is assumed.

Russ

---

From: Carsten Bormann <[email-masked]>
Date: Mon, 15 Mar 2021 21:33:46 +0100

I added to PR#2:

>  * By the application environment, e.g., a media type.  These can
>    define tables that amount to a static dictionary that can be used in
>    a CBOR data item for this application environment.
> +  Note that, without this information, a data item that uses such a
> +  static dictionary can be decoded at the CBOR level, but not fully
> +  unpacked.
> +  The table setup mechanisms provided by this document are defined in
> +  such a way that an unpacker can at least recognize if this is the
> +  case.
>  
>  * By one or more tags enclosing the packed content.
[…]

I’m not sure that this fully addresses your comment, but I agree that it is important to make this point.

Grüße, Carsten

---

From: Russ Housley <[email-masked]>
Date: Mon, 15 Mar 2021 18:18:21 -0400

> On Mar 15, 2021, at 4:33 PM, Carsten Bormann <[email-masked]> wrote:
> 

Not completely.  It mkes it clear that the unpack will know that it has an unknown table index.  That is good.  Should there be an explicit identifier of the predefined starting point for the tables?

Russ

---

From: Carsten Bormann <[email-masked]>
Date: Tue, 16 Mar 2021 10:45:02 +0100

On 2021-03-15, at 23:18, Russ Housley <[email-masked]> wrote:
> 
> Should there be an explicit identifier of the predefined starting point for the tables?

I’m a bit dense today and don’t understand that question.
Can you -v?

Grüße, Carsten

---

From: Russ Housley <[email-masked]>
Date: Tue, 16 Mar 2021 10:11:26 -0400

> On Mar 16, 2021, at 5:45 AM, Carsten Bormann <[email-masked]> wrote:
> 

Could there be some preamble that says, initialize the tables that includes an identifier?  That way, the decoder knows at the front whether the tables are at hand to successfully decode the content. 

Russ