Re: [PATCH] Don't bother checking for mbox files

Subject: Re: [PATCH] Don't bother checking for mbox files

Date: Sun, 13 Mar 2016 14:48:41 +0200

To: Edward Betts, notmuch@notmuchmail.org

Cc:

From: Jani Nikula


On Sun, 13 Mar 2016, Edward Betts <edward@4angle.com> wrote:
> Keith Packard <keithp@keithp.com> wrote:
>> Postfix adds mbox-style From lines when used in combination with
>> maildrop or .forward files. If they have another line starting with
>> 'From ' in them, notmuch complains about them not being mail files.
>> 
>> If we assume the user hasn't screwed up and misconfigured their mail
>> system, then we can safely ignore whether the file started with an
>> mbox header and just parse it as a single-message file.
>
> I think it is fine to go ahead with this change. At the same time the
> behaviour of Postfix should be corrected so it doesn't add mbox-style From
> lines to mails in maildir format.

I disagree with making the change (as-is, at least).

In general, Notmuch does not support mboxes. We expect maildir style one
message per file mail storage. We support single-message mboxes as a
special case, in part because, as you note, there's plenty of other
software that adds the mbox "From " line even though delivering to
maildir.

I think it's misleading and confusing to the users to accept and index
the first message of mboxes, and silently ignore the rest (or worse,
index all of the mbox and associate the text with the first message). I
think we should reject multi-message mboxes, because we have no code to
handle them. This patch throws away that check.

Now, IIUC, the problem here is not that the files actually are
multi-message mboxes. We could use a sample message (even a crafted one)
that exhibits the problem, so we could add a test case, and fix Notmuch
to deal with it gracefully (if we decide catering to potentially broken
other software is the way to go), while retaining the code to reject
multi-message mboxes. With the test case, we'd also avoid accidentally
breaking this in the future.

BR,
Jani.

Thread: