Subject: [Jmap] WGLC review of draft-jmap-blob-07

From: Jim Fenton <[email-masked]>
Date: Mon, 06 Dec 2021 16:42:44 -0800

I had a look at draft-ietf-jmap-blob-07, and have a few comments. Please bear in mind that I don’t have a great deal of experience with the other jmap documents, so some of this might be due to that.

General:

I found the large number of examples in the document to be a little distracting. I would prefer to see them in a non-normative appendix. At a minimum, it should be made clear that they’re informative examples, so they aren’t depended upon in the event of a conflict between the normative text and the example.

Specific sections:

Section 3: This appears to be a normative reference to [RFC8620], so that document should be moved from informative to normative in the references.

Section 3.1: “This” is apparently a reference to the section title. What is it about “this” that represents support: I guess the advertisement of this string in the capabilities object? Please make that more specific so that the section fully specifies what happens. What happens if some of the parameters (e.g., MaxSizeBlobSet) aren’t included: do they assume default values are are the limits nonexistent? If this appears in accountCapabilities, I presume it also has to appear in capabilities; is that specified somewhere?

Section 4.1.1: Is this a subset of the Blob/set method, and if so how is it distinguished? “exactly one of”, “Also optionally” isn’t as formal a definition as I’m used to in IETF standards-track documents. Often, something like this is specified using ABNF. I don’t have a good idea if this is unambiguous or not.
“can not contain” -> “MUST not contain”

Sections 4.1.2, 4.1.3: will result -> MUST result

Section 4.2: Do the null meanings of offset and length have the same meaning as for create? Is it possible to request both data:asText and data:asBase64? Some of the requirements seem not to be in normative style, e.g. “the size value is always” might be stated as “the size value MUST be” and “the key isEncodingProblem is set to true” -> “the key isEncodingProblem MUST be set to true”

Section 6.3: I’m confused about creating a registry where the existing entries reference a number of different existing specifications. Is this a registry that was overlooked in the past? (If so, glad you’re adding it)

Nit:

Is blob capitalized? It’s inconsistent but mostly capitalized here, but RFC 8620 doesn’t capitalize it.

Hope this helps,

-Jim (without hat)

---

From: "Bron Gondwana" <[email-masked]>
Date: Tue, 21 Dec 2021 00:46:29 +1100

Thanks Jim, and sorry for taking so long to get back to you!

Neil - you're added to explicit CC because I have a question for you in here about the 'ifInState' and 'state' fields in a Blob/{get,set} and what we should do about it.

> I found the large number of examples in the document to be a little distracting. I would prefer to see them in a non-normative appendix. At a minimum, it should be made clear that they’re informati…

This is tricky in general - I have found that not having enough examples leads to poorer quality implementations.  In the event of a conflict between the normative text and an example, that's just as bad as a conflict between one part of the normative text and another part of the normative text, and requires errata.  I don't see much value in "making it clear that they're informative" because people will check against them anyway.

Which probably means for the one which I wrote by hand, I should test it against a real implementation!  I'm as sure that it's right as I am that the prose is right though!

All the other examples exist as test cases in our Cassandane test framework for Cyrus IMAP, because that was easier than trying to write them by hand!

> Section 3: This appears to be a normative reference to [RFC8620], so that document should be moved from informative to normative in the references.

Oh for sure - that will be mmark.  I used the "normative" version of the RFC link coding: `[@!RFC8620]` everywhere except the registry table, so it decided I didn't really mean normative.  Woohoo.  Fixed.

Thanks, and my apologies for not proof-reading that myself.  The other two informative references should stay that way, as you can implement this without them.

> 
> Section 3.1: “This” is apparently a reference to the section title. What is it about “this” that represents support: I guess the advertisement of this string in the capabilities object? Please make that more specific so that the section fully specifies what happens. 

Updated with language cribbed from RFC9007 which is the most recent published JMAP extension that does something similar.

> What happens if some of the parameters (e.g., MaxSizeBlobSet) aren’t included: do they assume default values are are the limits nonexistent? If this appears in accountCapabilities, I presume it also has to appear in capabilities; is that specified somewhere?

No, it can only appear in accountCapabilities, as specified in the previous paragraph.

I have gone back and made it a MUST that servers support 64 items, and that clients assume that if it's not present.  Likewise I've said that a lack of maxSizeBlobSet just means "assume that it's maxSizeUpload".

> 
> Section 4.1.1: Is this a subset of the Blob/set method, and if so how is it distinguished?

A standard set method, per RFC8620:

https://datatracker.ietf.org/doc/html/rfc8620#section-5.3

It is distinguished by being the `create` argument to a standard `/set` method.  All `/set` methods take arguments.  But you make a good point - I should probably define that Blobs do not have a state as such, so it is an error to provide ifInState argument.

Neil - do we have a good generic way to handle objects for which there's no valid `state`?

> “exactly one of”, “Also optionally” isn’t as formal a definition as I’m used to in IETF standards-track documents. Often, something like this is specified using ABNF. I don’t have a good idea if this is unambiguous or not.

JMAP has not used ABNF in any of its specifications.  How would you propose saying a set containing precisely one of A, B or C in prose?

> “can not contain” -> “MUST not contain”

Yes, thanks - fixed.

> Sections 4.1.2, 4.1.3: will result -> MUST result

Hmm.... maybe.  At least within the scope of this document that is true.

It would be possible within this JMAP syntax for a future specification (which must be selected with a new capability) to alter this behaviour (for example: by adding an expiry time to unreferenced blobs and allowing `update` to change the expiry time and `delete` to remove the blob immediately).  So I don't want to sling too many MUSTs around for the behaviour of things which are legitimate extension points.

I would prefer to leave this as is.  It's clear enough without encouraging implementations to hard code assumptions by scattering MUST.

> Section 4.2: Do the null meanings of offset and length have the same meaning as for create?

Yes.  I guess it was unclear if you queried it.  Improved.

> 
> Is it possible to request both data:asText and data:asBase64? 

Yes.  This is quite expected to anybody familiar with JMAP, where you can fetch multiple representations of the same data in the same request.  RFC8621 in particular allows fetching different representations of headers using very similar syntax - but it's an error to set the same field twice, e.g:

   o  If a "bodyStructure" property is given, there MUST NOT be
      "textBody", "htmlBody", or "attachments" properties.

   o  If given, the "bodyStructure" EmailBodyPart MUST NOT contain a
      property representing a header field that is already defined on
      the top-level Email object.

   o  Within an EmailBodyPart:

      *  The client may specify a partId OR a blobId, but not both.  If
         a partId is given, this partId MUST be present in the
         "bodyValues" property.

But you can certainly fetch either or both with an Email/get.

> Some of the requirements seem not to be in normative style, e.g. “the size value is always” might be stated as “the size value MUST be” and “the key isEncodingProblem is set to true” -> “the key isEncodingProblem MUST be set to true”

Hmm.  Yes.  Am fixing these ones and reviewing for similar.

> Section 6.3: I’m confused about creating a registry where the existing entries reference a number of different existing specifications. Is this a registry that was overlooked in the past? (If so, g…

Yes.  There was much discussion on that very topic.  It was overlooked to a large extent because there was no real need for it until this document added backreferences.  You could also follow the JMAP capabilities registry to find the documents for each one, and then read the descriptions for the types that they provide... though that wasn't really clear about whether you could ask for push notifications about them.

And definitely the existing documents did not specify whether their types were amenable to being used for backreferences - because that wasn't a thing yet! - so this document defines that for the existing types in existing documents.  Future documents will need to specify each of their types and whether they can be used for backreferences or push notifications in this registry as well.

> Is blob capitalized? It’s inconsistent but mostly capitalized here, but RFC 8620 doesn’t capitalize it.

Mu.  Blob-the-object is capitalised, in the same way that RFC8621 capitalises 'Email' when referencing an object.  It shouldn't be otherwise.  I've gone through and lowered all the cases where it's not directly referencing the JMAP wire object.   I could see an argument for going the other way too, but I'm happy with lower.

I've uploaded a -08 just to show my workings.  Let me know if that resolves everything to your satisfaction other than maybe the open question that I've asked Neil for advice/suggestions on.

Thanks,

Bron.

--
  Bron Gondwana, CEO, Fastmail Pty Ltd
  [email-masked]

---

From: Jim Fenton <[email-masked]>
Date: Tue, 21 Dec 2021 13:57:15 -0800

Inline…I wasn’t sure where to cut so this might be a bit of a verbose reply.

> All the other examples exist as test cases in our Cassandane test framework for Cyrus IMAP, because that was easier than trying to write them by hand!

I completely agree on the importance of examples. This is mostly a style thing, but I find the spec easier to navigate with the examples concentrated in one or more appendices. But this is fine if you and others prefer in-line examples.

> Thanks, and my apologies for not proof-reading that myself.  The other two informative references should stay that way, as you can implement this without them.

Sounds good.

> I have gone back and made it a MUST that servers support 64 items, and that clients assume that if it's not present.  Likewise I've said that a lack of maxSizeBlobSet just means "assume that it's m…

Good; thanks.

> Neil - do we have a good generic way to handle objects for which there's no valid `state`?

Thanks for the explanation; will wait for Neil’s response as well.

> JMAP has not used ABNF in any of its specifications.  How would you propose saying a set containing precisely one of A, B or C in prose?

The place I’m having the most trouble is the syntax of the CatenateSourceObject in section 4.1.1. Is the blobId source a third alternative to data:asText or data:asBase64? Or is it in addition? It might be clearer if there was a defined [blobIdSource] think and you listed that as a third alternative. Otherwise it’s confusing because the “or a blobId source” is at the same level as the “exactly one of”.

> I would prefer to leave this as is.  It's clear enough without encouraging implementations to hard code assumptions by scattering MUST.

I still see this as specifying unambiguously what the behavior of the server should be. If there’s an extension later it would be “updates RFC xxxx” and can change that behavior.

> Yes.  I guess it was unclear if you queried it.  Improved.

Thanks.

> But you can certainly fetch either or both with an Email/get.

Thanks for educating me on this.

> Hmm.  Yes.  Am fixing these ones and reviewing for similar.

Thanks.

> And definitely the existing documents did not specify whether their types were amenable to being used for backreferences - because that wasn't a thing yet! - so this document defines that for the e…

Makes sense.

> Mu.  Blob-the-object is capitalised, in the same way that RFC8621 capitalises 'Email' when referencing an object.  It shouldn't be otherwise.  I've gone through and lowered all the cases where it's…

You might want to check 4.1.2 and 4.1.3 again on that.

> I've uploaded a -08 just to show my workings.  Let me know if that resolves everything to your satisfaction other than maybe the open question that I've asked Neil for advice/suggestions on.

See above. Probably wait for Neil before turning the crank on another revision.

-Jim