Summary: Has a DISCUSS. Has enough positions to pass once DISCUSS positions are resolved.
— Section 3.1 — I don’t understand “the client’s priority decision”: what decision is that? And what’s the point of giving the server a list of algorithms here, given that they all have to be ones that are supported by the server? Won’t the server always have to use the first one in the list? If not, please add some text explaining what the server does. — Section 3.2 — If the preview is not available, the server MUST return NIL as the PREVIEW response. A NIL response indicates to the client that preview information MAY become available in a future PREVIEW FETCH request. Note that this is semantically different than returning a zero-length string, which indicates an empty preview. I think the MUST here is hard to follow, because the text doesn’t make a clear enough distinction between “preview is not available” and “an empty preview”. Can you expand the text a bit to explain the distinction more clearly, as this is a protocol requirement? Also, as I noted in response to Meral’s Gen-ART review it would be good to be clear how encrypted messages should be handled in this regard. — Section 4.1 — The preview text MUST be treated as text/plain MIME data by the client. I think this requires a normative reference to RFC 2046. — Section 7 — As was mentioned in Ben’s review, either the ABNF for “capability” is in error (it should not include “preview-mod-ext”) or the description needs to be significantly beefed up. I’m guessing that the intent is that PREVIEW= capabilities include both algorithms and modifiers, that PREVIEW=FUZZY is required, that the presence of any preview algorithm implies PREVIEW=LAZY such that the latter not only need not be specified, but is not permitted to be. So we might have “PREVIEW=FUZZY PREVIEW=FURRY PREVIEW=SLEEPY”, which would mean we support the algorithms FUZZY and FURRY, and the modifiers LAZY and SLEEPY. Is that correct? That seems somewhat obtuse to me, overloading the PREVIEW= capability and inviting confusion. — Section 8 — It seems like a bad idea to have to keep the IMAP Capabilities registry in sync with the two new registries: as it stands, when you add a new algorithm you have to add it to the Preview Algorithms registry, and also add a corresponding entry in the Capabilities registry... and similarly for a modifier, if I have that right above. Why not follow the model of AUTH= and RIGHTS=, and just reserve the PREVIEW= capability in the registry, allowing it to apply to entries from the two new registries? That avoids inconsistencies in registrations if we later add algorithms or modifiers.
— Section 3.1 — Nit: Please change “alternately” to “alternatively”. These algorithms MUST be one of the algorithms identified as supported in the PREVIEW capability responses. There’s a number-agreement problem here. NEW All algorithms in the list MUST have been included in the list of algorithms identified as supported in the PREVIEW capability responses. END — Section 3.2 — This relaxed requirement permits a server to offer previews as an option without requiring potentially burdensome storage and/or processing requirements to guarantee immutability for a use case that does not require this strictness. That’s sensible, but can you include some text giving an example of a situation where the preview might change? Given that the messages themselves are immutable, why would applying the same algorithm to the same text give different results? — Section 4.1 — The server SHOULD limit the length of the preview text to 200 preview characters. This length should provide sufficient data to generally support both various languages (and their different average word lengths) and different client display size requirements. The server MUST NOT output preview text longer than 256 preview characters. The text here should make it clear, because many implementers do not understand the difference, that these refer to *characters*, not *bytes*, and that 200 or 256 characters can possibly be much longer than 256 bytes. I worry that an implementer might allocate a buffer of 256 bytes, thinking that’s enough, and have it overflowed. The server SHOULD remove any formatting markup that exists in the original text. This is OK as it is, but perhaps a bit more specific than necessary. I think the sense is that the server is meant to do its best to render the preview as plain text, because that’s what the client will treat it as. As such, I would fold this into the earlier paragraph that talks about no transfer encoding, and maybe say it something like this: The generated string will be treated by the client as plain text, so the server should do its best to provide a meaningful plain text string. The generated string MUST NOT be content transfer encoded and MUST be encoded in UTF-8 [RFC3629]. For purposes of this section, a "preview character" is defined as a single UCS character encoded in UTF-8. The server SHOULD also remove any formatting markup, and do what other processing might be useful in rendering the preview as plain text.
I support Barry's DISCUSS point on Section 3.2. In Section 4.1, I suggest s/The FUZZY algorithm/The FUZZY algorithm identifier/ (since it doesn't refer to an actual algorithm)
Thank you for addressing my DISCUSSes and comments.
[Re-issuing ballot position to direct discussion of the ABNF for 'capability' to Barry's ballot thread; the rest of the COMMENT is unchanged] Section 3.1 Alternately, the client may explicitly indicate which algorithm(s) should be used in a parenthesized list after the PREVIEW attribute containing the name of the algorithm. These algorithms MUST be one nit: there's potential for misparsing here (e.g., that the PREVIEW attribute contains the name of the algorithm and the parenthesized list is after that), so maybe put "after the PREVIEW attribte" in parentheses or offset by commas. of the algorithms identified as supported in the PREVIEW capability responses. [...] nit: singular/plural mismatch between "these algorithms" and "one of". Section 7 How much discussion was there about "SHOULD be registered" (as opposed to "MUST be registered")? Section 8 side note: This is one of the shortest IANA considerations sections I've seen thta creates registries (in that it doesn't lay out a registration template), but IANA seems to be saying they have what information they need, so who am I to complain... Section 9 I agree with Roman that there should be discussion of the caching/data retention strategy for message previews. This existing text about denial-of-service attacks is probably fine, though I might consider rephrasing it along the lines of "this mechanism introduces a new way for clients to make requests that consume server resources. As is the case for all such mechanisms, it could be used as part of a denial-of-service attack on server resources, e.g., via excessive memory or CPU usage, or increased storage if preview results are cached on the server after generation. The additional attack surface presented by this specific mechanism is not believed to higher risk that other similar mechanisms in use already, since the individual resource consumption per message processed is likely to be modest. Nonetheless, servers MAY limit the resources consumed by preview generation." As I write the above, though, I wonder how this interacts with the previous text in Section 3.1 about how the "server MUST honor a client's algorithm priority decision". Does that mean that if some future (expensive) algorithm is specified, once a server implements that algorithm, any client can force its use and thus the expensive resource consumption on the server? It's not entirely clear to me how this MAY interacts with that MUST, in such a hypothetical scenario.
A few small comments on the IANA section: It would be good to also provide a name for each of the new registries (but I'm sure IANA will ask about this in their review). However, I'm also wondering why you don't specify straight away the use of the IETF Review policy as specified in RFC5226? Is there something different in there that does not work for you?
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"
On the positive side, I really like the "it allows consistent representation of previews across all clients" in section 1. Some questions though: 1) In the FUZZY algorithm (section 4.1), are we sure that removing all markup does significantly change the message? a <br/> can sometimes be perceived as a '.' == end of sentence; and removing them could vastly change the meaning of the message. 2) Section 3.2 "A server SHOULD strive to generate the same string for a give message for each request." which seems to contradict the "consistent representation" stated in section 1.
In relation to Benjamin's Discuss. I also think there are necessary to clarify if the Capability usage of PREVIEW is a single algorithm or allows listing all the algorithms server supports.