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

[Paul Hoffman <[e-mail]>]

Thank you for the extensive review! We'll respond to the DISCUSS parts of the
ballot here; a later response will cover the COMMENTs.

--Paul Hoffman

On 2020-09-08, at 00:06, Benjamin Kaduk via Datatracker <[e-mail]> 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.

> 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>.

> 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>.

> 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>.

> 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>.