Last Comment Bug 256510 - Return receipts don't use Multiple identities
: Return receipts don't use Multiple identities
Status: VERIFIED FIXED
: privacy, verified1.8.1.8
Product: MailNews Core
Classification: Components
Component: Backend (show other bugs)
: Trunk
: All All
: -- major with 6 votes (vote)
: mozilla1.9alpha7
Assigned To: Lorenzo Simionato
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2004-08-22 11:14 PDT by Miguel
Modified: 2008-07-31 04:30 PDT (History)
8 users (show)
mscott: blocking‑thunderbird2-
dveditz: blocking1.8.0.1-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch for nsMsgMdnGenerator.cpp (2.20 KB, patch)
2007-07-18 00:47 PDT, Lorenzo Simionato
mozilla: review-
Details | Diff | Review
Patch for nsMsgMdnGenerator.cpp (no match -> no send) (2.23 KB, patch)
2007-07-19 00:26 PDT, Lorenzo Simionato
no flags Details | Diff | Review
Patch for nsMsgMdnGenerator.cpp (no match -> first identity) (2.43 KB, patch)
2007-07-19 10:56 PDT, Lorenzo Simionato
no flags Details | Diff | Review
Patch for nsMsgMdnGenerator.cpp (3.28 KB, patch)
2007-07-20 06:25 PDT, Lorenzo Simionato
mozilla: review-
Details | Diff | Review
[checked in] Patch for nsMsgMdnGenerator.cpp (3.26 KB, patch)
2007-07-25 11:10 PDT, Lorenzo Simionato
mozilla: review+
Details | Diff | Review
[checked in] Patch for nsMsgMdnGenerator.cpp 1.8 branch (3.44 KB, patch)
2007-07-27 02:05 PDT, Lorenzo Simionato
mozilla: review+
mscott: approval1.8.1.8+
Details | Diff | Review

Description Miguel 2004-08-22 11:14:06 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.2) Gecko/20040803
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.2) Gecko/20040803

When you receive an email which was sent to any identity which isn't the main
one for that account, the sender gets a return receipt from the main identity
instead of an email from the account which he sent the email to.

Reproducible: Always
Steps to Reproduce:
1.
2.
3.
Comment 1 Miguel 2005-01-17 15:17:19 PST
Thunderbird 1.0 has the same problem...
Comment 2 Miguel 2005-01-24 09:14:12 PST
Can somebody have a look at this and confirm it? It's an importante privacy
issue and a major loss of what Mozilla offers. If you can't use Return receipts
for your identities, what do you want Return receipts? :)
Comment 3 Pierre Etchemaite 2005-06-03 03:44:51 PDT
Still exists in Thunderbird 1.0.2.

Since the logic to pick the right identity to reply to emails already exists,
fixing that bug shouldn't be _that_ hard...
Comment 4 Miguel 2005-08-03 16:29:11 PDT
Still there in 20050803...
Comment 5 Miguel 2005-09-10 07:23:15 PDT
Maybe the same logic proppossed for bug# 303310 could be used here...
Comment 6 Stefan Geisseler 2005-10-01 06:56:20 PDT
Is this bug what the extension "Correct Identity" tries to fix? Can you confirm
that?

https://addons.mozilla.org/extensions/moreinfo.php?application=thunderbird&id=1203

If so, there are certainly too less people with confirmation rights here, when
they even cannot confirm bugs with extension-fixes around...
Comment 7 Miguel 2005-10-13 04:53:31 PDT
(In reply to comment #6)
> Is this bug what the extension "Correct Identity" tries to fix? Can you confirm
> that?
> 
> https://addons.mozilla.org/extensions/moreinfo.php?application=thunderbird&id=1203
> 
> If so, there are certainly too less people with confirmation rights here, when
> they even cannot confirm bugs with extension-fixes around...

I don't think so, the problem has nothing to do with the reply identity but the
identity that Mozilla uses when returns the 'Return receipts' for the mail...
Comment 8 Daniel Veditz [:dveditz] 2006-01-19 12:06:37 PST
Not a security/stability release blocker. Need a tested, safe fix before we can consider it for the branches
Comment 9 AlexIhrig 2006-02-26 10:04:56 PST
We should NOT send MDNs from a specific identity. In some cases ALL identities are the false ones. This occurs, when I'm using my webhosters service to collect messages from different mail accounts. This service sends the collected messages to a free eligible account/identity.

Having a look into the message header shows the original correct "To: My Name <foo@sender.de>" line. This address would be the only correct one, to use for the MDN.

The problem is, we have to use a working identity to send by SMTP. So we are not free to use foo@sender.de as "From: ....". A compromise would be to have the correct original senders address foo@sender.de in the messages body.
Comment 10 Miguel 2006-02-26 10:33:51 PST
(In reply to comment #9)
> We should NOT send MDNs from a specific identity. In some cases ALL identities
> are the false ones. This occurs, when I'm using my webhosters service to
> collect messages from different mail accounts. This service sends the collected
> messages to a free eligible account/identity.
> 
> Having a look into the message header shows the original correct "To: My Name
> <foo@sender.de>" line. This address would be the only correct one, to use for
> the MDN.
> 
> The problem is, we have to use a working identity to send by SMTP. So we are
> not free to use foo@sender.de as "From: ....". A compromise would be to have
> the correct original senders address foo@sender.de in the messages body.
> 

Yes, but from my point of view, that's is a particular case. I mean, if the problem is that your SMTP server isn't going to accept messages sent from any of your identities, then the problem is not only with the MDN, but also with normal mails you try to send from those identities and Thunderbird can't do anything to solve that.

Also, according to RFC 2298: "The From field of the message header of the MDN MUST contain the address of the person for whom the message disposition notification is being issued", so if the MDN was issued to name@domain.com, the MDN from field must be name@domain.com
Comment 11 Scott MacGregor 2006-05-25 14:20:38 PDT
Not going to stop ship for this, but this seems like a good bug to fix for thunderbird 2 so I'm putting it into the Thunderbird 2 milestone bucket. 

Comment 12 Neil Parks 2006-08-11 10:40:47 PDT
(In reply to comment #11)
> Not going to stop ship for this, but this seems like a good bug to fix for
> thunderbird 2 so I'm putting it into the Thunderbird 2 milestone bucket. 
> 

It has a minus sign for blocking Tbird 2.  Does that mean it won't be fixed in 2.0?



Comment 13 Lorenzo Simionato 2007-07-03 11:50:54 PDT
(In reply to comment #12)
> (In reply to comment #11)
> > Not going to stop ship for this, but this seems like a good bug to fix for
> > thunderbird 2 so I'm putting it into the Thunderbird 2 milestone bucket. 
> > 
> 
> It has a minus sign for blocking Tbird 2.  Does that mean it won't be fixed in
> 2.0?
> 
The bug is still present in Thunderbird 2.0.0.4 (20070604)!
This is an important issue, that should be fixed.
Comment 14 Lorenzo Simionato 2007-07-17 10:10:22 PDT
Ok...i'll try to fix it!
Comment 15 Lorenzo Simionato 2007-07-18 00:47:07 PDT
Created attachment 272749 [details] [diff] [review]
Patch for nsMsgMdnGenerator.cpp

Ok this fix should be ok, i hope.
Please double check the file since is my very first patch.
Comment 16 David :Bienvenu 2007-07-18 10:09:59 PDT
Comment on attachment 272749 [details] [diff] [review]
Patch for nsMsgMdnGenerator.cpp

Thx for the patch!

I think you'll want to fall back to the first identity in case we didn't find a match.

 Other than that, it looks OK. There are a couple of space formatting things:

m_identity=ident

should be

m_identity = ident;

and
+            if(servIdentities)

should be 
if (servIdentities)

And I think this comment:

+            // fix for bug 256510. Now search for the correct identity looking
+            // the field TO on the message header


should read something like:

Find the correct identity based on the "To:" header.

Is it possible to get into this code where's we're not on the To: field? If so, that would make falling back to some identity even more important.
Comment 17 Lorenzo Simionato 2007-07-18 10:45:21 PDT
I don't know if it's a good idea to fall back to the first identity if we haven't a match. This way the user who sended the message requesting the receipt would receive a message from an address he probably doesn't know (the first identity).
Anyway maybe is better than sending nothing at all...
Comment 18 Henrik Skupin (:whimboo) 2007-07-18 12:13:12 PDT
Yeah, otherwise it could be a privacy issue. We still have it now, but IMO this should be fixed. If I don't want to publicize my first identity of that account I also don't want to send a recipe from that address. That's why we shouldn't send a recipe for that case.
Comment 19 Lorenzo Simionato 2007-07-19 00:26:22 PDT
Created attachment 272927 [details] [diff] [review]
Patch for nsMsgMdnGenerator.cpp (no match -> no send)

Updated. As suggested by Henrik in case there is no match, i don't set m_identity so we don't send the message at all. If this is not ok, just tell me.
Comment 20 David :Bienvenu 2007-07-19 09:43:09 PDT
I believe you're allowed to request an mdn for someone on the cc list or even bcc - I don't see anything here that disallows that http://www.faqs.org/rfcs/rfc2298.html and I see some things that imply a mail sender could, e.g., the part about sending out multiple e-mails to handle multiple return receipts. And we have code and ui to handle this (in particular, if the return receipt requestee isn't in the to or cc list). So you can't simply not send a return receipt in that case. So I think you have to fall back to the first identity. The privacy issue is dealt with by prompting the user if the e-mail address of the identity used isn't in the to or cc list.

Comment 21 Lorenzo Simionato 2007-07-19 10:56:27 PDT
Created attachment 272989 [details] [diff] [review]
Patch for nsMsgMdnGenerator.cpp (no match -> first identity)

Ok, this version use the first identity if there is no match. Anyway i've not understand this: "privacy issue is dealt with by prompting
the user if the e-mail address of the identity used isn't in the to or cc list". In fact if you get a mail with EVERY ADDRESS in the To header (assume nothing in CC) the prompt to the user is the same as an email with one identity in the To header. That is why i would apply the first patch, anyway i'm not a thunderbird guru or expert.
Comment 22 David :Bienvenu 2007-07-19 12:12:20 PDT
I mean we allow you to configure our mdn response prompting differently if you aren't on the to: or cc: list - look in account, settings, return receipts.

When the prompt is displayed, you can look at the message header pane and see if you want to reply to it based on what the to: address is - if you don't want to your first identity to get used, just say no to sending the mdn response.

Is there a reason your patch doesn't also look for the the identity based on the cc: header if you don't find one based on the to: header? I.e., is this intentional, or just didn't seem worth the bother?
Comment 23 Lorenzo Simionato 2007-07-20 06:25:56 PDT
Created attachment 273114 [details] [diff] [review]
Patch for nsMsgMdnGenerator.cpp

Yes, you are right on all points. Don't send anything makes the settings in the options totally useless! I've also forgot looking in the Cc header, it wasn't intentional. Finally, there was another error looking for the email address: it's not possibile to use MailAddrMatch because for example the To can have a lot of addresses so you won't find anything.
Comment 24 David :Bienvenu 2007-07-23 15:55:34 PDT
Comment on attachment 273114 [details] [diff] [review]
Patch for nsMsgMdnGenerator.cpp

thx for the patch.

The braces here aren't needed:

+            if (!m_identity)
+            {
+              rv = accountManager->GetFirstIdentityForServer(m_server, getter_AddRefs(m_identity));
+            }

and this brace should be on its own line:

+              if (!m_identity) {
+                for (PRUint32 i = 0; i < count; i++)


But what's more worrisome is this check:

 PL_strcasestr(mailTo.get(), identEmail.get()))

Maybe this is what we do in our other identity detection code (I'll check) but it will take something like a@b.uk and have a positive match with a@b.uk.com, which isn't right. Maybe we don't care, but I'd want to make sure.

In any case, I think you'd want to use something like

mailTo.Find(identEmail, PR_TRUE) != kNotFound)
Comment 25 David :Bienvenu 2007-07-23 16:26:24 PDT
Comment on attachment 273114 [details] [diff] [review]
Patch for nsMsgMdnGenerator.cpp

I looked through our other identity managing code, and it does a simple Find as well, so if you just redo that code to use .Find, and fix the braces, we'll be done. Thx again
Comment 26 Bernd Jantzen 2007-07-23 23:35:55 PDT
The functionality suggested by Lorenzo Simionato and David Bienvenu will be a great improvement with respect to the code we have today, and I hope that this patch will soon enter the official versions of Thunderbird. This would - for the first time - make sending return receipts a usable feature for me.

But on the long term, I have a few more suggestions:

* When Thunderbird asks whether it should send a return receipt, it should display the criterion which led to this question, e.g. "You are not in the To or Cc of the message" or "The sender is outside of your domain". This would be a help for quick decisions on whether I would like to send this return receipt. (Yes, I can look this up in the e-mail itself, but especially for very long To or Cc lists, this is awful.)

* When Thunderbird cannot determine one of the e-mail addresses of the account from the To and Cc fields, it should ask about which identity to use. Would it be so difficult to include in the pop-up window which asks about sending the return receipt a drop-down menu with the list of all identities for the given account? When the user has chosen "Always send" for the case "If I'm not in the To or Cc of the message", the question about the identity to use can be omitted and the first identity chosen. But if the user has configured "Ask me", he/she should not only be asked yes or no, but also which identity to use.

Thanks for your contributions! I'm really happy to see that something goes on with this bug.
Comment 27 Lorenzo Simionato 2007-07-25 11:10:56 PDT
Created attachment 273809 [details] [diff] [review]
[checked in] Patch for nsMsgMdnGenerator.cpp 

Ok...we should be closer
Comment 28 David :Bienvenu 2007-07-25 14:29:12 PDT
Comment on attachment 273809 [details] [diff] [review]
[checked in] Patch for nsMsgMdnGenerator.cpp 

looks good, thx, Lorenzo. Do you need me to check this in for you?
Comment 29 Lorenzo Simionato 2007-07-26 00:20:33 PDT
Yes, please. I don't have CVS access. I would also put a ? on 'approval1.8.1.6' to get this fix also in the next 2.0 release. Correct? Thx for your time
Comment 30 Lorenzo Simionato 2007-07-26 11:04:53 PDT
I've noted that the file in the last released thunderbird is a little different from trunk...so to get this patch also in the next release i've to test and eventually modify a little the patch and send another one just for this branch?
Comment 31 David :Bienvenu 2007-07-26 11:07:02 PDT
If you could do that, that would be very helpful. Otherwise, I'd have to do it myself...
Comment 32 Lorenzo Simionato 2007-07-27 02:05:56 PDT
Created attachment 274138 [details] [diff] [review]
[checked in] Patch for nsMsgMdnGenerator.cpp 1.8 branch

Ok, that the version for the 1.8 branch, so it should be shipped with maybe thunderbird 2.0.0.6.
If you could please check in this to 1.8 and the other to trunk, i think we are done.
Comment 33 Bernd Jantzen 2007-07-27 06:09:12 PDT
(In reply to comment #32)

> If you could please check in this to 1.8 and the other to trunk, i think we are
> done.

I have successfully compiled Lorenzo's new patch with the 1.8 branch, and it works in principal. But there is still one problem:

(In reply to comment #24)
> Maybe this is what we do in our other identity detection code (I'll check) but
> it will take something like a@b.uk and have a positive match with a@b.uk.com,
> which isn't right. Maybe we don't care, but I'd want to make sure.

This problem is still there, even with the .Find method for string comparisons. I have sent an e-mail requesting a return receipt with something like
"To: my@e-mail.com.net <another@e-mail.com>".
The only part in this To: header corresponding to an e-mail of one of my Thunderbird identities is "my@e-mail.com". Lorenzo's patch found this identity "my@e-mail.com" although 1.) it is part of the real name, not of the address in the To: field, and although 2.) "my@e-mail.com.net" from the To: header is not the same address as "my@e-mail.com".

In fact, I don't wonder, because I guess that the .Find method does a simple string comparison, comparing the addresses from the identities to the whole string of the To: header.

Anyway, I remarked that the same happens when I reply to this message: Also my identity "my@e-mail.com" gets selected although it was not really in the To: or CC: fields.

So I suggest leaving the patch as it is for the moment and include it into Thunderbird 2.0.0.6. But on the long term, a stricter comparison would be desirable, both for replies and for return receipts. One could e.g. use a function which splits a To: or CC: header string into its parts (separated by comma), extracting the e-mail address from each part (omitting the real name if present) and yielding a list of e-mail addresses in some suitable format as result. Then this list could be compared entry by entry with the available identities. [By the way, everything necessary for this functionality should already be present in Thunderbird, because exactly this happens when a mail message is displayed.]
Comment 34 Worcester12345 2007-07-31 23:08:51 PDT
It sure makes sense to me to get this checked in ASAP as is and file any new bugs thereafter. Thanks for staying with this.
Comment 35 Karsten Düsterloh 2007-08-01 14:14:58 PDT
Comment on attachment 273809 [details] [diff] [review]
[checked in] Patch for nsMsgMdnGenerator.cpp 

Landed on trunk as per comment #28.
Comment 36 Henrik Skupin (:whimboo) 2007-08-02 14:09:36 PDT
Will there be more work on this bug or can we close it as resolved?
Comment 37 David :Bienvenu 2007-08-02 18:34:01 PDT
I think we should mark this fixed and open new bugs for remaining issues. If I understand correctly, the problem Bernd points out is real, but probably not common.
Comment 38 Lorenzo Simionato 2007-08-03 01:06:43 PDT
Yes, for the additional issue we could open new bugs.
Anyway i think we could also check in "Patch for nsMsgMdnGenerator.cpp 1.8 branch" to get the fix also in the next 2.0 release. Correct?
Comment 39 Henrik Skupin (:whimboo) 2007-08-03 02:35:16 PDT
Lorenzo, can you run a test with a current trunk nightly build and check if it is working properly? So after David gives his r+ we could ask for approval1.8.1.7.
Comment 40 Lorenzo Simionato 2007-08-04 00:48:58 PDT
Yes, before sending the patch i've compiled fine with the 1.8 branch and tested it, all seems ok. This patch is almost the same of the checked in, just a few type hange.
Comment 41 Henrik Skupin (:whimboo) 2007-08-04 08:54:01 PDT
(In reply to comment #20)
> can't simply not send a return receipt in that case. So I think you have to
> fall back to the first identity. The privacy issue is dealt with by prompting
> the user if the e-mail address of the identity used isn't in the to or cc list.

David, this is only half-implemented currently. If the recipients address is not listed in To or Cc you get ask, right. But there is no warning that another identity is used therefor. Should this be covered in a follow-up bug? 

Otherwise I can verify the behavior. Getting mail for an additional identity Thunderbird uses this one instead of the default identity.

=> VERIFIED.

David, could you please review the 1.8 branch patch? If we could check this in 1.8 we avoid sending of other private mail addresses, which will mainly fix the privacy issue for that bug.
Comment 42 David :Bienvenu 2007-08-04 09:30:01 PDT
Comment on attachment 274138 [details] [diff] [review]
[checked in] Patch for nsMsgMdnGenerator.cpp 1.8 branch

looks fine, thx.
Comment 43 David :Bienvenu 2007-08-04 10:12:36 PDT
>But there is no warning that another
>identity is used therefor. Should this be covered in a follow-up bug? 

Personally, I always base my decision whether or not to send an MDN response on who's asking for it, not who the mail was sent to. But if folks think such a warning is worthwhile, then someone can file a new bug.
Comment 44 Karsten Düsterloh 2007-09-12 12:23:15 PDT
Comment on attachment 274138 [details] [diff] [review]
[checked in] Patch for nsMsgMdnGenerator.cpp 1.8 branch

Landed on MOZILLA_1_8_BRANCH.
Comment 45 Marcia Knous [:marcia - use ni] 2007-10-19 12:16:29 PDT
verified fixed on the 1.8 branch using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.8pre) Gecko/20071019 Thunderbird/2.0.0.7pre ID:2007101904. I received the RR from the identity I sent it to, not the default identity. Adding keyword.

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