Bug 289084

Summary: kio_imap APPEND omits message size without flags
Product: [Applications] kdepimlibs Reporter: Luke-Jr <luke-jr+kdebugs>
Component: mailtransportAssignee: kdepim bugs <kdepim-bugs>
Severity: major CC: dilfridge, kdepim-bugs, perezmeyer, rakuco
Priority: NOR    
Version: 4.7   
Target Milestone: ---   
Platform: Gentoo Packages   
OS: Linux   
Latest Commit: Version Fixed In: 4.8.0
Attachments: Patch to fix the bug.
more compact version of the patch, makes it easier to see the change

Description Luke-Jr 2011-12-15 23:09:21 UTC
Created attachment 66792 [details]
Patch to fix the bug.

Version:           4.7 (using KDE 4.7.3) 
OS:                Linux

kio_imap's APPEND command is missing the (mandatory) message size when there are no flags being set on the new message.

Reproducible: Always

Steps to Reproduce:
KIO PUT a file to any IMAP 4.1 compliance server (like Courier IMAP).

Actual Results:  

Expected Results:  
Message added to mail folder.

This is a regression, broken by:
commit d961033b4a5c02edbda8901a2c90349f859f6c00
Author: Montel Laurent <montel@kde.org>
Date:   Sun May 22 15:05:57 2011 +0200

Comment 1 Andreas K. Huettel 2011-12-15 23:22:11 UTC
Please have a look at this, it may be related to 

Comment 2 Andreas K. Huettel 2011-12-15 23:49:56 UTC
Created attachment 66793 [details]
more compact version of the patch, makes it easier to see the change
Comment 3 Andreas K. Huettel 2011-12-16 00:02:03 UTC
I can confirm that my simplified patch resolves the problem described in the Gentoo bug.

> https://bugs.gentoo.org/show_bug.cgi?id=382411
Comment 4 Luke-Jr 2011-12-16 02:54:39 UTC
(In reply to comment #2)
> Created an attachment (id=66793) [details]
> more compact version of the patch, makes it easier to see the change

That was what I originally had, but I opted to reformat it slightly to make it easier to read (as opposed to easier to compare) in hopes of avoiding a regression like this in the future.

I would prefer whoever merges this provide proper attribution. `git am <file>` on my original patch will do this properly, or `git commit --author="Luke Dashjr <luke-jr+git@utopios.org>"` if another route is taken.
Comment 5 Allen Winter 2011-12-17 17:48:57 UTC
Git commit ddc273ca863aacc65742bf6193826809a2d3265e by Allen Winter.
Committed on 17/12/2011 at 18:38.
Pushed by winterz into branch 'master'.

In clientAppend(), be careful how the arguments for the APPEND command
are constructed so as not to break things.

Found by Luke Dashjr <luke-jr+git@utopios.org>
with help from Andreas. Thanks guys.
Please test this.

BUG: 289084
FIXED-IN: 4.8.0

M  +6    -1    kioslave/imap4/imapcommand.cpp

Comment 6 Luke-Jr 2011-12-17 18:32:58 UTC
I'm not sure Allen's rewrite will work, unless there's some C++/Qt magic I'm unaware of... You're starting with a char, adding a QString, then a const char*. In the original version, as well as my rewrite, everything begins with a QString, so there is operator+ in effect. Perhaps more importantly, the original commit that broke this was doing something to enable some QString optimization, and I suspect this rewrite may break that.

Is there a reason not to just revert this and apply my original, safer patch with `git am`?