lmtpd (mailbox) delivery notification -- apparent bug

[roburban]

Platform: OpenBSD 7.2-release
Cyrus-imapd version: 3.4.4
OpenBSD package: cyrus-imapd-3.4.4

As mentioned recently on the cyrus-info list, we've been seeing the following response from the v3.4.4 Cyrus imapd for successful delivery to a mailbox:

250 2.1.5 Undefined error: 0 SESSIONID=<imapd-1672619404-57025-2-15431856533157375633>

I started digging in the sources and found that between 3.0.8 and 3.4.4 the following code was added to the top of lmtpengine.c:send_lmtp_error():

     int code;

     if (resp) {
         int i;

         for (i = 0; i < strarray_size(resp); i++) {
             prot_puts(pout, strarray_nth(resp, i));
         }
         return;
     }

Presumably the "prot_puts(pout, strarray_nth(resp, i));" would emit the correct success notification 250 2.1.5 Ok SESSIONID=<sess-id> (although how it would do this is not clear to me) but this depends on "resp" being non-null, however, in lmtpd.c:deliver(), near the bottom of the loop which iterates through all the recipients, we see this line:

msg_setrcpt_status(msgdata, n, r, NULL);

Which causes msg_setrcpt_status() to always assign:

m->rcpt[rcpt_num]->resp = NULL;

Because resp is NULL, the "if (resp) {" block in send_lmtp_error() is not invoked, and control continues at the switch statement, which tests (r). As r contains zero (which indicates no error, I presume), the following matches:

     case 0:
         code = LMTP_OK;
         break;

which causes this code:

prot_printf(pout, error_message(code), error_message(r), session_id());

to emit the famous 250 2.1.5 Undefined error: 0 SESSIONID=... line.

This would seem to be a bug.

The 3.0.8 send_lmtp_error() emits the success notification line when r == 0. There seem to be some wires crossed about how to propagate success/failure status.

The 3.6.0 code seems to be the same as the 3.4.4 code.

ADDENDUM

Clearly the v3.0.8 version of send_lmtp_error() is responsible for sending the success notification upon success, which would make the name "send_lmtp_error" somewhat misleading. Perhaps during some code renovation/refactoring this aspect was overlooked?

[elliefm]

This smells familiar. I think BSD is one of the platforms which doesn't have an errno string for value 0, and thus produces "Undefined error: 0" for strerror(0). If I remember right, on Linux, the error string for errno 0 is something like "Success", and so on that platform or any like it this response would be 250 2.1.5 Success SESSIONID=..., or something like that, which I'm sure you would agree is fine.

I thought we'd fixed this before, but maybe we only discussed fixing it and didn't get to a clear solution, or maybe we got to a platform-specific solution (MacOS, perhaps?) that hasn't adequately generalised to OpenBSD...

Ahh, here's the previous discussion: #3035 -- note that this issue was about a crash, which was fixed, and thus the issue is closed. But the "not all platforms provide a sensible strerror(0)" detail was discussed along the way, and there's an open PR to work around that: #3826. We discussed a few approaches in the PR comments, and I thought one had been agreed upon, but the submitter has not returned since, so it languishes.

we've been seeing the following response from the v3.4.4 Cyrus imapd for successful delivery to a mailbox

In the meantime, you can just ignore this when you see it. It's telling you it succeeded -- it's just that your platform's libc expresses success in terms of it not having an error string for success, so that's the message you get.

If you're noticing this because you're scraping logs for "error", I'd suggest excluding specifically "Undefined error: 0" from that. Cyrus is probably not the only software to assume strerror(0) produces a meaningful success string, so even when we fix Cyrus, it'll still be useful for you to know to recognise and ignore "Undefined error: 0" elsewhere.