LIST responses and \HasChildren, \HasNoChildren flags

[ellie timoney]

> I think the answer is "yes, we'd like to always return it, and
> correctly".

Cool -- no exception for list-extended then, just always include
accurate CHILDREN flags?  I'm happy with that.

> LSUB is special, because it's whether there are SUBSCRIBED children,
> rather than mailbox children.

That's a good description of subscribed_cb()'s current behaviour, but I
can't see anywhere it's specified like this?

Like, the \HasChildren and \HasNoChildren flags are pretty specifically
introduced by the RETURN (CHILDREN) list-extended syntax.  And while the
list-extended RFC is fairly ambiguous about whether it's meant to apply
to LSUB too, or only LIST, it doesn't specify any special behaviour for
these attributes in the LSUB case. (If it did, that would resolve the
ambiguity.)  It is pretty clear that LIST (SUBSCRIBED) is a different
beast from LSUB, in that LIST (SUBSCRIBED) returns the actual mailbox
attributes (whereas some of LSUB's returned attributes are contextual --
though no mention of \HasChildren or \HasNoChildren here, either).

For what it's worth, we don't apply list-extended parsing to LSUB
commands, so it looks like we're taking rfc5258's use of the word "LIST"
literally, at least in that respect.

Given that we use subscribed_cb() for both LIST (SUBSCRIBED) and LSUB,
I'm inclined to think it should adhere to LIST (SUBSCRIBED)'s behaviour
wrt \HasChildren and \HasNoChildren (i.e. attributes are real mailbox
attributes) because that's the behaviour that's actually strictly
specified.  (Or it should check listargs->cmd and switch behaviours, but
that's getting messy.)

[For what it's worth, there's also the CHILDINFO (SUBSCRIBED) thing,
which /is/ specifically about subscribed children.  But it applies only
for LIST (SUBSCRIBED RECURSIVEMATCH), for which we have a substantially
different implementation, so it won't be affected by changes to LIST
(SUBSCRIBED) or LSUB.]

I think what I'm looking for is an understanding of how/why we got to
where we are.  If there is reasoning behind our current position, I want
to factor it in.  But if we got here by accident or inertia, there's no
particular reason to stay here.

Cheers,

ellie

On Tue, Sep 6, 2016, at 01:16 PM, Bron Gondwana wrote:
> I think the answer is "yes, we'd like to always return it, and
> correctly".
> 
> LSUB is special, because it's whether there are SUBSCRIBED children,
> rather than mailbox children.
> 
> Bron.
> 
> On Tue, 6 Sep 2016, at 12:46, ellie timoney wrote:
> > Our handling of the \HasChildren and \HasNoChildren flags in list
> > responses is currently inconsistent -- though not strictly in violation
> > of RFC (with the exception of a known bug in 2.5).  
> > 
> > [We pass our Cassandane List tests because they have these
> > inconsistencies baked into their expected results -- my fault, I wrote
> > the initial set based on "what Cyrus currently did at the time" and
> > they've grown from there.]
> > 
> > https://tools.ietf.org/html/rfc5258#section-4 :
> > >   The CHILDREN return option defines two new attributes that MUST be
> > >   returned within a LIST response: \HasChildren and \HasNoChildren.
> > >   Although these attributes MAY be returned in response to any LIST
> > >   command, the CHILDREN return option is provided to indicate that the
> > >   client particularly wants this information.  If the CHILDREN return
> > >   option is present, the server MUST return these attributes even if
> > >   their computation is expensive.
> > 
> > There's two points in here:
> > 
> >     1. If the client explicitly requests RETURN (CHILDREN), we MUST
> >     return \HasChildren or \HasNoChildren appropriately for each
> >     mailbox.
> > 
> >     2. We MAY return \HasChildren or \HasNoChildren in response to any
> >     other list command.
> > 
> > We handle (1) correctly, so we're in compliance.  The inconsistency is
> > in the handling of (2):
> > 
> >     a. For non-extended LIST/RLIST/XLIST, we act as if the client had
> >     requested CHILDREN, and return these attributes appropriately.
> > 
> >     b. For most extended list commands, if the client explicitly does
> >     not request the CHILDREN return option, we still return these
> >     attributes.  The RFC doesn't specifically state what to do in this
> >     case, which I think means (2) applies, and that this is therefore
> >     okay.  (The aforementioned known bug in 2.5 occurs in this case.)
> > 
> >     c. If the client is listing subscribed folders, using 'LIST
> >     (SUBSCRIBED) ...' or 'LSUB ...' (or their X/R variants), and does
> >     not explicitly request RETURN (CHILDREN), then we only return the
> >     \HasChildren attribute appropriately.  Subscribed mailboxes without
> >     children do not get the \HasNoChildren attribute in their response.
> > 
> >     d. For 'LIST (RECURSIVEMATCH) ...', I believe we return the child
> >     attributes only if RETURN (CHILDREN) was explicitly requested. But
> >     we don't have tests for this yet, so I'm not certain.
> > 
> > With the exception of the 2.5 bug, this inconsistency affects both 2.5
> > and master.
> > 
> > From the current implementation, it's ambiguous to me what the intended
> > behaviour is:
> > 
> >     * (a) shakes out as a combination of cmdloop() explicitly
> >     initialising listargs.ret with LIST_RET_CHILDREN for these commands
> >     (intent: always return child attributes?), and list_response()
> >     ensuring that if LIST_RET_CHILDREN is set, that these attributes are
> >     included (which is how we successfully handle (1)).
> >     * for (b), getlistargs() explicitly resets listargs.ret to 0 if it's
> >     an extended list command, overriding cmdloop()'s initialisation
> >     (intent: for extended list commands, only return what was
> >     requested?)... but then list_cb() sets the child attributes
> >     unconditionally anyway (intent: always return child attributes?)
> >    * (c) occurs because subscribed_cb() sets the \HasChildren flag in
> >    the same way list_cb() does, i.e. without regard to the return
> >    options (intent: always return child attributes?).  but it does not
> >    set \HasNoChildren at all, so the only way the latter gets set is if
> >    CHILDREN was requested, in which case list_response() fills it in
> >    * (d) occurs because recursivematch_cb() does not set the child
> >    attributes at all, and so it falls to list_response() to fill them in
> >    later, which, because of getlistargs()'s extended list command
> >    behaviour, it only does if CHILDREN was explicitly requested.
> > 
> > Generally it looks like the intent is to just always return the child
> > attributes, whether they were requested or not (in which case
> > subscribed_cb() has a bug whereby it omits the \HasNoChildren flag, and
> > recursivematch_cb() whereby it omits both).
> > 
> > But getlistargs()'s behaviour looks like we wanted to make an exception
> > for extended list commands, such that we'd only return what was asked
> > for (in which case list_cb() and subscribed_cb() both have a bug,
> > because they ignore this intent).
> > 
> > I think I understand these code paths well enough now to tidy this up
> > fairly easily (and knock off that 2.5 bug along the way), I just need
> > clarification on how we want it to work.
> > 
> > So I guess my questions here are:
> > 
> >     * do we intend to generally return \HasChildren or \HasNoChildren,
> >     regardless of whether they were requested? (I think "yes")
> > 
> >     * do we intend to make an exception to this for extended list
> >     commands, such that we only return what was asked for? (I'm not
> >     sure)
> > 
> > [There's interactions with \NoInferiors here too, but that's dealt with
> > separately, so can be set aside for now.]
> > 
> > Cheers,
> > 
> > ellie
> 
> 
> -- 
>   Bron Gondwana
>   brong at fastmail.fm