[Cbor] draft-ietf-cbor-edn-literals-10 implementation notes

[Joe Hildebrand <[e-mail]>]

I'm taking a pass over implementing draft-ietf-cbor-edn-literals-10.  Here are some comments:

- Overall, this spec is in pretty good shape.  Aside from the app-strings, I was able to implement directly from the ABNF using my standard tooling.  Reading the actual text of the spec answered most of my questions.

- Like others, I don't like the two-level ABNF.  I understand that you're going for extensibility, but you can still leave an extension point in place while having a single grammar.

- The doc structure is quite odd in that it presents the extension points before the main format.  I understand that's an outgrowth of how the doc grew over time, but it needs a small refactor before publishing.  I'm willing to provide more suggestions or help, if the authors want.

- I wish that floating point encodings had a separate spec syntax from integers, rather than relying on getting a decimal point or "e" into the output somehow (e.g. JS doesn't have good float formatting built in).  For example, 2_f1 could mean 0xf94000, while 2_1 would mean 0x190002.

- s3 could be moved after the ABNF section and the (possibly new) app-string section and make more sense.

- s3.2, "Herewith I buy" /.../ "gned: Alice & Bob" doesn't match the grammar.  I think it needs a +.

- s4.1 It should be an error to mix app-strings and strings, or app-strings of multiple types.  The text at the end of the section starting with "Some of the strings may be app-strings..." is not proscriptive enough.

- It's not clear to me what happens if there are multiple items in the sequence inside <<>>.  I assume they are concatenated together, even though that's a little odd unless you are generating cbor streams.  I would have expected the production to be:

    embedded = "<<" one-item ">>"

You would still get concatenation with << 1 >> + << 2 >> in the unlikely event you need it.

- (nit) I'd prefer basenumber to be split into 3 or 4 rules, one for each base, since each needs special processing.

- s4.2 could be outdented to s5, containing both ABNF and the descriptions of the app-string formats from s2.  Having to go back and forth made reading more difficult than it needed to be.

- s4.2.1, h'/head/ 63 /contents/ 66 6f 6f' should become << "cfoo" >>, not << "foo" >> if I'm understanding correctly.

- s6 Security Considerations seems like it could use some more text about how this format isn't intended for interchange.

I'm building up a large-ish set of test vectors.  I'm willing to put those into a separate repo for sharing if anyone is interested in collaborating.

I'm not quite done with the implementation, so there are likely to be a few more comments as I continue to dig.

— 
Joe Hildebrand