Bug 3822 - AttRemover deletes message and fails to create new one when disk is full
Summary: AttRemover deletes message and fails to create new one when disk is full
Status: RESOLVED FIXED
Alias: None
Product: Claws Mail (GTK 2)
Classification: Unclassified
Component: Plugins/AttRemover (show other bugs)
Version: 3.11.1
Hardware: PC Linux
: P3 major
Assignee: users
URL:
Depends on:
Blocks:
 
Reported: 2017-05-09 20:36 UTC by J
Modified: 2017-06-17 15:07 UTC (History)
0 users

See Also:


Attachments
Possible fix for this (878 bytes, patch)
2017-05-11 23:14 UTC, Ricardo Mones
no flags Details | Diff

Description J 2017-05-09 20:36:44 UTC
The other day, my server disk space was full. I tried to gain space using AttRemover and realized that the messages were simply deleted.

I guess the plugin deletes the old message, then tries to create a new one without the attachments, but if the creation fails, everything is lost.

Perhaps the new message should be created first and the whole process aborted in case of creation failure.
Comment 1 Ricardo Mones 2017-05-10 00:12:37 UTC
(In reply to comment #0)
> The other day, my server disk space was full. I tried to gain space using
> AttRemover and realized that the messages were simply deleted.
> 
> I guess the plugin deletes the old message, then tries to create a new one
> without the attachments, but if the creation fails, everything is lost.

That's correct, see http://git.claws-mail.org/?p=claws.git;a=blob;f=src/plugins/att_remover/att_remover.c;h=1092c15ef650c18b226bdacee559de011f1583d3;hb=HEAD#l109

> Perhaps the new message should be created first and the whole process
> aborted in case of creation failure.

Yep, but checking for creation failure is also missing in current code.

And this is not just a normal bug, since it causes data loss.
Comment 2 Ricardo Mones 2017-05-11 23:14:19 UTC
Created attachment 1749 [details]
Possible fix for this

Hi! would be nice if you can build with the attached patch and test if it fixes the problem for you. Any comment welcome.

Thanks in advance,
Comment 3 J 2017-05-12 07:22:23 UTC
LGTM

I don't think I'll have the time to build and reproduce the test case (fortunately, I fixed the "server full" issue, and I don't have any test server to try that on).

Does claws-mail have a unit tests suite? I didn't find it in the source.

Thanks for the fix.
Comment 4 Ricardo Mones 2017-05-13 10:39:30 UTC
(In reply to comment #3)
> LGTM
> 
> I don't think I'll have the time to build and reproduce the test case
> (fortunately, I fixed the "server full" issue, and I don't have any test
> server to try that on).

A virtual machine with a small disc could be a cheap test server for this case.

> Does claws-mail have a unit tests suite? I didn't find it in the source.

You answered yourself ;)

> Thanks for the fix.

You're welcome!
Comment 5 users 2017-06-17 15:07:03 UTC
Changes related to this bug have been committed.
Please check latest Git and update the bug accordingly.
You can also get the patch from:
http://git.claws-mail.org/

++ ChangeLog	2017-06-17 17:07:03.036944194 +0200
http://git.claws-mail.org/?p=claws.git;a=commitdiff;h=805dd86ab3e179e9c69e5b98e2b6aed243e6c3ea
Merge: 384a0e7 d0cd541
Author: Colin Leroy <colin@colino.net>
Date:   Sat Jun 17 17:07:02 2017 +0200

    Merge branch 'master' of file:///home/git/claws

http://git.claws-mail.org/?p=claws.git;a=commitdiff;h=d0cd54152fa784f31fa3d01e80cc4f20eb2d2d6f
Author: Ricardo Mones <ricardo@mones.org>
Date:   Sat Jun 17 17:05:55 2017 +0200

    Fix bug 3822 ‘AttRemover deletes message and fails to create new one when disk is full’

Note You need to log in before you can comment on or make changes to this bug.