Re: [Cbor] [Ext] Benjamin Kaduk's Discuss on draft-ietf-cbor-7049bis-14: (with DISCUSS and COMMENT)

Benjamin Kaduk <kaduk@mit.edu> Fri, 18 September 2020 15:13 UTC

Return-Path: <kaduk@mit.edu>
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 5344A3A0C31; Fri, 18 Sep 2020 08:13:42 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.919
X-Spam-Level:
X-Spam-Status: No, score=-1.919 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RCVD_IN_MSPIKE_H4=-0.01, RCVD_IN_MSPIKE_WL=-0.01, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=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 o7Qwkbk8U67S; Fri, 18 Sep 2020 08:13:40 -0700 (PDT)
Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id B41893A0C20; Fri, 18 Sep 2020 08:13:40 -0700 (PDT)
Received: from kduck.mit.edu ([24.16.140.251]) (authenticated bits=56) (User authenticated as kaduk@ATHENA.MIT.EDU) by outgoing.mit.edu (8.14.7/8.12.4) with ESMTP id 08IFDFug018999 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 18 Sep 2020 11:13:17 -0400
Date: Fri, 18 Sep 2020 08:13:15 -0700
From: Benjamin Kaduk <kaduk@mit.edu>
To: Paul Hoffman <paul.hoffman@icann.org>
Cc: The IESG <iesg@ietf.org>, "cbor@ietf.org" <cbor@ietf.org>
Message-ID: <20200918151315.GE89563@kduck.mit.edu>
References: <159951641517.13535.7424396818917958932@ietfa.amsl.com> <E2C5EB38-72FD-4055-BCC1-BE24271984F3@icann.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <E2C5EB38-72FD-4055-BCC1-BE24271984F3@icann.org>
User-Agent: Mutt/1.12.1 (2019-06-15)
Archived-At: <https://mailarchive.ietf.org/arch/msg/cbor/FTaqEYWRqD7Ec515LvTFSM8tUuQ>
Subject: Re: [Cbor] [Ext] Benjamin Kaduk's Discuss on draft-ietf-cbor-7049bis-14: (with DISCUSS and COMMENT)
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: Fri, 18 Sep 2020 15:13:42 -0000

Hi Paul,

On Fri, Sep 18, 2020 at 02:45:17PM +0000, Paul Hoffman wrote:
> Thank you for the extensive review! We'll respond to the DISCUSS parts of the
> ballot here; a later response will cover the COMMENTs.

Thanks; that seems a wise split.

> 
> On 2020-09-08, at 00:06, Benjamin Kaduk via Datatracker <noreply@ietf.org> wrote:
> > 
> > Benjamin Kaduk has entered the following ballot position for
> > draft-ietf-cbor-7049bis-14: Discuss
> > 
> > […]
> > ----------------------------------------------------------------------
> > DISCUSS:
> > ----------------------------------------------------------------------
> > 
> > Thanks for this document; it's generally well-written and the changes
> > since 7049 are helpful.  I do have a few points that may need discussion
> > before publication, though.
> > 
> > Let's discuss whether the framing of tag number 35 for "regular
> > expressions that are roughly in [PCRE] form or a version of the
> > JavaScript regular expression syntax" meets the interoperability
> > expectations for Internet Standard status (bearing in mind that we are
> > defining a data format and not a protocol).  I note that it is okay
> > to leave the codepoint allocated with the current meaning and the
> > previous document as its reference, but decline to discuss it in the
> > document going for STD (we are in the process of doing that with COSE
> > countersignatures at the moment).
> 
> * — tag 35
> 
> Please see <https://github.com/cbor-wg/CBORbis/pull/210> for the proposed
> resolution. This PR is currently open.

That seems pretty much exactly like what I was expecting/hoping for;
thanks!

> > The example in Section 5 of "the item is a map that has byte strings for
> > keys and contains at least one pair whose key is 0xab01" seems to be in
> > violation of the definition of a valid map, since applications are not
> > allowed to rely on invalid behavior.  (That is, the implied "more than
> > one pair whose key is 0xab01" would be invalid.)
> 
> We have now changed this to "...and contains a pair whose key is 0xab01..." in
> <https://github.com/cbor-wg/CBORbis/pull/212>.

Thanks.

> > I think that the new deterministic encoding rules for sorting map keys
> > should be clear about whether "no content" sorts before or after
> > "content present" -- that is, how 0x10 and 0x1020 are ordered when the
> > 0x10 byte is identical and we have to compare \<nothing> with 0x20.
> 
> We have now added an implementation note at the end of 4.2.1
> in <https://github.com/cbor-wg/CBORbis/pull/212>.

Ah, a good point; sorry for missing that the first time around!

> > The discussion in Appendix C suggests that C (programming language)
> > implementations all use two's-complement representation of signed
> > integers; this requirement is present in POSIX but not C itself (I
> > verified this for C99 and C11).
> 
> Indeed. This no longer useful variation allowed by C finally seems to be fixed
> in draft C++ 20, which unanimously passed its final technical approval ballot
> on September 4, 2020: http://eel.is/c++draft/basic.fundamental
> 
> We agree the best way to handle this is to make this requirement explicit.
> C++20 is now referenced in new text in the part of the terminology section that
> talks about C-style notation in <https://github.com/cbor-wg/CBORbis/pull/208>.

This is probably fine, though using C++ as the reference for "notation
familiar from [C]" was a little surprising on first look.  (I actually had
missed that the draft C++20 was filing off this rough edge; that's good to
know.)

> > Additionally, the encode_sint() function (also Appendix C) relies on C
> > implementation-defined behavior while right-shifting a signed integer.
> 
> Similarly, this needless wart in C finally seems to be fixed in draft C++ 20 as
> well: http://eel.is/c++draft/expr.shift Also now referenced in 1.2
> in <https://github.com/cbor-wg/CBORbis/pull/208>.
> 
> > The C decode_half() function in Appendix D assumes that 'int' is wider
> > than 16 bits (since assigning a value to an int16_t variable when the
> > value is not representable in int16_t incurs implementation-defined
> > behavior).  Given that this spec is specifically targetting constrained
> > devices, it's not clear that such an assumption is justified.  (It also
> > right shifts a signed integer, incurring the same implementation-defined
> > behavior mentioned above.  (The bitwise AND against 0x8000 is also
> > problematic for an int16_t.))
> 
> Good point.  This can easily be fixed by declaring all the ints in that fragment as unsigneds instead.
> Done in <https://github.com/cbor-wg/CBORbis/pull/208>.

>From memory, that should work.  I will probably take another look at the
whole code when the revised I-D lands (but am under the weather at the
moment and don't have time to look right now).

Thanks for the updates!

-Ben