Subject: [core] Adam Roach's Discuss on draft-ietf-core-links-json-07: (with DISCUSS and COMMENT)

From: Adam Roach <[email-masked]>
Date: Fri, 21 Apr 2017 16:11:46 -0700

Adam Roach has entered the following ballot position for
draft-ietf-core-links-json-07: 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-core-links-json/

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

- The protocol has technical flaws that will prevent it from working
properly, or the description is unclear in such a way that the reader
cannot understand it without ambiguity. 

Ambiguity 1: Section 1.1 describes "round-tripping" between 6690 format
and JSON/CBOR; however, the document describes the transform in one
direction, leaving the other implicit. The non-normative code in Appendix
A shows a conversion into link relation format, but it makes certain
assumptions about the syntax of link relation parameters that, while
possibly true today, are not guaranteed to hold true in the future. As
defined in this document, the transform requires parameter-specific
syntax knowledge of each link relation parameter that it will transform.
The problem this causes is probably best demonstrated with an example.

Let's say some future document defined a link relation ("smellslike")
that included a parameter with ABNF syntax like:

  "smellsys" "=" quoted-string

Now, if we were to apply the transform in this document to the following
link relation:

  </smellr2>;smellsys="h9341";rel="smellslike"

We get:

  [{"href":"/smellr2", "smellsys":"h9341","rel":"smellslike"}]

Applying the logic that is implicit in the document and explicit (if
non-normative) in Appendix A, converting this back into a link relation
yields:

  </smellr2>;smellsys=h9341;rel=smellslike

Note the stripping of double quotes from the parameter values. This is
fine for "rel", whose ABNF syntax allows for the presence or absence of
quotes -- but the emitted syntax for "smellsys" is actually in violation
of its defined ABNF.

This needs to be addressed by either indicating that conversion into link
relations requires that the conversion routine has explicit knowledge of
the syntax for each parameter it converts (and must fail to convert
parameters it does not understand), *or* the JSON and CBOR
representations need some additional indication that preserves knowledge
of whether the indicated value is required to be quoted when converted to
a link relation. If the first path is taken, the associated Ruby code
needs to be updated accordingly.

Ambiguity 2: The document requires that the thirteen defined values MUST
be encoded as integers. The document does not define what implementations
are to do if they receive a CBOR object that does not conform to this
encoding: is the parameter ignored? Is the entire link relation ignored?
Do you reject the entire collection of link relations? Or do you just go
ahead and parse it anyway, since the intended meaning is ambiguous (even
if out of spec)?

-  The draft omits a normative reference necessary for its
implementation, or cites such a reference merely informatively rather
than normatively. 

This document appears to use CDDL to define the formal schema for both
the JSON and CBOR representations of its data format, although the CDDL
document itself is cited only informatively. Additionally , figure 1
shows an application of CDDL to define schema for JSON. It's not clear
from a skim through the CDDL document that it can be used for JSON; it
would appear that using it in this fashion would require additional text
in this document to talk about how to apply CDDL to JSON, or waiting for
some other document to do so.

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

In section 2.2, the phrase "outer collection" is a bit difficult to
understand at first (on first reading, the sense of the word "outer" is
unclear); recommend a more descriptive phrase like "The collection of
link relations is represented as a JSON or CBOR array of links"

The first paragraph on page 5 is using normal English prose to describe
normative behavior, which is fine; however it plainly says that the
encoding is normal JSON name/value pairs, and that "the value is a
string," full stop, no exceptions. Further down, this section says the
value can be a boolean, which effectively contradicts the earlier
statement. Even further down, it says the value can be an array of
strings. It would be much easier to understand if the initial statement
said "the value can be a string, a boolean, or an array of strings, as
described below," and then have each of the three situations described
independently.

Both of the CDDL schemata seem to omit support for array values (i.e., if
a name appears twice in a link relation, as shown in Figure 4).

Section 2.3 opens with "The above specification could be used as is..."
-- there is a risk that implementors might read this as permission rather
than hypothetical musing. Suggest rephrasing like "...might have been
able to be used as-is..."

Table 1 reads is introduced with the phrase "The substitution is
summarized below," which had me looking for the actual definition
elsewhere in the document. Not finding one, I conclude this table
defines, rather than summarizes, the substitution. (Replace "summarized"
with "defined").

One would presume the paragraph below table 1 should be removed.

The example in Figure 6 would benefit greatly by showing both the array
encoding and "foo" encoding used in Figure 4 (including, in particular,
the string -- rather than integer -- encoding of the "foo=3" parameter).

---

From: Carsten Bormann <[email-masked]>
Date: Sat, 22 Apr 2017 10:03:47 +0200

Hi Adam,

thank you very much for this deep review.
This will allow us to improve the document a lot.

Let me pick one issue here that I think goes beyond the details of links-json.

>  "smellsys" "=" quoted-string

My knee-jerk reaction is “then don’t do that”.

But the more fundamental problem is that RFC 5988 actually defines a data model and its serialization without ever saying so.  Any reasonably defined link parameter will either have a syntax that is limited to ptoken or allow both ptoken and quoted-string serialization.
The fact that we then use ABNF to define the link parameter syntax obscures that property and allows the above deviation.  We already made the mistake for [anchor, title, rt, if] (see MUSTBEQUOTED in the reference implementation).  There is no need to make it again,  but the use of ABNF for what should be a data model definition does make it too easy to shoot ourselves in the foot.

Note that the above issue hits any implementation that wants to generate RFC 5988 syntax, not just one that converts links-json into RFC 5988 syntax.

So my takeaway summary is that links-json only works for reasonably defined link parameters (after grandfathering in anchor, title, rt, if) and probably should say so much more explicitly.  But I don’t know how to fix RFC 5988 about this issue.

Grüße, Carsten

---

From: Carsten Bormann <[email-masked]>
Date: Sat, 22 Apr 2017 10:32:47 +0200

Interesting.
Julian Reschke reminds me that this is repaired in draft-nottingham-rfc5988bis-05.txt:

3.  Link Serialisation in HTTP Headers

   The Link header field provides a means for serialising one or more
   links into HTTP headers.

   The ABNF for the field value is:

     Link       = #link-value
     link-value = "<" URI-Reference ">" *( OWS ";" OWS link-param )
     link-param = token BWS "=" BWS ( token / quoted-string )

   Note that any "link-param" can be generated with values using either
   the "token" or the "quoted-string" syntax, and therefore recipients
   MUST be able to parse both forms.  Individual "link-param"s specify
   their syntax in terms of the value after any necessary unquoting (as
   per [RFC7230], Section 3.2.6).

Unfortunately, this doesn’t retroactively fix RFC 6690, which still has restrictive ABNF for rt, if, title, anchor.  Hmm.

Grüße, Carsten

> On Apr 22, 2017, at 10:03, Carsten Bormann <[email-masked]> wrote:
> 
> Hi Adam,
> 
> _______________________________________________
> core mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/core
> 
>

---

From: Alexey Melnikov <[email-masked]>
Date: Mon, 24 Apr 2017 10:00:12 +0100

Carsten and Adam,

> On 22 Apr 2017, at 09:32, Carsten Bormann <[email-masked]> wrote:
> 
> Interesting.

I think this document should recognize this change and just reference draft-nottingham-rfc5988bis normatively. (I will be AD sponsoring it shortly anyway.) Adam, will this address this concern

> Unfortunately, this doesn’t retroactively fix RFC 6690, which still has restrictive ABNF for rt, if, title, anchor.  Hmm.

Well, we just have to special case them. The list of special attributes is fixed.

---

From: Adam Roach <[email-masked]>
Date: Mon, 24 Apr 2017 11:01:48 -0500

> I think this document should recognize this change and just reference draft-nottingham-rfc5988bis normatively. (I will be AD sponsoring it shortly anyway.) Adam, will this address this concern

I think citing 5988bis is definitely called for, but not sufficient to 
solve this issue. The thing that actually prevents the situation I 
describe from occurring is that 5988bis will prevent new link relation 
parameters of type quoted-string (only) from being defined, and my read 
of 6690 is that the syntax of CoAP link relations will mirror those of 
5988bis link relations, so I'm far less concerned about the 
future-proofness of this protocol.

In terms of the issue at hand, I think this document really needs to 
have English prose that captures the logic in the Ruby script (since the 
script is non-normative).

My reading is that these rules are:

  * Always include quotes for the known special cases (anchor, title,
    rt, and if).
  * For other parameters, omit quotes if the value is composed entirely
    of token characters
  * Include quotes otherwise

This necessarily requires some description of how to take the JSON/CBOR 
format and convert it to HTTP/CoAP format, but that's something I think 
the document really needs anyway.

/a

---

From: Alexey Melnikov <[email-masked]>
Date: Mon, 24 Apr 2017 17:17:57 +0100

Hi Adam,

On Mon, Apr 24, 2017, at 05:01 PM, Adam Roach wrote:
>> I think this document should recognize this change and just reference
>> draft-nottingham-rfc5988bis normatively. (I will be AD sponsoring it
>> shortly anyway.) Adam, will this address this concern> I think citing 5988bis is definitely called for,

Let's at least do this.

> but not sufficient to solve this issue. The thing that actually
> prevents the situation I describe from occurring is that 5988bis will
> prevent new link relation parameters of type quoted-string (only) from
> being defined, and my read of 6690 is that the syntax of CoAP link
> relations will mirror those of 5988bis link relations, so I'm far less
> concerned about the future-proofness of this protocol.So other than the currently defined parameters (in rfc5988bis or RFC
6690), this situation will not occur, right?
> have English prose that captures the logic in the Ruby script (since
> the script is non-normative).Ok.

>  * Include quotes otherwiseSeems sensible.
> This necessarily requires some description of how to take the
> JSON/CBOR format and convert it to HTTP/CoAP format, but that's
> something I think the document really needs anyway.Agreed.

---

From: Adam Roach <[email-masked]>
Date: Mon, 24 Apr 2017 11:19:56 -0500

On 4/24/17 11:17, Alexey Melnikov wrote:
>> I think citing 5988bis is definitely called for,
>>
> Let's at least do this.
>
>> prevent new link relation parameters of type quoted-string (only) 
>> from being defined, and my read of 6690 is that the syntax of CoAP 
>> link relations will mirror those of 5988bis link relations, so I'm 
>> far less concerned about the future-proofness of this protocol.
> So other than the currently defined parameters (in rfc5988bis or RFC 

As far as I can tell, that is true.

/a