Bug 289084 - kio_imap APPEND omits message size without flags
Summary: kio_imap APPEND omits message size without flags
Alias: None
Product: kdepimlibs
Classification: Applications
Component: mailtransport (show other bugs)
Version: 4.7
Platform: Gentoo Packages Linux
: NOR major
Target Milestone: ---
Assignee: kdepim bugs
Depends on:
Reported: 2011-12-15 23:09 UTC by Luke-Jr
Modified: 2011-12-17 18:32 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In: 4.8.0

Patch to fix the bug. (1.37 KB, patch)
2011-12-15 23:09 UTC, Luke-Jr
more compact version of the patch, makes it easier to see the change (717 bytes, patch)
2011-12-15 23:49 UTC, Andreas K. Huettel

Note You need to log in before you can comment on or make changes to this bug.
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`?