Re: [Extra] Barry Leiba's Discuss on draft-ietf-extra-imap-fetch-preview-03: (with DISCUSS and COMMENT)

["Chris Newman" <[e-mail]>]

On 8 Apr 2019, at 12:38, Chris Newman wrote:

> On 7 Apr 2019, at 20:25, Barry Leiba via Datatracker wrote:
>> Barry Leiba has entered the following ballot position for
>> draft-ietf-extra-imap-fetch-preview-03: Discuss
>>
>> 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/
>>
>>
>>
>> ----------------------------------------------------------------------
>> DISCUSS:
>> ----------------------------------------------------------------------
>>
>> — 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 5.1 —
>>
>> The way you have LAZY working isn’t really consistent with the IMAP 
>> protocol
>> model.  In that model, the client would not have to ask for the 
>> preview twice,
>> one with LAZY and one without.  Instead, with LAZY, the server would 
>> return
>> FETCH PREVIEW responses when it could — perhaps some in the first 
>> set of FETCH
>> responses, and some, where the PREVIEW part was missing before, in 
>> unsolicited
>> FETCH responses when the preview became available.  That way, the 
>> server has
>> the responsibility of setting off a separate task to generate the 
>> previews, and
>> to send them to the client when it has them (at which point it either 
>> saves the
>> for future FETCHes or doesn’t).
>
> That alternative design would take control away from the client, make 
> the client code more complex, and make the server code more complex. 
> For clients using deployed request/response-style IMAP APIs, your 
> proposal would violate the API model and thus be incredibly hard to 
> implement without replacing a lot of the API. On the server side, even 
> if I was willing to implement what you propose (and I'm not), I'd have 
> to refuse to issue the untagged response until all PREVIEWs were 
> generated to make sure request/response style client APIs could handle 
> that server behavior -- at which point it's far less efficient than 
> the proposed design where the client has control.

Sorry for the typo: s/refuse to issue the untagged response/refuse to 
issue the tagged response/

		- Chris

> I happen to consider the tagged/asynchronous part of the IMAP model to 
> be an elegant design that is a failure in practice when it encounters 
> the real world of implementations. The fact is the majority of clients 
> and client APIs just get the model wrong -- they're so used to 
> request/response that they try to shove a cache/update model into a 
> request/response model with predictably bad results. In our 8.0 server 
> release, we tried to push the envelope just a little bit by generating 
> untagged EXISTS responses between commands (a behavior that's 
> explicitly encouraged by the IMAP base spec) and that broke 
> interoperability with more than one IMAP client in practice so we had 
> to add an option to suppress that behavior to maintain 
> interoperability with those (admittedly buggy) clients.
>
>> As it’s written here, the client has to open a separate IMAP 
>> session with the
>> server and ask a second time for the previews it’s missing — a 
>> separate session
>> to avoid blocking other action on the main session.  And if the 
>> server has spun
>> off a task to preemptively generate them because the client asked 
>> once (a good
>> practice, given the description here) it has to retain them for some 
>> indefinite
>> period waiting for the client to ask again.
>
> As it turns out, that's exactly the implementation model followed by 
> clients that generate previews manually without this extension. So 
> with the new preview extension those clients just tweak the first 
> command to add LAZY PREVIEW fetch so they possibly get early previews 
> (improving the UI for a very small change). Then the client 
> implementer can add an alternate code path to use the non-LAZY preview 
> fetch variant instead of computing manually or the client implementer 
> could ignore the non-LAZY preview feature and have fewer codepaths to 
> test -- their choice (and the better choice might depend on whether 
> the client is mobile or desktop -- something the server doesn't know).
>
> Since our server implemented PREVIEW before LAZY was added, I find the 
> proposed LAZY an inconvenient but defensible addition to the protocol 
> because it allows the client this additional implementation 
> flexibility. But if the LAZY extension didn't give the client 
> flexibility, then I'd consider the feature objectionable -- it would 
> be significant unnecessary complexity without a defensible 
> implementation benefit.
>
>> Why was this not done with the first mechanism?
>
> I can't speak to why the author wrote it that way, but as a server 
> implementor, I find the way LAZY is presently specified to be 
> perfectly acceptable and it has a straightforward implementation in my 
> server. However, the proposed alternate design would be a royal PITA 
> to implement on my server.
>
> 		- Chris
>
>> — 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.
>>
>>
>> ----------------------------------------------------------------------
>> COMMENT:
>> ----------------------------------------------------------------------
>>
>> — 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.
>>
>>
>> _______________________________________________
>> Extra mailing list
>> [e-mail]
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.ietf.org_mailman_listinfo_extra&d=DwIGaQ&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=K_BObr5Kfkr3rxt1oBPF9KFiEU3xl9LcD2OOJG3TXfI&m=MX001Ss_AeNyXvo6ikSivMny92vtZHwv1o5u46suxv0&s=N-3gUpkcA9xNcWGc9Rom-LAlShVKARcwVlk-0CUMjHc&e=
>
> _______________________________________________
> Extra mailing list
> [e-mail]
> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.ietf.org_mailman_listinfo_extra&d=DwIGaQ&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=K_BObr5Kfkr3rxt1oBPF9KFiEU3xl9LcD2OOJG3TXfI&m=q6dTBfAoGFIPzPPL0c5W5HJEhd3f5a3XZxbznsvpXIs&s=fZWNDIoHV89pwoQ6inIBrCdhzPnNlCI2MCUWe-KON0o&e=