search

lake-wg / edhoc

Ephemeral Diffie-Hellman Over COSE (EDHOC)
Other
7 stars 12 forks source link

C_1: use "nil" instead of "null" #118

Closed chrysn closed 3 years ago

chrysn commented 3 years ago

CDDL knows both names for "null" and "nil", but (especially with #117) a careless reader could read the "prepending a null" as "prepend the byte zero", which is not what is meant (given that the prepending is introduced as "prepend the CBOR serialization of C_x").

A downside is that in CBOR itself, the value is always called "Null" -- but unlike CDDL (and COSE), it does not introduce much of terminology for the simple types (it just describes it as meaning "null"; most of the remaining use of the term is in the context of a JSON null).

I suggest this change in an issue rather than in a PR, as doing any change now while #117 is in the air would just create more work later anyway.

If this is accepted (and "let's better stick to plain CBOR terminology" is still a plausible outcome), I can PR this once #117's fate is decided.

emanjon commented 3 years ago

I am personally fine with both.

gselander commented 3 years ago

Agree "null" is confusing. Not sure "nil" is less confusing. The fact that RFC8610 defines null as nil may result in the same interpretation, and most people don't read beyond RFC8949 where the term "nil" is not used.

Would it be less ambiguous to pick another CBOR Simple Value, like "true" (0xf5)?

gselander commented 3 years ago

OTOH, if it just a matter of disambiguating the word "null", then we could use "nil" instead. Given that we stick to the simple CBOR value 0xf6, as in -08, then making the change is an improvement.

gselander commented 3 years ago

Given the usefulness to implementers of CBOR tools like cbor.me, I think there is a good reason to stick to the diagnostic notation in RFC 8949. For this reason I reopen this issue.

gselander commented 3 years ago

We should thus replace the current 'nil' with some 1-byte value with RFC 8949 diagnostic notation. Instead of reverting to 'null' I propose we pick another simple value. I did not note any objection to the proposal above to use 'true' and will therefore make that change now.

gselander commented 3 years ago

Change made in 31f2ce58d

emanjon commented 3 years ago

This has now been changes to true. Which make this issue outdated