Subject: Re: [Gen-art] Genart last call review of draft-ietf-calext-jscalendar-25

From: Alissa Cooper <[email-masked]>
Date: Wed, 23 Sep 2020 11:15:19 -0400

Robert, thanks for your review. Neil, thanks for the updates. I entered a No Objection ballot.

Alissa

> On Mar 7, 2020, at 9:58 PM, Neil Jenkins <[email-masked]> wrote:
> 
> Thanks for the review Robert.
> 
>> ABNF is used, but there is no reference to RFC 5234.
> 
> I have added a normative reference to this RFC. 
> 
>> The first registry in section 8.4.3 should be explicit that it is a registry for values of the "@type" property.
> 
> That's not quite correct; the registry is for the names of types. Not all types have an "@type" property (for example, primitive types like UnsignedInt). The "@type" property is primarily to help where the context allows multiple types (e.g. the "entries" property in a JSGroup object is a list of JSEvent and/or JSTask objects, so you need the @type property on these to know which one you are getting).
> 
>> It's not stated clearly whether a patch should succeed or fail if the resulting
>> object doesn't meet this specification's requirements. 
> 
> Good spot. I have added a requirement for the PatchObject to be valid that:
> 
> The value for the patch MUST be valid for the property being set (of the correct type and obeying any other applicable restrictions), or if null the property MUST be optional.
> 
> (Thus if this is not true the whole PatchObject should be ignored, as per any other patch failure.)
> 
>> Side question: if a patch changes 'updated' (4.1.6) in an object that didn't already have a 'created' (4.1.5), should it fail if it doesn't also result in an object that has a 'created'? Or is it ok to lose the original creation time information?
> 
> No, this doesn't fail. The "created" property is optional, so you can have an object that doesn't record its creation date.
> 
>> The security consideration section only points to section 7 of RFC 3986 for
>> potential issues related to the URIs that can be carried in this
>> representation. I don't think that's sufficient. There should be some
>> discussion (or a pointer to discussions) about the potential for malicious
>> construction of jscalendar objects containing potentially very large numbers of
>> URIs in, say, as Link objects (4.2.7). Is there an opportunity for amplified
>> attacks here? (Especially if these URLs might be automatically referenced by
>> any client, and even more so if the object is sent from a calendar with a large
>> number of subscribers.)
> 
> I have added:
> 
> A maliciously constructed JSCalendar object may contain a very large number of URIs. In the case of published calendars with a large number of subscribers, such objects could be widely distributed. Implementations should be careful to limit the automatic fetching of linked resources to reduce the risk of this being an amplification vector for a denial-of-service attack.
> 
>> Have you considered any tighter integrity checking for (4.2.7) links? Maybe a
>> checksum property?
> 
> No. The integrity of data is left to the transmission protocol and not part of the data format being described in this document. I don't see any good reason why this property should be subject to extra integrity checks beyond the whole object.
> 
>> It doesn't look like application/jscalendar+json has had a media type review.
> 
> Thanks, I have now sent this for review.
> 
>> Nits:
> 
> I have addressed these, thanks for noting them. One comment:
> 
>> I don't expect anything to change at this point, but I do have to point to the
>> dissonance in the conventions A[] and A[B]. It would have been far less
>> confusing for A have the same semantic in both cases (preferably value), than
>> the current situation where it means value for A[], but key for A[B].
> 
> I see your point, but we have already used the same syntax in JMAP (RFC8620/8621) and I think it is better to stay consistent.
> 
> I have published v26 <https://tools.ietf.org/html/draft-ietf-calext-jscalendar-26> with all of the above updates.
> 
> Cheers,
> Neil.
> _______________________________________________
> Gen-art mailing list
> [email-masked] <mailto:[email-masked]>
> https://www.ietf.org/mailman/listinfo/gen-art <https://www.ietf.org/mailman/listinfo/gen-art>