Tony Finch <dot@xxxxxxxx> writes:
> Some MUAs delete their "drafts" folder when it is empty, so
> git imap-send should be able to create it if necessary.
>
> This change checks that the folder exists immediately after
> login and tries to create it if it is missing.
>
> There was some vestigial code to handle a [TRYCREATE] response
> from the server when an APPEND target is missing. However this
> code never ran (the create and trycreate flags were never set)
> and when I tried to make it run I found that the code had already
> thrown away the contents of the message it was trying to append.
>
> Signed-off-by: Tony Finch <dot@xxxxxxxx>
> ---
The basic idea looks good, but I have doubts on one point.
> diff --git a/imap-send.c b/imap-send.c
> index 524fbab..5e4a24e 100644
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -1156,6 +1133,25 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc)
> credential_approve(&cred);
> credential_clear(&cred);
>
> + /* check the target mailbox exists */
> + ctx->name = folder;
> + switch (imap_exec(ctx, NULL, "EXAMINE \"%s\"", ctx->name)) {
> + case RESP_OK:
> + /* ok */
> + break;
> + case RESP_BAD:
> + fprintf(stderr, "IMAP error: could not check mailbox\n");
> + goto bail;
> + case RESP_NO:
> + if (imap_exec(ctx, NULL, "CREATE \"%s\"", ctx->name) == RESP_OK) {
> + imap_info("Created missing mailbox\n");
> + } else {
> + fprintf(stderr, "IMAP error: could not create missing mailbox\n");
> + goto bail;
> + }
> + break;
> + }
At any and all the existing places that "goto bail" in the function,
we know we failed to authenticate. I think they are all sensible
places to call credential_reject().
On the other hand, at this point before you try to "check the target
mailbox exists", we have authenticated sucessfully, we know the
credential used was good, and called credential_approve() to mark it
as such. I do agree that you would want to signal an error to the
caller upon these two failures, but I do not think you want to "goto
bail" and reject the credential. The error you observed in the new
codepath is caused by something else, not authentication failure,
and in such a case you do not want to cause the credential helper to
evict the user/pass pair from the keyring, no?
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html