Subject: [Cbor] Adam Roach's Discuss on draft-ietf-cbor-cddl-06: (with DISCUSS and COMMENT)

From: Adam Roach <[email-masked]>
Date: Mon, 19 Nov 2018 21:22:26 -0800

Adam Roach has entered the following ballot position for
draft-ietf-cbor-cddl-06: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-cbor-cddl/

----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

Thanks for all the work that went into creating this document. I have one point
that I think needs discussion, although it's entirely possible that I'm thinking
about this the wrong way.

§3.8:

Given that the list of control operators can be expanded in the future, it's
not clear what automated tools are supposed to do if they encounter controls
that they do not understand. I initially thought that it might be possible to
just ignore control operators (and their parameters) if they aren't understood,
as this would simply result in a more permissive validation of data against a
schema; but the ".and" control gives an example of a control operator where this
kind of elision would fail.

With the lack of any version indicators in CDDL, this seems like a straight-up
interoperability issue.

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

I also have a handful of non-critical comments of varying importance.

Please expand "CBOR":
 (1) In the title
 (2) Upon first use in the document body

See https://www.rfc-editor.org/materials/abbrev.expansion.txt for details.

---------------------------------------------------------------------------

§1.2:

>  New terms are introduced in _cursive_.  CDDL text in the running text
>  is in "typewriter".

This is perplexing, as I know of no tool that will render the canonical form
of current RFCs in the way being described. Is the intention to hold this
document until the new RFC format is available?

---------------------------------------------------------------------------

§2:

>  The rest of this section introduces a number of basic concepts of
>  CDDL, and section Section 3 defines additional syntax.  Appendix C

Nit: "...and Section 3..."

---------------------------------------------------------------------------

§2.2.2:

>  delimited by a "//" (double slash).  Note that the "//" operators
>  binds much more weakly than the other CDDL operators, so each line

Nit: "...operator binds..." or "...operators bind..."

---------------------------------------------------------------------------

§3.1:

>  o  A name can consist of any of the characters from the set {'A',
>     ..., 'Z', 'a', ..., 'z', '0', ..., '9', '_', '-', '@', '.', '$'},

This looks like a formal syntax of some kind, but I don't know where it's
defined. Notably, since this document has just defined ".." to be an inclusive
range operator and "..." to be an exclusive range operator, defining the set of
allowed characters in this way seems to run the risk of interpreting, e.g., "Z"
to be disallowed.

I suggest either defining the set of allowed characters using a formally defined
and cited grammar (e.g., ABNF), or using prose.

---------------------------------------------------------------------------

§3.1:

>  o  outside strings, whitespace (spaces, newlines, and comments) is
>     used to separate syntactic elements for readability (and to
>     separate identifiers or numbers that follow each other); it is
>     otherwise completely optional.

This seems nominally at odds with the following text in §2.2.2.1, which points
to at least one other case where whitespace is mandatory:

>  When using a name as
>  the left hand side of a range operator, use spacing as in
>
>     min .. max
>
>  to separate off the range operator.

---------------------------------------------------------------------------

§3.1:

>     If prefixed as "h" or "b64", the string is
>     interpreted as a sequence of pairs of hex digits (base16) or a
>     base64(url) string, respectively

Please normatively cite RFC 4648, sections 8 and 5 respectively.

---------------------------------------------------------------------------

§3.8.1:

>  When applied to an unsigned integer, the ".size" control restricts
>  the range of that integer by giving a maximum number of bytes that
>  should be needed in a computer representation of that unsigned
>  integer.  In other words, "uint .size N" is equivalent to
>  "0...BYTES_N", where BYTES_N == 256**N.
>
>     audio_sample = uint .size 3 ; 24-bit, equivalent to 0..16777215
>
>               Figure 9: Control for integer size in bytes

While they're semantically the same, the example is oddly mismatched with the
preceding text. Consider instead:

      audio_sample = uint .size 3 ; 24-bit, equivalent to 0...16777216

---------------------------------------------------------------------------

Appendix B:

>           / "#" "6" ["." uint] "(" S type S ")" ; note no space!

No space where? I see two space productions in that rule (so it clearly
applies to some specific location), and there are several places where spaces
cannot appear.

>     type1 = type2 [S (rangeop / ctlop) S type2]

This rule doesn't seem to properly capture the ambiguity of "a...b". There is a
terribly complex way to address this by defining parallel "type2" and "type3"
rules that differ only in whether a dot is allowed to appear in their value, and
defining type1 as requiring a space after the type that can contain dots -- but
that is probably overkill. It's probably sufficient to reiterate the warning
about requiring a space under such circumstances as a comment on this rule.

>     HEXDIG = DIGIT / "A" / "B" / "C" / "D" / "E" / "F"

It is a common implementor mistake to forget that ABNF is, by default,
case-insensitive. It is probably worth adding a comment here as a reminder.
(The same applies to "0x", "0b", "e", and "p" above, but these seem less likely
to appear in arbitrary case.)

---------------------------------------------------------------------------

Appendix B:

>     SCHAR = %x20-21 / %x23-5B / %x5D-10FFFD / SESC
>     SESC = "\" %x20-10FFFD
...
>     PCHAR = %x20-10FFFD

These almost certainly should be:

      SCHAR = %x20-21 / %x23-5B / %x5D-7E / %x80-10FFFD / SESC
      SESC = "\" %x20-7E / %x80-10FFFD
...
      PCHAR = %x20-7E / %x80-10FFFD

(i.e., exclude the control character %x7F)

---------------------------------------------------------------------------

Appendix C:

>  (It is not an error to extend a rule name
>  that has not yet been defined; this makes the right hand side the
>  first entry in the choice being created.)

Is it an error to redefine a rule name that has already been defined?

---

From: Carsten Bormann <[email-masked]>
Date: Tue, 20 Nov 2018 12:58:13 +0100

Hi Adam,

thank you for this detailed review.

My comments inline below.

Grüße, Carsten

> On Nov 20, 2018, at 06:22, Adam Roach <[email-masked]> wrote:
> 

There is no expectation that a tool that does not implement a control operator is particularly useful with a specification that does use it.
Since CDDL is not intended as an on-the-wire format, this does not per se create an interoperability issue.
(The situation is similar to the situation we have with ABNF and RFC 7405: If a specification uses %s, this does not create interoperability problems, but makes it harder to employ tools that do not implement %s.  In both cases, version numbers do not help.)

Introduction of a new control operator does limit the set of tools that can be used with specifications that use it, so it should not be done lightly.
However, the WG considered it important to open up CDDLs to SDOs that may have specific requirements in this space (e.g., imagine a format for carrying MODBUS binary data around — it is unlikely that the IETF would be the right body to create any CDDL extension needed for that).
Explicitly exposing control operators as the preferred extension point makes it more likely that tools can then be adapted to adopt such extensions.
The registry is intended to help avoid parallel development of control operators with the same name.

CBOR may be at the right stage to adopt it as a “well-known” abbreviation.

The title becomes very unwieldy with an expansion:

  Concise data definition language (CDDL): a notational convention to
  express CBOR (Concise Binary Object Representation) and JSON data structures

I expect the RFC editor will have an opinion on this; I’d like to defer to that.

The intention is to live with the limitations of the current format, but to also make versions available that make use of fonts.
Using underscores to simulate cursive text is an established convention; unfortunately conventions for typewriter text are less well-established (and XML2RFC uses simple quotes).

> Nit: “...and Section 3..."

Thanks.  Now c77a2b6 in the github version.

> Nit: “...operator binds..." or "...operators bind..."

Now 1c64571.

I went for prose:
    {‘A' to 'Z', 'a' to 'z', '0' to '9', '\_', '-', '@', '.',
[…]

Now a6c183a.

Case added:
    to separate syntactic elements for readability (and to separate
    identifiers, range operators, or numbers that follow each other); it is otherwise
    completely optional.

Now a3ecb14.

Oops.  RFC 4648 indeed is a required normative reference.  Also added the reference in section 3.1.

Note that both the CDDL and the extended diagnostic notation are agnostic between RFC 4648’s base64 variants, so both section 4 and 5 of that do apply.

Now 1fa4fd1.

>      audio_sample = uint .size 3 ; 24-bit, equivalent to 0…16777216

Indeed.  Now 0dd510d

We tried to be brief while alerting the reader.
Removed.

> about requiring a space under such circumstances as a comment on this rule.

Added.

> to appear in arbitrary case.)

The clarification went into the beginning of Appendix B, as there is no single place inside where it would fit.

All these editorial changes to Appendix B are now 5aacd34.

Good point.  Now bf39464.

It seems we never say so.
The CDDL tool discussed in Appendix F makes it an error only if the new definition differs from the existing one.

This is now written down in 60c22df.

.oOo.

---

From: Adam Roach <[email-masked]>
Date: Tue, 20 Nov 2018 08:08:31 -0600

> (The situation is similar to the situation we have with ABNF and RFC 7405: If a specification uses %s, this does not create interoperability problems, but makes it harder to employ tools that do no…

Thanks for the analogy. This makes sense, and so I'm clearing my DISCUSS.

/a

---

From: Michael Richardson <[email-masked]>
Date: Tue, 20 Nov 2018 11:08:20 -0500

Adam> With the lack of any version indicators in CDDL, this seems like a
    Adam> straight-up interoperability issue.

Carsten Bormann <[email-masked]> wrote:
    > There is no expectation that a tool that does not implement a control
    > operator is particularly useful with a specification that does use it.

okay, but wouldn't it still be good if there was a declaration at the top of
the file as to which revision of the specification the CDDL is assuming,
just so that the tool can fail gracefully?

--
Michael Richardson <[email-masked]>, Sandelman Software Works
 -= IPv6 IoT consulting =-

---

From: Carsten Bormann <[email-masked]>
Date: Tue, 20 Nov 2018 17:17:14 +0100

On Nov 20, 2018, at 17:08, Michael Richardson <[email-masked]> wrote:
> Signed PGP part

Once there is such a revision, that might make sense.

Adding a control operator is not a revision — that is the whole point about exposing an extension point.

(A tool can easily compare its inventory of control operators with the set used in the specification; no need for a separate declaration here.)

Grüße, Carsten

---

From: Michael Richardson <[email-masked]>
Date: Tue, 20 Nov 2018 13:56:20 -0500

> Once there is such a revision, that might make sense.

Not really. It needs to in this version.
Something like:
          "CDDL RFC9123"

at the top....

    > about exposing an extension point.

Sure.

    > (A tool can easily compare its inventory of control operators with the
    > set used in the specification; no need for a separate declaration
    > here.)

So, the error is:
    "RFC9756 control operator found in specification that claims RFC9123"

---

From: Jeffrey Yasskin <[email-masked]>
Date: Tue, 20 Nov 2018 11:59:05 -0800

>     "RFC9756 control operator found in specification that claims RFC9123"

I think Carsten's point is that in order to declare the set of control
operators in use, it's not sufficient to declare just the CDDL RFC. A CDDL
document might be using CDDL RFC9123 and also control operators from
RFC9234, ISO123456, and W3C REC better-cddl-1.

Needing to list all of those, or even just explicitly list the control
operators in use, seems like a pain, and doesn't seem to help old tools
reject new controls much earlier in processing.

Jeffrey