ietf
[Top] [All Lists]

Gen-ART Last Call Review of draft-melnikov-imap-search-res-06.txt

2007-12-31 15:12:47
Hi, Alexey,

I have been selected as the General Area Review Team (Gen-ART)
reviewer for this draft (for background on Gen-ART, please see
http://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html).

Please resolve these comments along with any other Last Call comments
you may receive.

Thanks, and Happy New Year,

Spencer

Document: draft-melnikov-imap-search-res-06.txt
Reviewer: Spencer Dawkins
Review Date:  2007-12-31
IETF LC End Date: 2008-01-12
IESG Telechat date:

Summary: This document is on the right path for publication as a proposed standard, but has some open issues, described in the review.

Comments:

I used three prefixes - "Spencer (protocol)", "Spencer (readability)", and
"Spencer (nit)" in my review. The nits are included as a convenience for
anyone editing the document.

The "protocol" comments are most significant, and there are only a few of
these.

Abstract

  Many IMAP clients use the result of a SEARCH command as the input to
  perform another operation, for example fetching the found messages,
  deleting them or copying them to another mailbox.

  This can be achieved using standard IMAP operations described in RFC
  3501, however this would be suboptimal: the server will send the list
  of found messages to the client, after that the client will have to
  parse the list, reformat it and send it back to the server.  The
  client can't pipeline the SEARCH command with the subsequent command.

Spencer (readability) - in this paragraph, there are three ways that this extension improves normal operation - for bandwidth (because the list isn't sent in both directions when it doesn't have to be), for processing (since the list isn't parsed and reformatted when it doesn't have to be), and for serialization (since the two commands can now be pipelined). In the body of the document, a fourth improvement is also mentioned ("server optimization"), but isn't described at all. The two lists should be the same, and I wouldn't mind seeing some description of "server optimization" in the body of the document.

  This document proposes an IMAP extension that allows a client to tell
  a server to use the result of a SEARCH (or UID SEARCH) command as an
  input to any subsequent command.

2.   Introduction and Overview

Spencer (readability) - This section combines introduction/overview (not
normative) and protocol operation (normative). I'd suggest separating the
two. I'd also suggest using some of the abstract text as an actual
introduction - this section assumes that you've read the abstract, I think,
and not everyone will automatically do that.

  The SEARCH result reference extension defines a new SEARCH result
  option [IMAPABNF] "SAVE" that tells the server to remember the result
  of the SEARCH or UID SEARCH command (as well as any command based on
  SEARCH, e.g. SORT and THREAD [SORT]) and store it in an internal
  variable that we will reference as the "search result variable". The
  client can use the "$" marker to reference the content of this
  internal variable. The "$" marker can be used instead of message (or
  UID) sequence in order to indicate that the server should substitute

Spencer (nit) - "message" looks like a noun until you get past "(or UID)"
and discover it's an adjective. Suggest s/message (or UID) sequence/message
sequence or UID sequence/.

  it with the list of messages from the search result variable.  Thus
  the client can use the result of the latest remembered SEARCH command
  as a parameter to another command.  The search result marker has
  several advantages:
   * it avoids wasted bandwidth and associated delay;
   * it allows the client to pipeline a SEARCH [IMAP4] command with a
     subsequent FETCH/STORE/COPY/SEARCH [IMAP4] or UID EXPUNGE
  [UIDPLUS] command;
   * the client doesn't need to spend time reformatting the
     result of a SEARCH command into a message set used in
     the subsequent command;
   * it allows the server to perform optimizations.

Spencer (readability) - in this list, there are FOUR advantages, not three as above... can you give any information about what optimizations a server can perform because this extension is being used?

  In absence of any other SEARCH result option, the SAVE result option
  also suppresses any SEARCH response that would have been otherwise
  returned by the SEARCH command.

Spencer (readability) - somewhere around here, the actual protocol
specification section starts... :-)

  Upon successful completion of a SELECT or an EXAMINE command (after
  the tagged OK response), the current search result variable is reset
  to the empty sequence.

Spencer (protocol) - I'd like to better understand why this design choice
was made. This statement seems to conflict with text in the next paragraph, so I'm not exactly sure what's going on here.

  A successful SEARCH command with the SAVE result option sets the
  value of the search result variable to the list of messages found in
  the SEARCH command. For example, if no messages were found, the
  search result variable will contain the empty list.  A SEARCH command
  that caused the server to return BAD tagged response, a SEARCH
  command with no SAVE result option that caused the server to return
  NO tagged response, or a successful SEARCH command with no SAVE
  result option MUST NOT change the search result variable.  A SEARCH

Spencer (protocol) - I'm now confused. The previous paragraph says "reset to
the empty sequence" upon successful completion of a SELECT command, but this
is saying that "a successful SEARCH command with no SAVE result option MUST
NOT change the search result variable" - what am I missing? (I'm actually OK
with the "no messages" and "BAD tagged response" exceptions, but the successful case seems to contradict the previous paragraph)

  command with the SAVE result option that caused the server to return
  NO tagged response sets the value of the search result variable to
  the empty sequence.

  When a message listed in the search result variable is EXPUNGEd, it
  is automatically removed from the list.  Implementors are reminded
  that if the server stores the list as a list of message numbers, it
  MUST automatically adjust them when notifying the client about

Spencer (readability) - is the meaning of "automatically adjust them"
obvious to everyone but me? Do you just remove the message number from the
list, or renumber the list, or ... ?

  expunged messages.

  Note that even if the "$" marker contains the empty list of messages,
  it must be treated by all commands accepting message sets as
  parameters, as a valid, but non matching list of messages. For

Spencer (readability) - this is correct as written, but hard to parse.
Perhaps "treated as a valid but non matching list of messages, by all
commands that accept message sets as parameters"?

  example, the "FETCH $" command would return tagged OK response and no
  FETCH responses.  See also the Example # 5 below.

2.1.   Examples

  The client can also pipeline the two commands:

  Example 2:
              C: A282 SEARCH RETURN (SAVE) FLAGGED SINCE 1-Feb-1994
                 NOT FROM "Smith"
              C: A283 FETCH $ (UID INTERNALDATE FLAGS RFC822.HEADER)
              S: A282 OK SEARCH completed
              S: * 2 FETCH (UID 14 ...
              S: * 84 FETCH (UID 100 ...
              S: * 882 FETCH (UID 1115 ...
              S: A283 OK completed

Spencer (protocol) - I'm not seeing any description of how pipelined
commands deal with first-command failure - is this relevant? (does the
server execute the second command if the first command generates a
BAD-tagged response? etc)

  2) The following example demonstrates that the result of one SEARCH
     command can be used to subset the result of another SEARCH
     command:

  Example 3:
              C: A300 SEARCH RETURN (SAVE) SINCE 1-Jan-2004
                  NOT FROM "Smith"
              S: A300 OK SEARCH completed
              C: A301 UID SEARCH UID $ SMALLER 4096
              S: * SEARCH 17 900 901
              S: A301 OK completed

  Note that the second command in Example 3 can be replaced with:
              C: A301 UID SEARCH $ SMALLER 4096
  and the result of the command would be the same.

Spencer (readability) - I didn't quite follow this. I think what confused me
is the introductory text - isn't this example demonstrating "that the result
of one SEARCH command can be used as input to a second SEARCH command"?
"Subset" as written seemed to say that the first result was taking an action
("subset"), but it's just input to the second SEARCH...

2.2.   Multiple Commands in Progress

Spencer (readability) - could you insert a sentence that introduces Example
7 and explains what you illustrate in this example? (Is it only what the
//comment says?) (same for Example 8)

       Example 7:
                   C: G282 SEARCH RETURN (SAVE) KEYWORD $Junk
                   C: G283 SEARCH RETURN (ALL) SINCE 28-Oct-2006
                       FROM "Eric"
                   //The server can execute the two SEARCH commands
                   //in any order, as the don't have any dependency.

Spencer (nit) - s/the/they/
                   //Note that the second command is making use of
                   //the [ESEARCH] extension.
                   S: * ESEARCH (TAG "G283") ALL 3:15,27,29:103
                   S: G283 OK SEARCH completed
                   S: G282 OK SEARCH completed

       Example 8:
                   C: H282 SEARCH RETURN (SAVE) KEYWORD $Junk
                   C: H283 SEARCH RETURN (SAVE) SINCE 28-Oct-2006
                       FROM "Eric"
                   //The result of the second SEARCH overrides the
                   //result of the first.
                   S: H283 OK SEARCH completed
                   S: H282 OK SEARCH completed


2.3.   Interaction with ESEARCH extension

       When the SAVE result option is combined with the MIN or MAX
       [ESEARCH] result option, and none of the other ESEARCH result
       options are present, the corresponding MIN/MAX is returned (if
       the search result is not empty) but the "$" marker would contain
       a single message as returned in the MIN/MAX return item. If the
       SAVE result option is combined with both MIN and MAX result
       options, and none of the other ESEARCH result options are
       present, the "$" marker would contain one or two messages as
       returned in the MIN/MAX return items.  If the SAVE result option
       is combined with ALL and/or COUNT result option, the "$" marker
       would always contain all messages found by the SEARCH or UID
       SEARCH command.  (Note that the last rule might affect ESEARCH
       implementations that optimize how COUNT result is constructed.)

Spencer (readability) - is it obvious to everyone except me *how* the last
rule might affect these implementations?

Spencer (nit) - the previous paragraph is pretty dense. It might be more
readable as a bulleted list (one bullet per case).

  3.   Formal Syntax

4.   Security Considerations

  This extension requires the server to keep additional state, that may
  be used to simplify Deny of Service attacks. In order to minimize
  damage from such attacks server implementations MAY limit the number
  of saved searches they allow across all connections at any given time
  and return the tagged NO response to a SEARCH RETURN (SAVE) command
  when this limit is exceeded.

Spencer (readability) - I'm guessing there is no way for a client to
discover this is the reason for the tagged NO response? but it might be nice
to say so explicitly. Just curious - is this what an IMAP server would do
today, when it detects a DoS attack (by an attacker who's just working harder)? If so, it might be nice to point that out, too.

5.  IANA Considerations

  This document defines the "X-DRAFT-I05-SEARCHRES" <<Fix upon
  publication>> IMAP capability.  IANA is requested to add it to the
  IMAP4 Capabilities Registry, which is currently located at:

     http://www.iana.org/assignments/imap4-capabilities

Spencer (nit) - IANA has suggested "search-res" as the permanent name for this capability. You might add it here, just for completeness.


_______________________________________________
Ietf mailing list
Ietf(_at_)ietf(_dot_)org
https://www1.ietf.org/mailman/listinfo/ietf

<Prev in Thread] Current Thread [Next in Thread>
  • Gen-ART Last Call Review of draft-melnikov-imap-search-res-06.txt, Spencer Dawkins <=