We have moved to a new Sailfish OS Forum. Please start new discussions there.
7

Sent emails may end up containing multiple "From:" headers.

asked 2018-09-14 17:25:45 +0300

pichlo gravatar image

Observed in

2.2.1.18 Nurmonjoki

Steps to reproduce

  1. Start composing a new email but close it (swipe right) half way through to move it to Drafts
  2. Got to Drafts to finish the email
  3. Send it

As long as the recipient's email server is fussy enough (for example Gmail), the email bounces with the server complaining that the email is "not RFC compliant" because it contains "multiple From headers".

Here is an example I have reproduced earlier, with personal data anonymized:

From: My Name <my.name@blabla.com>
X-Priority: 3
To: blablabla@gmail.com
From: My Name <my.name@blabla.com>
Subject: Test
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: base64
Date: Sun, 9 Sep 2018 05:48:21 +0000
Message-ID: <7jf0ws.perxgm.1hge162-qmf@mail54.blabla.com>
MIME-Version: 1.0

QmxhIEJsYSBCbGE=

To be fair, I do not know whether this is a regression or it has always been like that. I have saved emails in Drafts before to send them later, but they may not have been to a Gmail address.

edit retag flag offensive close delete

2 Answers

Sort by » oldest newest most voted
11

answered 2018-09-14 18:54:20 +0300

updated 2018-09-17 11:56:36 +0300

Edit 2018-09-17: Thanks to the comments from @Mohjive and @DaveRo I've updated the patch upstream with a new version ensuring that all header fields mentioned in the RFC should be present only once. I've expanded the test set also for all these fields. Waiting for review now.

Edit 2018-09-15: I've created a patch upstream to ensure that From: field cannot be inserted twice and is updated instead. I've added a test demonstrating the issue and the fix. I will wait for review and after acceptance, it should come in a later version of SFOS. Thank @pichlo for the detailed report which make the bug easy to identify and correct.

The issue comes from nemo-qml-plugin-email/src/emailmessage.cpp#237, there, if the mail is a draft, it is creating a new message, setting its From field at line #229 and then copy all header fields (including the From field of the draft) at line #239.

The setFrom() call should be done _after_ appending all fields.

I'm preparing a MR in messaging framework to ensure that the From header field is present only once. This should fix the awkward construction in nemo-qml-plugin-email. I'll update the answer when ready.

edit flag offensive delete publish link more

Comments

1

I cannot comment on the review site, so I do it here:

I don't know the full application, but to me it seems that the modified function prevents the from-field to be added at all. Is it guaranteed that it is added elsewhere? Also, shouldn't other fields also be checked for duplicates?

Mohjive ( 2018-09-16 11:21:54 +0300 )edit

I, to, am not 100% convinced about the efficacy of the suggested fix, but I do not know enough to be the judge. As a layman, it seems to me that we are curing the symptom (removing duplicates) rather than the disease (finding out why it was duplicated in the first place). But, as I say, who am I to judge, as long as it works.

pichlo ( 2018-09-16 11:54:09 +0300 )edit

@Mohjive thank you for the remarks. I've added a test to ensure that I didn't break any current behaviour. From: field can still be added after the modification. The update() function, if field does not exist yet, is not calling the member append() function (which would result in an infinite loop with my modification) but the QList<> append() function on the attribute _headerFileds which is a QList<>. About your second point, that's a very good remark. I need to look closer to the RFC to answer though.

@pichlo, indeed your remark is meeting the second point of @Mohjive. The API is allowing to set From: field by QMailMessage::setFrom(), which is a sensible API, and also by QMailMessagePartContainer::appendHeaderField() which is more generic. I think my patch is indeed curing the root of the issue by adding exceptions in the generic routine for cases that cannot be treated by a generic append action. Of course the point raised by @Mohjive stands and need to be addressed.

That being said, I'm open to any suggestion of modification and would be happy to change the patch for a more robust one.

Damien Caliste ( 2018-09-16 16:05:31 +0300 )edit

RFC2822 (3.6) is the latest, I think

DaveRo ( 2018-09-17 09:08:08 +0300 )edit

@DaveRo, thank you. I'm going to update the patch for all these headers.

Damien Caliste ( 2018-09-17 10:32:26 +0300 )edit
2

answered 2018-09-16 21:54:15 +0300

WT.Sane gravatar image

updated 2018-09-16 21:54:30 +0300

Same problem here. But thank you. Now I at least know, where that multiple header problem came from.

Brilliant Workaround: Being determined in writing email helps. ;-)

edit flag offensive delete publish link more
Login/Signup to Answer

Question tools

Follow
2 followers

Stats

Asked: 2018-09-14 17:25:45 +0300

Seen: 359 times

Last updated: Sep 17 '18