Re: [Cbor] Review of draft-ietf-cbor-packed-02
Carsten Bormann <cabo@tzi.org> Mon, 15 March 2021 17:35 UTC
Return-Path: <cabo@tzi.org>
X-Original-To: cbor@ietfa.amsl.com
Delivered-To: cbor@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 8813F3A1816 for <cbor@ietfa.amsl.com>; Mon, 15 Mar 2021 10:35:41 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.92
X-Spam-Level:
X-Spam-Status: No, score=-1.92 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, SPF_HELO_NONE=0.001, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id XjJvK8FeAHBE for <cbor@ietfa.amsl.com>; Mon, 15 Mar 2021 10:35:37 -0700 (PDT)
Received: from gabriel-vm-2.zfn.uni-bremen.de (gabriel-vm-2.zfn.uni-bremen.de [134.102.50.17]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id B1C813A181D for <cbor@ietf.org>; Mon, 15 Mar 2021 10:35:37 -0700 (PDT)
Received: from [192.168.217.118] (p5089a828.dip0.t-ipconnect.de [80.137.168.40]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by gabriel-vm-2.zfn.uni-bremen.de (Postfix) with ESMTPSA id 4Dzk8l5jvBzyX7; Mon, 15 Mar 2021 18:35:35 +0100 (CET)
Content-Type: text/plain; charset="utf-8"
Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.120.23.2.4\))
From: Carsten Bormann <cabo@tzi.org>
In-Reply-To: <3835E637-85A1-44B3-812D-FD6A80EE8473@vigilsec.com>
Date: Mon, 15 Mar 2021 18:35:35 +0100
Cc: cbor@ietf.org
X-Mao-Original-Outgoing-Id: 637522535.322997-83906eab7491f7961196278767c6490b
Content-Transfer-Encoding: quoted-printable
Message-Id: <08DB2CF6-21D7-4EE6-85C7-1E73416D1A70@tzi.org>
References: <3835E637-85A1-44B3-812D-FD6A80EE8473@vigilsec.com>
To: Russ Housley <housley@vigilsec.com>
X-Mailer: Apple Mail (2.3608.120.23.2.4)
Archived-At: <https://mailarchive.ietf.org/arch/msg/cbor/8MQ5DbVf1NIm1oRrYCcAnD486jU>
Subject: Re: [Cbor] Review of draft-ietf-cbor-packed-02
X-BeenThere: cbor@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: "Concise Binary Object Representation \(CBOR\)" <cbor.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/cbor>, <mailto:cbor-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/cbor/>
List-Post: <mailto:cbor@ietf.org>
List-Help: <mailto:cbor-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/cbor>, <mailto:cbor-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 15 Mar 2021 17:35:42 -0000
Hi Russ, thank you for this review. > 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. 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. > 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. 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. > 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. 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
- [Cbor] Review of draft-ietf-cbor-packed-02 Russ Housley
- Re: [Cbor] Review of draft-ietf-cbor-packed-02 Carsten Bormann
- Re: [Cbor] Review of draft-ietf-cbor-packed-02 Russ Housley
- Re: [Cbor] Review of draft-ietf-cbor-packed-02 Carsten Bormann
- Re: [Cbor] Review of draft-ietf-cbor-packed-02 Russ Housley
- Re: [Cbor] Review of draft-ietf-cbor-packed-02 Carsten Bormann
- Re: [Cbor] Review of draft-ietf-cbor-packed-02 Russ Housley