Bug 289084

Summary:	kio_imap APPEND omits message size without flags
Product:	[Applications] kdepimlibs	Reporter:	Luke-Jr <luke-jr+kdebugs>
Component:	mailtransport	Assignee:	kdepim bugs <kdepim-bugs>
Status:	RESOLVED FIXED
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
Sentry Crash Report:
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:  
Error.

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

    Fix compile with "(-DQT_USE_FAST_CONCATENATION -DQT_USE_FAST_OPERATOR_PLUS)"

Comment 1 Andreas K. Huettel 2011-12-15 23:22:11 UTC

Please have a look at this, it may be related to 

https://bugs.kde.org/show_bug.cgi?id=279432
https://bugs.gentoo.org/show_bug.cgi?id=382411

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

http://commits.kde.org/kdepimlibs/ddc273ca863aacc65742bf6193826809a2d3265e

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`?