Hello,
I am testing 2006c1 for a new deployment and I noticed imapd was
crashing if I tried copying a message into empty mbox mailboxes (as in
zero length). I did some digging around and I am either doing something
very wrong or else this case needs to be handled differently. This is
the first location where I detected a crash:
unix.c file, unix_copy function:
if (!tstream->rdonly) {
((UNIXLCAL *) tstream->local)->dirty = T;
}
when the mailbox is zero length tstream is just &unixproto (which itself
is ={&unixdriver}) and therefore tstream->local is NULL - hence the
crash. I can test for that in the above if statement but it crashes
later on again. This time in unix_close where again it tries to
reference local. Even after testing for NULL there it will still crash
because mail_close_full wants to do fs_give on &unixproto and that is
not going to work and now we are going over my head
Any hints? Help? Please?
Thanks!
Josko P.
Imap-uw mailing list
Imap-uw@u.washington.edu

Thu, 26 2006, Josko Plazonic wrote:
I am testing 2006c1 for a new deployment and I noticed imapd was crashing if
I tried copying a message into empty mbox mailboxes (as in zero length).

I believe that this issue is resolved in the imap-2006d development
snapshot tarball:

This problem was introduced as part of UIDPLUS support back in imap-2005
development days, is specific to traditional UNIX mailbox format, and only
with zero-length files. Normally, the UW IMAP software creates
traditional mailbox format files with the internal DN'T DELETE message,
so this issue only affects those sites which use external software to
manipulate the files. The fact that nobody encountered it until now
suggests that community is rather small.

So, I'm calling this a less-than critical problem, and will hold off on
promoting imap-2006d to release status for a little while. We've had a
few too many rapid releases lately, and I'm sure everybody would like a
break ;-)
-- Mark --

Science does not emerge from voting, party politics, or public debate.
Si vis pacem, para bellum.

Imap-uw mailing list
Imap-uw@u.washington.edu

Mark Crispin wrote:
Normally, the UW IMAP software creates traditional mailbox format
files with the internal DN'T DELETE message, so this issue only
affects those sites which use external software to manipulate the
files. The fact that nobody encountered it until now suggests that
community is rather small.
You'd think so but I initially encountered it in the following
scenario. If an account is using mbox driver it seems like in the
current version the driver will move emails from
/var/spool/mail/username into mbox file and then leave
/var/spool/mail/username with 0 length. If you now move an email into
inbox imapd will attempt to add it to /var/spool/mail/username and crash.

Anyway, I am more then happy to have the dev/test version - good enough
for me I'll test it tomorrow.

Thanks for a prompt fix!

Josko P.

Imap-uw mailing list
Imap-uw@u.washington.edu

Thu, 26 2006, Josko Plazonic wrote:
>Normally, the UW IMAP software creates traditional mailbox format files
>with the internal DN'T DELETE message, so this issue only affects those
>sites which use external software to manipulate the files. The fact that
>nobody encountered it until now suggests that community is rather small.
You'd think so but I initially encountered it in the following scenario. If
an account is using mbox driver it seems like in the current version the
driver will move emails from /var/spool/mail/username into mbox file and then
leave /var/spool/mail/username with 0 length. If you now move an email into
inbox imapd will attempt to add it to /var/spool/mail/username and crash.

Doesn't this scenario require the removal of ~/mbox prior to moving a
message into inbox? If there's an ~/mbox, then moving a message to inbox
should move to ~/mbox rather than the spool file.

Because people complained about the DN'T DELETE message being left in the
spool file after it was drained, the "snarf" code which moves mail from
the spool file to an alternative INBX was changed to empty the spool
file. The Law of Unintended Consequences bites again!
-- Mark --

Science does not emerge from voting, party politics, or public debate.
Si vis pacem, para bellum.

Imap-uw mailing list
Imap-uw@u.washington.edu

Mark Crispin wrote:
Doesn't this scenario require the removal of ~/mbox prior to moving a
message into inbox? If there's an ~/mbox, then moving a message to
inbox should move to ~/mbox rather than the spool file.
Copying of a message into Inbox invokes unix_copy (there is no
mbox_copy) which doesn't know about mbox being the inbox so the message
gets appended to /var/spool/mail/username. I guess it would be nice if
it did - this way there is a delay before imapd notices there is new
staff in /var/spool/mail/username and moves it to the mbox.

Anyway, the dev version fixes issues I've seen. Thanks!

Josko P.

Imap-uw mailing list
Imap-uw@u.washington.edu

Fri, 27 2006, Josko Plazonic wrote:
Copying of a message into Inbox invokes unix_copy (there is no mbox_copy)
which doesn't know about mbox being the inbox so the message gets appended to
/var/spool/mail/username.

An mbox_copy() wouldn't help; unlike append, the driver method call for
copy is selected by the source mailbox rather than the target. Put
another way, an mbox_copy() would only be called if the source mailbox is
INBX -- and then it would have to do the same actions as unix_copy().

However, what can be done is make unix_copy() recognize the case of the
target being INBX and testing for mbox. Please see if the following
patch to unix.c does the trick.
-- Mark --

Science does not emerge from voting, party politics, or public debate.
Si vis pacem, para bellum.

unix/unix.c2006/10/26 21:04:161.16
unix/unix.c2006/10/27 21:39:06

23,29
*Internet: MRC (AT) CAC (DOT) Washington.EDU
*
* Date:20 December 1989
! * Last Edited:26 2006
*/

23,29
*Internet: MRC (AT) CAC (DOT) Washington.EDU
*
* Date:20 December 1989
! * Last Edited:27 2006
*/

880,886
if (!((options & CP_UID) ? mail_uid_sequence (stream,sequence) :
mail_sequence (stream,sequence))) return NIL;
/* make sure destination is valid */
! if (unix_valid (mailbox) || !errno) {
/* try to open rewrite */
if (!(tstream = mail_open_work (&unixdriver,NIL,mailbox,
P_SILENT|P_NKD)))
880,887
if (!((options & CP_UID) ? mail_uid_sequence (stream,sequence) :
mail_sequence (stream,sequence))) return NIL;
/* make sure destination is valid */
! if ((mbox_valid (mailbox) && (mailbox = "mbox")) ||
! unix_valid (mailbox) || !errno) {
/* try to open rewrite */
if (!(tstream = mail_open_work (&unixdriver,NIL,mailbox,
P_SILENT|P_NKD)))

Imap-uw mailing list
Imap-uw@u.washington.edu

Fri, 27 2006, Mark Crispin wrote:
However, what can be done is make unix_copy() recognize the case of the
target being INBX and testing for mbox. Please see if the following patch
to unix.c does the trick.

It turns out that a bit more needs to be done, including making sure that
the mbox driver is enabled. The latest imap-2006d development snapshot
tarball has the full code.

Note that this doesn't try to handle the case of other INBX formats, such
as mbx or mix; those still have to go through the spool file. If it did,
that would disable copying to INBX in applications which do not arm a
proxycopy method. I felt that would be more harmful than the trip through
the spool file.
-- Mark --

Science does not emerge from voting, party politics, or public debate.
Si vis pacem, para bellum.

Imap-uw mailing list
Imap-uw@u.washington.edu

Mark Crispin wrote:
It turns out that a bit more needs to be done, including making sure
that the mbox driver is enabled. The latest imap-2006d development
snapshot tarball has the full code.
Excellent, that seems to work.

Now that you are in the mood for tuning mbox I have to ask if the
mbox_create function should be what it is now:
long mbox_create (MAILSTREAM *stream,char *mailbox)
{
return unix_create (NIL,"mbox");
}

It seems to me that it either should not exist or else it should be
something like:
long mbox_create (MAILSTREAM *stream,char *mailbox)
{
if (!compare_cstring (mailbox,"INBX"))
return unix_create (NIL,"mbox");
else
return unix_create (NIL,mailbox);
}

At least I've had to do this change in order to enable users with mbox
to create new folders. the imapd replies with:
CREATE failed: Can't create mailbox node /u/username/mbox: File exists

Now it is true that I change the source code in a few places in order to
match the local environment better but nothing that should break the above.

Thanks,

Josko P.

Imap-uw mailing list
Imap-uw@u.washington.edu

Tue, 31 2006, Josko Plazonic wrote:
Now that you are in the mood for tuning mbox I have to ask if the mbox_create
function should be what it is now:
long mbox_create (MAILSTREAM *stream,char *mailbox)
{
return unix_create (NIL,"mbox");
}

Yes. The mbox driver is only valid for INBX, and not for any other name.

At least I've had to do this change in order to enable users with mbox to
create new folders. the imapd replies with:
CREATE failed: Can't create mailbox node /u/username/mbox: File exists

I'm surprised to hear this. Did you do something like set CREATEPRT to
mbox or same-as-inbox? , mbox_create() should never be called.
-- Mark --

Science does not emerge from voting, party politics, or public debate.
Si vis pacem, para bellum.

Imap-uw mailing list
Imap-uw@u.washington.edu

Mark Crispin wrote:
At least I've had to do this change in order to enable users with mbox
to create new folders. the imapd replies with:
>CREATE failed: Can't create mailbox node /u/username/mbox: File exists
>
I'm surprised to hear this. Did you do something like set CREATEPRT
to mbox or same-as-inbox? , mbox_create() should never be
called.
Got me - CREATEPRT is same-as-inbox - mostly because of mbx (and soon
mix) users and I've had to adjust mbox_create in order not to break
remaining mbox users.

Josko P.

Imap-uw mailing list
Imap-uw@u.washington.edu