Subject: [Extra] Adam Roach's No Objection on draft-ietf-extra-imap-fetch-preview-03: (with COMMENT)

From: Adam Roach via Datatracker <[email-masked]>
Date: Mon, 08 Apr 2019 13:38:09 -0700

Adam Roach has entered the following ballot position for
draft-ietf-extra-imap-fetch-preview-03: No Objection

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-extra-imap-fetch-preview/

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

Thanks to everyone who has worked on this document. I have a handful of comments
that should be considered prior to publication.

---------------------------------------------------------------------------

§3.1:

>  These algorithms MUST be one
>  of the algorithms identified as supported in the PREVIEW capability
>  responses.

This is confusing, as "algorithms" and "one of" don't make sense with each
other. Is it meant to say something like "...MUST be a subset of the
algorithms..."?

---------------------------------------------------------------------------

§3.1:

>  If a client requests an algorithm that is unsupported,
>  the server MUST return a tagged BAD response.

This appear to be underspecified, or at least nonintuitive. As written,
it seems to say that an algorithm list of (known-1, known-2, unknown, known-3)
would be considered invalid, even though there are plenty of supported,
requested algorithms that could be used. Is this actually intended to be an
error? If so, please make it explicit, as it's pretty counterintuitive.

(As an aside: it's also unnecessarily fragile from an interop perspective, so I
would suggest that this is not the desired behavior: I believe you want to
return an error only when *none* of the requested algorithms are supported).

---------------------------------------------------------------------------

§6, example 5:

>    C: E1 CAPABILITY
>    S: * CAPABILITY IMAP4rev1 PREVIEW=FUZZY SEARCHRES
>    S: E1 OK Capability command completed.
>    [...a mailbox is SELECTed...]
>    C: E2 SEARCH RETURN (SAVE) FROM "FOO"
>    C: E3 FETCH $ (UID PREVIEW (LAZY=FUZZY))

This example shows the use of a modifier ("LAZY") with an algorithm; however,
this modifier doesn't appear to be advertised by the server in its CAPABILITY
line. If I understand how this is supposed to work (looking at the definitions
in section 7), I would have expected:

     S: * CAPABILITY IMAP4rev1 PREVIEW=FUZZY PREVIEW=LAZY SEARCHRES

I'll note that this syntax effectively places algorithms and modifiers in the
same namespace, although IANA doesn't seem to be given any explicit
instructions about this. I think this needs to be cleaned up prior to
publication.  I would make this last point a DISCUSS, except that it appears
to be covered by Benjamin's DISCUSS already.

---------------------------------------------------------------------------
§1:

>  Using server generated previews allows global generation once per0

nit: "server-generated"

---

From: "Alexey Melnikov" <[email-masked]>
Date: Wed, 10 Apr 2019 08:52:31 -0400

Hi Adam,
Thank you for your comments:

I think this section is a mess. If we want to allow for multiple algorithms here, then, as you point out, handling of error cases need to be described. This will be fixed.

> instructions about this. 

Right, Barry has DISCUSS on this point.

> I think this needs to be cleaned up prior to

---

From: Michael Slusarz <[email-masked]>
Date: Wed, 10 Apr 2019 21:45:48 -0600 (MDT)

Adam,

Thanks for your comments.  Discussion below.

> On April 8, 2019 at 2:38 PM Adam Roach via Datatracker <[email-masked]> wrote:

Noted that the topic of multiple algorithm selection is being discussed in Barry's DISCUSS.

That being said, if multiple algorithm support is decided to be kept, what about changing this to:

"All algorithms specified in the list MUST be identified as being supported by the server by appearing in a PREVIEW capability response." (or "PREVIEW= capability response" maybe?)

> return an error only when *none* of the requested algorithms are supported).

See Barry's DISCUSS response.  Agreed that error handling is not specified adequately in current text.

To mirror duplicate algorithm language, how about: "Unsupported algorithms in the list MUST be ignored. If the list contains no supported algorithms, the server MUST return a tagged BAD response to the FETCH command."

This has been discussed in several other places already.  I am proposing removing priority modifier extensions, which would address this concern.

michael

---

From: "Alexey Melnikov" <[email-masked]>
Date: Thu, 11 Apr 2019 06:23:23 -0400

Hi Michael,

On Thu, Apr 11, 2019, at 4:46 AM, Michael Slusarz wrote:
> "All algorithms specified in the list MUST be identified as being 
> supported by the server by appearing in a PREVIEW capability response." 
> (or "PREVIEW= capability response" maybe?)

You already need to deal with unrecognized algorithms (the text you suggested below), so having a MUST here doesn't make sense.

> To mirror duplicate algorithm language, how about: "Unsupported 
> algorithms in the list MUST be ignored. If the list contains no 
> supported algorithms, the server MUST return a tagged BAD response to 
> the FETCH command."

This looks reasonable to me.

> This has been discussed in several other places already.  I am 
> proposing removing priority modifier extensions, which would address 
> this concern.

I think this is a reasonable course of action.

Best Regards,
Alexey