Re: Writing an custom imap command

[Timo Sirainen]

On 5. May 2021, at 17.28, Ryan Beethe  wrote:
> 
 You probably shold look some much more simple commands as
 insipiration. Try looking e.g. how cmd_id is implemented instead.
>>> 
>>> I implemented a simpler command as well, but because it was simple I
>>> didn't have any questions :)
>>> 
>>> Unfortunately I do need a long-running command more like IDLE as well.
>> 
>> What kind of "long running command" did you have in mind?
> 
> My email service offers a layer of encryption which is not transparent
> to IMAP, and where the keys are created and kept on each client device.
> Since IMAP synchronization is bidirectional, each client needs to
> encrypt uploaded messages to all known client devices.  Thus, clients
> need a way update their list of all known keys.
> 
> So the command is roughly:
> 
>tag XKEYSYNC [known_fingerprint ...]
>...
>DONE
> 
> And the responses are rougly:
> 
>* XKEYSYNC DELETED fingerprint
> 
>* XKEYSYNC CREATED public_key
> 
> The full source can be found at:
> 
> github.com/splintermail/splintermail-client/blob/dev/server/xkeysync.c 
> 

These look rather simple commands. Like Aki said, look at the simple commands 
for examples rather than the more complicated IDLE and APPEND commands which 
have various special cases.

But do you have to implement these command extensions? Looks like it would be 
much easier to just use the IMAP METADATA extension and store them in the 
server metadata.

Re: Writing an custom imap command

[Aki Tuomi]

> On 05/05/2021 18:28 Ryan Beethe  wrote:
> 
>  
> On Wed, May 05, 2021 at 10:53:30AM +0300, Aki Tuomi wrote:
> >
> > > On 04/05/2021 16:42 Ryan Beethe  wrote:
> > >
> > >
> > > On Mon, May 03, 2021 at 09:14:13AM +0300, Aki Tuomi wrote:
> > > >
> > > > > On 01/05/2021 18:32 Ryan Beethe  wrote:
> > > > >
> > > > > 1. Why does cmd-idle.c sometimes call client_command_free()?  But
> > > > > sometimes it doesn't?
> > > > >
> > > > > For example, cmd_idle_continue() frees it in some branches but not
> > > > > others.  That makes no sense to me; it seems like it should be 
> > > > > based
> > > > > on your entrypoint (mailbox notify callback vs input callback vs
> > > > > timeout callback), not based on which branch of logic within that
> > > > > entrypoint.
> > > > >
> > > > > 2. Why does cmd-idle.c ever call client_destroy()?  That seems like
> > > > > something that should be invoked only by the imap process, not by any
> > > > > command.
> > > > >
> > > > > It calls it in cmd-idle.c:idle_callback (which is a mailbox notify
> > > > > callback).  It invokes it after idle_sync_now() when it detects 
> > > > > that
> > > > > client->disconnected is set.  Maybe that happens in 
> > > > > imap_sync_init()
> > > > > or something?
> > > > >
> > > > > 3. Why does cmd-idle.c ever call client_disconnect()?  That also seems
> > > > > like the responsibility of the imap process, and not any command.
> > > > >
> > > > > idle_client_input_more() detects when i_stream_read returns -1,
> > > > > meaning that the client has *already disconnected*.  Then it calls
> > > > > client_disconnect().
> > > > >
> > > > > I think this is the crazy part... the istream is effectively 
> > > > > unique
> > > > > to the imap process, so it seems unreasonable that any command is
> > > > > responsible for cleaning it up; it should just always happen at 
> > > > > the
> > > > > imap process level before exiting, right?
> > > > >
> > > >
> > > > IDLE cmd can be sometimes delegated to a separate worker called 
> > > > imap-hibernate, in which case the connection is moved to another 
> > > > process. This explains about all your questions.
> > >
> > > Wait, but then why does APPEND also make each of these calls?  APPEND
> > > can't be hibernated, as far as I can tell?
> > >
> >
> > Because APPEND might need to read quite a lot of data from the client.
> 
> So then I am back at my original questions.  Maybe I can guess at some
> answers and you can tell me if I'm understanding correctly:
> 
> 1. Why does cmd-idle.c sometimes call client_command_free()? But
> sometimes it doesn't?
> 
> Earlier I said I though cmd_idle_continue() freed it in some
> branches but not others, but I think I was mistaken.  It looks
> like the only path where client_command_free is called is inside
> an io_add_istream callback.  That makes sense, and I can do the
> same thing with my command.
> 
> 2. Why does cmd-idle.c ever call client_destroy()?  That seems like
> something that should be invoked only by the imap process, not by
> any command.
> 
> This is only ever triggered by idle_callback, which is a
> mailbox_notify_changes callback, which I don't have to interact
> with, so maybe I can ignore this.
> 
> 3. Why does cmd-idle.c ever call client_disconnect()?  That also
> seems like the responsibility of the imap process, and not any
> command.
> 
> While I'm still not sure why the imap process is not responsible
> for calling this, it does seem like it only gets called when
> i_stream_read() returns -1, and I can probably immitate that
> without much risk.
> 
> But wait, why does cmd-idle.c call client_disconnect() when
> i_stream_read() returns -1, but cmd-append.c calls
> client_command_free() and client_destroy() but not
> client_disconnect()?
> 
> > > > You probably shold look some much more simple commands as
> > > > insipiration. Try looking e.g. how cmd_id is implemented instead.
> > >
> > > I implemented a simpler command as well, but because it was simple I
> > > didn't have any questions :)
> > >
> > > Unfortunately I do need a long-running command more like IDLE as well.
> >
> > What kind of "long running command" did you have in mind?
> 
> My email service offers a layer of encryption which is not transparent
> to IMAP, and where the keys are created and kept on each client device.
> Since IMAP synchronization is bidirectional, each client needs to
> encrypt uploaded messages to all known client devices.  Thus, clients
> need a way update their list of all known keys.
> 
> So the command is roughly:
> 
> tag XKEYSYNC [known_fingerprint ...]
> ...
> DONE
> 
> And the responses are rougly:
> 
> * XKEYSYNC DELETED fingerprint
> 
> * XKEYSYNC CREATED public_key
> 
> The full source can be found at:
> 
> 

Re: Writing an custom imap command

[Ryan Beethe]

On Wed, May 05, 2021 at 10:53:30AM +0300, Aki Tuomi wrote:
>
> > On 04/05/2021 16:42 Ryan Beethe  wrote:
> >
> >
> > On Mon, May 03, 2021 at 09:14:13AM +0300, Aki Tuomi wrote:
> > >
> > > > On 01/05/2021 18:32 Ryan Beethe  wrote:
> > > >
> > > > 1. Why does cmd-idle.c sometimes call client_command_free()?  But
> > > > sometimes it doesn't?
> > > >
> > > > For example, cmd_idle_continue() frees it in some branches but not
> > > > others.  That makes no sense to me; it seems like it should be based
> > > > on your entrypoint (mailbox notify callback vs input callback vs
> > > > timeout callback), not based on which branch of logic within that
> > > > entrypoint.
> > > >
> > > > 2. Why does cmd-idle.c ever call client_destroy()?  That seems like
> > > > something that should be invoked only by the imap process, not by any
> > > > command.
> > > >
> > > > It calls it in cmd-idle.c:idle_callback (which is a mailbox notify
> > > > callback).  It invokes it after idle_sync_now() when it detects that
> > > > client->disconnected is set.  Maybe that happens in imap_sync_init()
> > > > or something?
> > > >
> > > > 3. Why does cmd-idle.c ever call client_disconnect()?  That also seems
> > > > like the responsibility of the imap process, and not any command.
> > > >
> > > > idle_client_input_more() detects when i_stream_read returns -1,
> > > > meaning that the client has *already disconnected*.  Then it calls
> > > > client_disconnect().
> > > >
> > > > I think this is the crazy part... the istream is effectively unique
> > > > to the imap process, so it seems unreasonable that any command is
> > > > responsible for cleaning it up; it should just always happen at the
> > > > imap process level before exiting, right?
> > > >
> > >
> > > IDLE cmd can be sometimes delegated to a separate worker called 
> > > imap-hibernate, in which case the connection is moved to another process. 
> > > This explains about all your questions.
> >
> > Wait, but then why does APPEND also make each of these calls?  APPEND
> > can't be hibernated, as far as I can tell?
> >
>
> Because APPEND might need to read quite a lot of data from the client.

So then I am back at my original questions.  Maybe I can guess at some
answers and you can tell me if I'm understanding correctly:

1. Why does cmd-idle.c sometimes call client_command_free()? But
sometimes it doesn't?

Earlier I said I though cmd_idle_continue() freed it in some
branches but not others, but I think I was mistaken.  It looks
like the only path where client_command_free is called is inside
an io_add_istream callback.  That makes sense, and I can do the
same thing with my command.

2. Why does cmd-idle.c ever call client_destroy()?  That seems like
something that should be invoked only by the imap process, not by
any command.

This is only ever triggered by idle_callback, which is a
mailbox_notify_changes callback, which I don't have to interact
with, so maybe I can ignore this.

3. Why does cmd-idle.c ever call client_disconnect()?  That also
seems like the responsibility of the imap process, and not any
command.

While I'm still not sure why the imap process is not responsible
for calling this, it does seem like it only gets called when
i_stream_read() returns -1, and I can probably immitate that
without much risk.

But wait, why does cmd-idle.c call client_disconnect() when
i_stream_read() returns -1, but cmd-append.c calls
client_command_free() and client_destroy() but not
client_disconnect()?

> > > You probably shold look some much more simple commands as
> > > insipiration. Try looking e.g. how cmd_id is implemented instead.
> >
> > I implemented a simpler command as well, but because it was simple I
> > didn't have any questions :)
> >
> > Unfortunately I do need a long-running command more like IDLE as well.
>
> What kind of "long running command" did you have in mind?

My email service offers a layer of encryption which is not transparent
to IMAP, and where the keys are created and kept on each client device.
Since IMAP synchronization is bidirectional, each client needs to
encrypt uploaded messages to all known client devices.  Thus, clients
need a way update their list of all known keys.

So the command is roughly:

tag XKEYSYNC [known_fingerprint ...]
...
DONE

And the responses are rougly:

* XKEYSYNC DELETED fingerprint

* XKEYSYNC CREATED public_key

The full source can be found at:

github.com/splintermail/splintermail-client/blob/dev/server/xkeysync.c

Ryan

Re: Writing an custom imap command

[Aki Tuomi]

> On 04/05/2021 16:42 Ryan Beethe  wrote:
> 
>  
> On Mon, May 03, 2021 at 09:14:13AM +0300, Aki Tuomi wrote:
> >
> > > On 01/05/2021 18:32 Ryan Beethe  wrote:
> > >
> > > 1. Why does cmd-idle.c sometimes call client_command_free()?  But
> > > sometimes it doesn't?
> > >
> > > For example, cmd_idle_continue() frees it in some branches but not
> > > others.  That makes no sense to me; it seems like it should be based
> > > on your entrypoint (mailbox notify callback vs input callback vs
> > > timeout callback), not based on which branch of logic within that
> > > entrypoint.
> > >
> > > 2. Why does cmd-idle.c ever call client_destroy()?  That seems like
> > > something that should be invoked only by the imap process, not by any
> > > command.
> > >
> > > It calls it in cmd-idle.c:idle_callback (which is a mailbox notify
> > > callback).  It invokes it after idle_sync_now() when it detects that
> > > client->disconnected is set.  Maybe that happens in imap_sync_init()
> > > or something?
> > >
> > > 3. Why does cmd-idle.c ever call client_disconnect()?  That also seems
> > > like the responsibility of the imap process, and not any command.
> > >
> > > idle_client_input_more() detects when i_stream_read returns -1,
> > > meaning that the client has *already disconnected*.  Then it calls
> > > client_disconnect().
> > >
> > > I think this is the crazy part... the istream is effectively unique
> > > to the imap process, so it seems unreasonable that any command is
> > > responsible for cleaning it up; it should just always happen at the
> > > imap process level before exiting, right?
> > >
> >
> > IDLE cmd can be sometimes delegated to a separate worker called 
> > imap-hibernate, in which case the connection is moved to another process. 
> > This explains about all your questions.
> 
> Wait, but then why does APPEND also make each of these calls?  APPEND
> can't be hibernated, as far as I can tell?
> 

Because APPEND might need to read quite a lot of data from the client.

> > > 4. What does client_continue_pending_input() actually do, and under
> > > what conditions does it need to be called?
> >
> > It means that you did not consume all the input there was.
> 
> Ok, reading over the code now with this understanding makes a lot of
> sense, thank you.
> 
> > You probably shold look some much more simple commands as insipiration. Try 
> > looking e.g. how cmd_id is implemented instead.
> 
> I implemented a simpler command as well, but because it was simple I
> didn't have any questions :)
> 
> Unfortunately I do need a long-running command more like IDLE as well.
> 
> Ryan

What kind of "long running command" did you have in mind?

Aki

Re: Writing an custom imap command

[Ryan Beethe]

On Mon, May 03, 2021 at 09:14:13AM +0300, Aki Tuomi wrote:
>
> > On 01/05/2021 18:32 Ryan Beethe  wrote:
> >
> > 1. Why does cmd-idle.c sometimes call client_command_free()?  But
> > sometimes it doesn't?
> >
> > For example, cmd_idle_continue() frees it in some branches but not
> > others.  That makes no sense to me; it seems like it should be based
> > on your entrypoint (mailbox notify callback vs input callback vs
> > timeout callback), not based on which branch of logic within that
> > entrypoint.
> >
> > 2. Why does cmd-idle.c ever call client_destroy()?  That seems like
> > something that should be invoked only by the imap process, not by any
> > command.
> >
> > It calls it in cmd-idle.c:idle_callback (which is a mailbox notify
> > callback).  It invokes it after idle_sync_now() when it detects that
> > client->disconnected is set.  Maybe that happens in imap_sync_init()
> > or something?
> >
> > 3. Why does cmd-idle.c ever call client_disconnect()?  That also seems
> > like the responsibility of the imap process, and not any command.
> >
> > idle_client_input_more() detects when i_stream_read returns -1,
> > meaning that the client has *already disconnected*.  Then it calls
> > client_disconnect().
> >
> > I think this is the crazy part... the istream is effectively unique
> > to the imap process, so it seems unreasonable that any command is
> > responsible for cleaning it up; it should just always happen at the
> > imap process level before exiting, right?
> >
>
> IDLE cmd can be sometimes delegated to a separate worker called 
> imap-hibernate, in which case the connection is moved to another process. 
> This explains about all your questions.

Wait, but then why does APPEND also make each of these calls?  APPEND
can't be hibernated, as far as I can tell?

> > 4. What does client_continue_pending_input() actually do, and under
> > what conditions does it need to be called?
>
> It means that you did not consume all the input there was.

Ok, reading over the code now with this understanding makes a lot of
sense, thank you.

> You probably shold look some much more simple commands as insipiration. Try 
> looking e.g. how cmd_id is implemented instead.

I implemented a simpler command as well, but because it was simple I
didn't have any questions :)

Unfortunately I do need a long-running command more like IDLE as well.

Ryan

Re: Writing an custom imap command

[Aki Tuomi]

> On 01/05/2021 18:32 Ryan Beethe  wrote:
> 
>  
> I'm interested in writing a custom imap command that behaves a bit like
> IDLE but synchronizes some state that is specific to my mail client /
> mail server.
> 
> I found that stateless commands were trivial to understand, and I really
> like the plugin pattern for registering custom commands.
> 
> But I have a few questions on how to write a long-running command that I
> was not able to answer by reading through the code.  This mailing list
> seems like the best place to ask them.
> 
> For reference, the source code for my custom command is here:
> 
> 
> https://github.com/Splintermail/splintermail-client/blob/dev/server/xkeysync.c
> 
> Thanks,
> 
> Ryan
> 
> --
> 
> 1. Why does cmd-idle.c sometimes call client_command_free()?  But
> sometimes it doesn't?
> 
> For example, cmd_idle_continue() frees it in some branches but not
> others.  That makes no sense to me; it seems like it should be based
> on your entrypoint (mailbox notify callback vs input callback vs
> timeout callback), not based on which branch of logic within that
> entrypoint.
>
> 2. Why does cmd-idle.c ever call client_destroy()?  That seems like
> something that should be invoked only by the imap process, not by any
> command.
> 
> It calls it in cmd-idle.c:idle_callback (which is a mailbox notify
> callback).  It invokes it after idle_sync_now() when it detects that
> client->disconnected is set.  Maybe that happens in imap_sync_init()
> or something?
> 
> 3. Why does cmd-idle.c ever call client_disconnect()?  That also seems
> like the responsibility of the imap process, and not any command.
> 
> idle_client_input_more() detects when i_stream_read returns -1,
> meaning that the client has *already disconnected*.  Then it calls
> client_disconnect().
> 
> I think this is the crazy part... the istream is effectively unique
> to the imap process, so it seems unreasonable that any command is
> responsible for cleaning it up; it should just always happen at the
> imap process level before exiting, right?
>

IDLE cmd can be sometimes delegated to a separate worker called imap-hibernate, 
in which case the connection is moved to another process. This explains about 
all your questions.
 
> 4. What does client_continue_pending_input() actually do, and under what
> conditions does it need to be called?
> 
> There is one place that you *can't* call it; there is a section in
> imap-client.c:client_handle_input() that calls
> imap-client.c:client_handle_next_command(), which calls the
> cmd->func().  That makes sense; that's the input trigger for the
> command plugin, so maybe you only have to trigger it when you are
> receiving input that doesn't fit into the normal command args
> behavior.
> 
> It has a comment that says "this function is called at the end of
> I/O callbacks (and only there)".  It _is_ called by client_input()
> and by client_output(), but also by:
>  - cmd-idle.c:idle_client_input (io_add_istream callback)
>  - cmd-append.c:client_input_append (io_add_istream callback)
>  - imap-search.c:cmd_search_more_callback (timeout_add callback)
> The first two cases seem to be the only io_add_istream() commands
> that even exist, so that explains them.  I can't explain the
> imap-search.c case at all.
> 
> Reading through it I have really no idea what
> client_continue_pending_input is really doing.
> 
> My command has a DONE mechanic just like IDLE so I'm pretty sure I
> need to invoke this function, I'm just concerned I'm going to do it
> wrong if I don't understand the mechanics of it.

It means that you did not consume all the input there was.

You probably shold look some much more simple commands as insipiration. Try 
looking e.g. how cmd_id is implemented instead. 

Aki

Writing an custom imap command

[Ryan Beethe]

I'm interested in writing a custom imap command that behaves a bit like
IDLE but synchronizes some state that is specific to my mail client /
mail server.

I found that stateless commands were trivial to understand, and I really
like the plugin pattern for registering custom commands.

But I have a few questions on how to write a long-running command that I
was not able to answer by reading through the code.  This mailing list
seems like the best place to ask them.

For reference, the source code for my custom command is here:


https://github.com/Splintermail/splintermail-client/blob/dev/server/xkeysync.c

Thanks,

Ryan

--

1. Why does cmd-idle.c sometimes call client_command_free()?  But
sometimes it doesn't?

For example, cmd_idle_continue() frees it in some branches but not
others.  That makes no sense to me; it seems like it should be based
on your entrypoint (mailbox notify callback vs input callback vs
timeout callback), not based on which branch of logic within that
entrypoint.

2. Why does cmd-idle.c ever call client_destroy()?  That seems like
something that should be invoked only by the imap process, not by any
command.

It calls it in cmd-idle.c:idle_callback (which is a mailbox notify
callback).  It invokes it after idle_sync_now() when it detects that
client->disconnected is set.  Maybe that happens in imap_sync_init()
or something?

3. Why does cmd-idle.c ever call client_disconnect()?  That also seems
like the responsibility of the imap process, and not any command.

idle_client_input_more() detects when i_stream_read returns -1,
meaning that the client has *already disconnected*.  Then it calls
client_disconnect().

I think this is the crazy part... the istream is effectively unique
to the imap process, so it seems unreasonable that any command is
responsible for cleaning it up; it should just always happen at the
imap process level before exiting, right?

4. What does client_continue_pending_input() actually do, and under what
conditions does it need to be called?

There is one place that you *can't* call it; there is a section in
imap-client.c:client_handle_input() that calls
imap-client.c:client_handle_next_command(), which calls the
cmd->func().  That makes sense; that's the input trigger for the
command plugin, so maybe you only have to trigger it when you are
receiving input that doesn't fit into the normal command args
behavior.

It has a comment that says "this function is called at the end of
I/O callbacks (and only there)".  It _is_ called by client_input()
and by client_output(), but also by:
 - cmd-idle.c:idle_client_input (io_add_istream callback)
 - cmd-append.c:client_input_append (io_add_istream callback)
 - imap-search.c:cmd_search_more_callback (timeout_add callback)
The first two cases seem to be the only io_add_istream() commands
that even exist, so that explains them.  I can't explain the
imap-search.c case at all.

Reading through it I have really no idea what
client_continue_pending_input is really doing.

My command has a DONE mechanic just like IDLE so I'm pretty sure I
need to invoke this function, I'm just concerned I'm going to do it
wrong if I don't understand the mechanics of it.