Elwyn, many thanks for your review. I’ve included it in my ballot position and am holding a DISCUSS while the major issue gets resolved.
Authors/WG, thanks for engaging with Elwyn’s review. Alissa > On Apr 25, 2017, at 6:19 PM, Elwyn Davies <[email protected]> wrote: > > Reviewer: Elwyn Davies > Review result: Not Ready > > I am the assigned Gen-ART reviewer for this draft. The General Area > Review Team (Gen-ART) reviews all IETF documents being processed > by the IESG for the IETF Chair. Please treat these comments just > like any other last call comments. > > For more information, please see the FAQ at > > <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>. > > Document: draft-ietf-core-links-json-07 > Reviewer: Elwyn Davies > Review Date: 2017-04-25 > IETF LC End Date: 2017-04-21 > IESG Telechat date: 2017-04-27 > > Summary:Not ready for publication. There a number of issues that need > to be addressed as discussed below. In particular whether the > formats could be returned as the web link specification instead of RFC > 6990 format in response to a GET /.well-known/core request. > > Having thought about the quote stripping/addition issue cited in Adam > Roach's DISCUSS, I would take a slightly different view... see below. > > Major issues: > > Intentions: > Is it one of the intentions of this draft that a server should be able > to return web link descriptions using JSON or CBOR, specifically in > response to GET /.well-known/core? Content formats for the new > formats are registered (s3.2) - could a user ask for the alternative > formats by specifying at ct filter with the GET request? It strikes > me that if one has a constrained server of sufficiently limited > capabilities that it wants to use CBOR then having to encode the RFC > 6690 format responses for the web links requests is wasting resources. > Some more thought needs to be given to this as an update to RFC 6690 > - If I read correctly, RFC 6690 implicitly requires that a response to > GET /.well-known/core MUST be encoded as described in RFC6690. > > Minor issues: > Title: The document appears only to address the CoRE Web Links format > rather than any other. Should the title reflect this more precisely, > e.g., > Representing CoRE Web Links Format in JSON and CBOR > > s1.1: Concerning: > o The simplest thing that could possibly work > > * Do not cater for RFC 5988 complications caused by HTTP > header > character set issues [RFC2047] > > Having ferreted around in RFC 5988 and RFC 2047, I can't see what is > being referred to here. However, I observe later that the "title*" > attribute (with language specifier) does not appear to be supported > (It is missing from Table 1) - is this what is relevant here? If so it > needs clarification. > [Aside: I notice that the relevant ABNF in s5 of RFC 5998 is missing > external references to various productions (e.g., ext-value, > quoted-string) that are defined in other documents - in the given > examples RFC 2987, RFC 2616.] > > s2.2/s5: This statement: > The resulting structure can be represented in CDDL > [I-D.greevenbosch-appsawg-cbor-cddl] as: > > requires that the CDDL draft is a normative reference rather than > informative. > [Aside: Having skimmed the CDDL draft, I am of the opinion that a > good deal more work will be needed to get this ready for publication, > possibly to the extent that the CDDL quoted here becomes invalid. > Given the simplicity of the specification could it be done without the > use of CDDL?] > > s2.2: There needs to be some discussion of handling of double quoted > and non-double quoted strings during conversion: > I think it works to require that... >> From RFC 6690 to JSON: > - If the parameter value is a double quoted string then it should have > the double quotes stripped, any necessary JSON character encodings > performed and the double quotes repapplied. > - if the parameter value is anything else, then the necessary JSON > character encodings are done and the result enclosed in double > quotes. > [what about % encodings on the RFC 6690 side?] > >> From RFC 6690 to CBOR: > - If the parameter value is a double quoted string, the double quotes > are stripped and the result used as the CBOR string type value. > - Otherwise, the parameter value is used as the CBOR string value. > [what about % encodings on the RFC 6690 side?] > >> From JSON to RFC 6690: > - Remove the double quotes from the JSON string value and do any > necessary decoding and encoding. Reapply double quotes. Note that > this may result in values that were originally not enclosed in double > quotes in the RFC 6690 repreentation becoming enclosed in double > quotes. However, [AFAICS] this does not alter the semantics of any of > the predefined parameters. For example the ABNF productions mean that > ct=40 and ct="40" are equivalent (the second case is needed so that > one can also have ct="40 41 42"). What IS needed is a statement that > this must also apply to any application specific parameters. For > example the case in examples 4 and 5 of ..;foo="bar";foo=3;... > transforming to "foo":["bar","3"] and then back to > ...;foo="bar";foo="3";.. MUST require that the two RFC 6690 > representations are equivalent. > >> From CBOR to RFC 6690: > [Essentially the same process - decode/encode and apply double quotes. > The discussion of equivalent semantics is equally applicable.] > > The conversion from CBOR to JSON or in reverse is similar. > > s2.3: It is not stated whether a CBOR decoder should accept literal > use of the encodable parameters - i.e., if the encoded CBOR contains [ > "href": "/mumble" ] rather than [1 : "mumble£ ] in CDDL format. > Similarly, should the use of the encoded values be mandatory on the > CBOR encoder? > > s2.3, Table 1: Is the omission of title* from the list of parameter > names deliberate? If so the omission justifies a note and rationale. > Clearly the format of the value for a title* parameter is different > from all the others, which may have something to do with this. > > s2.3/s5: eEfereences in Table 1 make RFC 7252 and RFC 7641 normative. > > Nits/editorial comments: > s1: s/e.g. /e.g., / (two places) > > s1.1: The term "round-tripping" and the associated text are opaque > jargon that would normally be applied to message transmission round a > loop rather than format conversion. A more explicit formulation would > help naive readers. Suggest (if I understand what was intended): > OLD: > o Canonical mapping > > * lossless round-tripping with [RFC6690] and between JSON and > CBOR > > * but not trying for bit-preserving (DER-style) round-tripping > NEW: > o Canonical mapping > > * supporting inter-conversion in both directions between any > pair > of [RFC6690] format and the CBOR and JSON formats defined > here > with unaltered and unambiguous semantics > > * but not attempting to ensure that a sequence of conversions > from > one of the formats through one or both of the others and back > to > the original would result in an identical representation > (c.f., > as might be achieved by different BER transcoders rather than > by all > DER transcoders with ASN.1 [X.690]). > ENDS > This needs an informative reference to X.690 ... but I am not sure > that the DER comparison is essential. > > s2.2: Suggest: > OLD: > We straightforwardly map: > > o the outer collection to an array of links; > > o each link to a JSON object or CBOR map, mapping attribute names > to > attribute values. > > NEW: > We straightforwardly map: > > o the outer collection to an array of parameterized web links; > > o each parameterized web link to a JSON object or CBOR map, > mapping attribute names to > attribute values. > ENDS > > s2.2: > OLD: > The resulting structure can be represented in CDDL > [I-D.greevenbosch-appsawg-cbor-cddl] as: > NEW: > The resulting structure can be represented in CBOR Data Definition > Language (CDDL) > [I-D.greevenbosch-appsawg-cbor-cddl] as shown in Figure 1. > > s2.4: Note that the use of ct=40 in RFC 6690 is an anchronism. The ct > parameter appeared in earlier versions of the draft that led to RFC > 6690 but was moved out to be used more generally in CoAP and is > actually defined in RFC 7252 as mentioned in Table 1 here. Thus use > of ct=40 in the example copied from RFC 6690 really needs an erratum > for 6690 but that is for another day! > > _______________________________________________ > Gen-art mailing list > [email protected] > https://www.ietf.org/mailman/listinfo/gen-art _______________________________________________ Gen-art mailing list [email protected] https://www.ietf.org/mailman/listinfo/gen-art
