Moodle
  1. Moodle
  2. MDL-30698

Message-ID causes mail problem when sending to more than one person (e.g. forum subscription)

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1.2, 2.2
    • Fix Version/s: 2.3
    • Component/s: Forum
    • Labels:
    • Environment:
      Moodle: Windows Server 2008 R2; MySQL DB;
      Mailserver: Linux SuSE postfix, mails dropped and picked up by POP3
    • Testing Instructions:
      Hide

      For this test you'll need 2 users and a course containing a forum. This test is easier if you turn down your forum maximum editing time to 1 minute (or you can check "mail now" when posting in the forum).

      Unless you have recently seen your Moodle send email please verify that your email sending is working. To do this check that a user's messaging preferences are set so that they receive email notifications for personal messages while both online and offline. Have another user message them within Moodle. Check that the email notification is received. This code is not affected by this bug fix. If it does not work your system is not configured correctly.

      Assuming that works, on to the test. You will need 2 users.

      Have a user subscribe to the forum (if they arent already). The subscription/unsubscription link appears in the Settings block while you are viewing the forum.

      Check that the user has their messaging preferences set to "email" while both online and offline for forum posts.

      As another user user post in the forum. Wait for your editing time to elapse. Run /admin/cron.php. Check that the subscribed user receives the notification email about the forum post.

      As the user who posted in the forum, post in the forum again replying to your previous post (the user is replying to themselves). Again, wait then run cron. The second notification email should be grouped by your email client.

      Show
      For this test you'll need 2 users and a course containing a forum. This test is easier if you turn down your forum maximum editing time to 1 minute (or you can check "mail now" when posting in the forum). Unless you have recently seen your Moodle send email please verify that your email sending is working. To do this check that a user's messaging preferences are set so that they receive email notifications for personal messages while both online and offline. Have another user message them within Moodle. Check that the email notification is received. This code is not affected by this bug fix. If it does not work your system is not configured correctly. Assuming that works, on to the test. You will need 2 users. Have a user subscribe to the forum (if they arent already). The subscription/unsubscription link appears in the Settings block while you are viewing the forum. Check that the user has their messaging preferences set to "email" while both online and offline for forum posts. As another user user post in the forum. Wait for your editing time to elapse. Run /admin/cron.php. Check that the subscribed user receives the notification email about the forum post. As the user who posted in the forum, post in the forum again replying to your previous post (the user is replying to themselves). Again, wait then run cron. The second notification email should be grouped by your email client.
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:
      MDL-30698_multiple_emails
    • Rank:
      33525

      Description

      I don't know if this is a mailserver-specific problem - so I've set it as an "Improvement".

      If a new forum post is written, every subscriber gets an e-mail. If more than one person has subscribed to the forum, only one of all e-mails arrive. The problem was, that the mailserver identified them as "duplicate" because of the same "message-ID". So only the first mail was delivered. Looking into the code, this was clear....:

      ../mod/forum/lib.php line 602 (v2.1.2): 'Message-ID: <moodlepost'.$post->id.'@'.$hostname.'>',

      The message-ID consists of the "post-ID"...
      To have individual message-IDs, I just put the "userto-ID" into it as well: 'Message-ID: <moodlepost'.$post->id.$userto->id.'@'.$hostname.'>',

      Now it works, every subscriber gets an e-mail.

      I wonder if the behaviour of our mailserver is normal, but maybe this hint can help others too...

        Issue Links

          Activity

          Hide
          Andrew Davis added a comment - - edited

          That is an interesting problem. It's not something I've heard of before. The line in question, the mail ID line, was last changed in March of 2008 so lots of people have been using it for quite some time without a problem. That would indicate something unusual is going on with your mail server.

          That said, its still possible that we should modify the message ID as, even if you are the first to run into this, you're unlikely to be the last.

          Update: after some googling it seems that the message ID should indeed be unique for each message sent. http://stackoverflow.com/questions/831183/is-the-message-id-email-header-unique-for-each-recipient

          Show
          Andrew Davis added a comment - - edited That is an interesting problem. It's not something I've heard of before. The line in question, the mail ID line, was last changed in March of 2008 so lots of people have been using it for quite some time without a problem. That would indicate something unusual is going on with your mail server. That said, its still possible that we should modify the message ID as, even if you are the first to run into this, you're unlikely to be the last. Update: after some googling it seems that the message ID should indeed be unique for each message sent. http://stackoverflow.com/questions/831183/is-the-message-id-email-header-unique-for-each-recipient
          Hide
          Michael de Raadt added a comment -

          Could you please let us know what mail server you are using, and perhaps other details of your setup?

          Show
          Michael de Raadt added a comment - Could you please let us know what mail server you are using, and perhaps other details of your setup?
          Hide
          Dominik Jeni added a comment - - edited

          Hi Andrew!

          After not having found any problems like mine, I also suggested that something is wrong with our mail server.

          While trying to figure out the problem, I found out the following: If you send mails to multiple persons with a common mail system (googlemail etc), all mails are sent in one session (one "TO" header contains all mail addresses), while mails sent by moodle are all separated into different new sessions. I know, this can be configured under "Site administration -> Plugins -> Message outputs -> Email" (SMTP session limit), but this doesn't work - moodle always sends separated mails.

          Update: Hi Michael! Long time no see! I'll gather information and come back to you soon.

          Update 2: We've got a standard Linux SuSE postfix mail server, that stores mails in files (maildrop). Our mail program picks them up using pop3. But due to the same message-ID, they are recognized as "duplicate".

          Kind regards,
          Dominik

          Show
          Dominik Jeni added a comment - - edited Hi Andrew! After not having found any problems like mine, I also suggested that something is wrong with our mail server. While trying to figure out the problem, I found out the following: If you send mails to multiple persons with a common mail system (googlemail etc), all mails are sent in one session (one "TO" header contains all mail addresses), while mails sent by moodle are all separated into different new sessions. I know, this can be configured under "Site administration -> Plugins -> Message outputs -> Email" (SMTP session limit), but this doesn't work - moodle always sends separated mails. Update: Hi Michael! Long time no see! I'll gather information and come back to you soon. Update 2: We've got a standard Linux SuSE postfix mail server, that stores mails in files (maildrop). Our mail program picks them up using pop3. But due to the same message-ID, they are recognized as "duplicate". Kind regards, Dominik
          Hide
          Andrew Davis added a comment -

          Adding a potential solution for this. I was a little wary of putting the user ID in the email but it should be fine as its going to the user themselves (theoretically) and they could easily get their own user ID from Moodle itself if they so desired. Plus, I wasn't sure what else to use. We could plug in a random number there but that seemed a bit crappy. Not sure why.

          Show
          Andrew Davis added a comment - Adding a potential solution for this. I was a little wary of putting the user ID in the email but it should be fine as its going to the user themselves (theoretically) and they could easily get their own user ID from Moodle itself if they so desired. Plus, I wasn't sure what else to use. We could plug in a random number there but that seemed a bit crappy. Not sure why.
          Hide
          Aparup Banerjee added a comment -

          +1 certainly makes sense to me.

          also "can be configured under "Site administration -> Plugins -> Message outputs -> Email" (SMTP session limit), but this doesn't work - moodle always sends separated mails." - is there an issue for that?

          G'nite moodleverse.

          Show
          Aparup Banerjee added a comment - +1 certainly makes sense to me. also "can be configured under "Site administration -> Plugins -> Message outputs -> Email" (SMTP session limit), but this doesn't work - moodle always sends separated mails." - is there an issue for that? G'nite moodleverse.
          Hide
          Andrew Davis added a comment -

          Adding branches and test instructions.

          Show
          Andrew Davis added a comment - Adding branches and test instructions.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Sorry, reopening.

          That Message-ID header is used, in conjunction with the 'In-Reply-To' and the 'References' headers to provide nesting along a varietè of email clients.

          So all them must follow the same naming schema, or it will break badly.

          To keep all uses always on sync I'd suggest:

          1) Create one function to calculate the messageid.
          2) Use it everywhere

          Also, this should be (still) be backported to 20_STABLE, if I'm not wrong. EOL is Dec 31th.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Sorry, reopening. That Message-ID header is used, in conjunction with the 'In-Reply-To' and the 'References' headers to provide nesting along a varietè of email clients. So all them must follow the same naming schema, or it will break badly. To keep all uses always on sync I'd suggest: 1) Create one function to calculate the messageid. 2) Use it everywhere Also, this should be (still) be backported to 20_STABLE, if I'm not wrong. EOL is Dec 31th. Ciao
          Hide
          Andrew Davis added a comment -

          Adding this to the next sprint as it got left behind.

          Show
          Andrew Davis added a comment - Adding this to the next sprint as it got left behind.
          Hide
          Andrew Davis added a comment -

          I've fixed this up so that nesting of emails should all work as expected.

          Show
          Andrew Davis added a comment - I've fixed this up so that nesting of emails should all work as expected.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The most I think on this, the less logic I found to these message-ids to be globally unique.

          If email A sends one email to B and C, and B is set to automatically forward to C (or B and C are the same mailbox), at the end C will receive 2 emails with the same message id.

          So I really think that no software should rely on unique Message-ID headers at all, in fact, they are set by senders and no system should rely on non-trusted data in that way.

          Also, I use postfix since ages ago and never have detected any problem like that, both in sent and received emails with Moodle. Strange.

          In any case, and as far as by changing this it's going to cause all client using such information for threading to become crazy (lost the threads), perhaps we should consider to:

          1) Do it only for master. So breakage for clients will happen only in 2.3, and not now in the middle of all stables. Or instead,

          2) Do it controlled by setting, so existing sites will continue using the old way and new ones will get, by default, the new (more unique) way. And any existing site will be able to switch at any moment (causing breakage of threading). Or instead,

          3) Ignore the breakage problem and do it for all branches.

          BUT, in any case of the alternative 1,2,3 above:

          4) I would hash the postid + userid information. I cannot imagine any benefit about having that information there in a plain way (specially the userid). Perhaps some bounce control may be using that? Can anybody find a good reason to keep them plain?

          While the patch is 100% correct, I'm going to keep this open util it's decided. Perhaps I'm being too much exigent, but considering that we introduced those headers for the pure reason of providing threading for clients, and knowing that the change will break current ones... I think it's worth deciding and implementing the change is the best possible way.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The most I think on this, the less logic I found to these message-ids to be globally unique. If email A sends one email to B and C, and B is set to automatically forward to C (or B and C are the same mailbox), at the end C will receive 2 emails with the same message id. So I really think that no software should rely on unique Message-ID headers at all, in fact, they are set by senders and no system should rely on non-trusted data in that way. Also, I use postfix since ages ago and never have detected any problem like that, both in sent and received emails with Moodle. Strange. In any case, and as far as by changing this it's going to cause all client using such information for threading to become crazy (lost the threads), perhaps we should consider to: 1) Do it only for master. So breakage for clients will happen only in 2.3, and not now in the middle of all stables. Or instead, 2) Do it controlled by setting, so existing sites will continue using the old way and new ones will get, by default, the new (more unique) way. And any existing site will be able to switch at any moment (causing breakage of threading). Or instead, 3) Ignore the breakage problem and do it for all branches. BUT, in any case of the alternative 1,2,3 above: 4) I would hash the postid + userid information. I cannot imagine any benefit about having that information there in a plain way (specially the userid). Perhaps some bounce control may be using that? Can anybody find a good reason to keep them plain? While the patch is 100% correct, I'm going to keep this open util it's decided. Perhaps I'm being too much exigent, but considering that we introduced those headers for the pure reason of providing threading for clients, and knowing that the change will break current ones... I think it's worth deciding and implementing the change is the best possible way. Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          reopening for further discussion (see previous comment).

          1) will break existing threads?
          2) consider setting controlling it?
          3) hash it?

          Show
          Eloy Lafuente (stronk7) added a comment - reopening for further discussion (see previous comment). 1) will break existing threads? 2) consider setting controlling it? 3) hash it?
          Hide
          Andrew Davis added a comment -

          "If email A sends one email to B and C, and B is set to automatically forward to C (or B and C are the same mailbox), at the end C will receive 2 emails with the same message id."

          They wont. the message ID includes the user ID of the user being emailed. Provided B and C have different Moodle user IDs the emails will have 2 different message IDs.

          Hashing the data to create the message ID seems like a good idea. Its possible someone could parse plain text message IDs to do some sort of fancy mail sorting but that seems unlikely.

          And having this only go into master seems reasonable to me. Certainly breaking everyone's email threading for a point release seems bad.

          Adding a setting also sounds a bit bad. It would effectively just be a toggle switch that has no obvious effect apart from breaking everyone's email threading. "toggle this every time you want to annoy your users"

          Show
          Andrew Davis added a comment - "If email A sends one email to B and C, and B is set to automatically forward to C (or B and C are the same mailbox), at the end C will receive 2 emails with the same message id." They wont. the message ID includes the user ID of the user being emailed. Provided B and C have different Moodle user IDs the emails will have 2 different message IDs. Hashing the data to create the message ID seems like a good idea. Its possible someone could parse plain text message IDs to do some sort of fancy mail sorting but that seems unlikely. And having this only go into master seems reasonable to me. Certainly breaking everyone's email threading for a point release seems bad. Adding a setting also sounds a bit bad. It would effectively just be a toggle switch that has no obvious effect apart from breaking everyone's email threading. "toggle this every time you want to annoy your users"
          Hide
          Andrew Davis added a comment -

          I've added a second commit to the master version that uses a hash in the message ID. Note the message is meant to be formated somewhat like an email address. For some reason. http://en.wikipedia.org/wiki/Message-ID

          Ive removed the stable version for the time being at least as theyre out of date and may not be integrated.

          Show
          Andrew Davis added a comment - I've added a second commit to the master version that uses a hash in the message ID. Note the message is meant to be formated somewhat like an email address. For some reason. http://en.wikipedia.org/wiki/Message-ID Ive removed the stable version for the time being at least as theyre out of date and may not be integrated.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          As far as it is assumed (and perhaps commented somewhere, in release notes or so), that breakage of current threads may happen for some clients, np about doing so. I think that some clients, like Thunderbird, uses to fallback to subject matching if headers don't match.

          Looks perfect IMO. Both the email-like format and the master-only target. +1

          Show
          Eloy Lafuente (stronk7) added a comment - As far as it is assumed (and perhaps commented somewhere, in release notes or so), that breakage of current threads may happen for some clients, np about doing so. I think that some clients, like Thunderbird, uses to fallback to subject matching if headers don't match. Looks perfect IMO. Both the email-like format and the master-only target. +1
          Hide
          Andrew Davis added a comment -

          Putting this up for integration. Master only.

          Show
          Andrew Davis added a comment - Putting this up for integration. Master only.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Adding the docs_required tag as far as this (the change and potential breakage) should be commented, in release notes or Forum FAQ or wherever.

          Show
          Eloy Lafuente (stronk7) added a comment - Adding the docs_required tag as far as this (the change and potential breakage) should be commented, in release notes or Forum FAQ or wherever.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Uhm,

          just saw that you're using the hash() function. While it's installed by default since php 5.1.x, it may be opted-out. So I've created MDL-31707 about that.

          It does not prevent integration of this, though... so doing.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Uhm, just saw that you're using the hash() function. While it's installed by default since php 5.1.x, it may be opted-out. So I've created MDL-31707 about that. It does not prevent integration of this, though... so doing. Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated, thanks!
          Hide
          Rajesh Taneja added a comment -

          Works Great,

          Thanks for fixing this Andrew. Just wondering, why it's an improvement and not a bug. It might be nice to backport this to 22 and 21

          Show
          Rajesh Taneja added a comment - Works Great, Thanks for fixing this Andrew. Just wondering, why it's an improvement and not a bug. It might be nice to backport this to 22 and 21
          Hide
          Rajesh Taneja added a comment -

          Sorry, forgot to mention about docblock.
          New function forum_get_email_message_id, doesn't have a docblock.

          Do we fail test, if functionality works but docblock or space etc. is an issue?

          Show
          Rajesh Taneja added a comment - Sorry, forgot to mention about docblock. New function forum_get_email_message_id, doesn't have a docblock. Do we fail test, if functionality works but docblock or space etc. is an issue?
          Hide
          Andrew Davis added a comment - - edited

          Read the comments for the reason this wasn't backported to 2.2 and 2.1

          No, you shouldn't fail the test for docs type issues. It should have been picked up by me or the peer reviewer. Ill open a new MDL for that.

          update: MDL-31739

          Show
          Andrew Davis added a comment - - edited Read the comments for the reason this wasn't backported to 2.2 and 2.1 No, you shouldn't fail the test for docs type issues. It should have been picked up by me or the peer reviewer. Ill open a new MDL for that. update: MDL-31739
          Hide
          Rajesh Taneja added a comment -

          Hello Andrew,

          I saw the comments, but the problem was reported for 2.1 and 2.2. Just wondering, if we plan to have anything done for stable branches (As, I can't see any related bug for solving this issue on stable branches.)

          Show
          Rajesh Taneja added a comment - Hello Andrew, I saw the comments, but the problem was reported for 2.1 and 2.2. Just wondering, if we plan to have anything done for stable branches (As, I can't see any related bug for solving this issue on stable branches.)
          Hide
          Andrew Davis added a comment -

          No. There wont be a fix for this in the stable branches as fixing this would break up everyone's existing email threads.

          Show
          Andrew Davis added a comment - No. There wont be a fix for this in the stable branches as fixing this would break up everyone's existing email threads.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Some changes to Moodle should be milestones in the project by themselves.

          This is not the case and your fix is not so important, but your collaboration is highly appreciated, thanks!

          Closing as fixed, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Some changes to Moodle should be milestones in the project by themselves. This is not the case and your fix is not so important, but your collaboration is highly appreciated, thanks! Closing as fixed, ciao
          Hide
          Sara Valla added a comment -

          I've got the same problems with email notifications from forum.
          Cron runs correctly, shows user processing and indicates that "n users were sent post idnumber" but only one user receives email notification.

          I upgraded to Moodle 2.2.2+ (Build: 20120323) but that didn't solve the issue.
          I therefore modified file ../mod/forum/lib.php as suggested, but I didn't solve the problem.

          Any suggestions?

          Show
          Sara Valla added a comment - I've got the same problems with email notifications from forum. Cron runs correctly, shows user processing and indicates that "n users were sent post idnumber" but only one user receives email notification. I upgraded to Moodle 2.2.2+ (Build: 20120323) but that didn't solve the issue. I therefore modified file ../mod/forum/lib.php as suggested, but I didn't solve the problem. Any suggestions?
          Hide
          Dominik Jeni added a comment -

          Is the email domain of all recipients the same? If so, I'd suggest to look into the mail server's log and check out whether the mails arrive or not (or if they are rejected due to a special reason) ...

          Show
          Dominik Jeni added a comment - Is the email domain of all recipients the same? If so, I'd suggest to look into the mail server's log and check out whether the mails arrive or not (or if they are rejected due to a special reason) ...
          Hide
          Sara Valla added a comment -

          No domain was not the same. I looked carefully into the mail server's log and verified that using sendmail on Moodle without a smart smtp I did not realise that that server had been closed by firewall on port 25. Now I used smart smtp in my mail program to pass through the problem. Thanks!

          Show
          Sara Valla added a comment - No domain was not the same. I looked carefully into the mail server's log and verified that using sendmail on Moodle without a smart smtp I did not realise that that server had been closed by firewall on port 25. Now I used smart smtp in my mail program to pass through the problem. Thanks!
          Hide
          Helen Foster added a comment -

          Please can anyone advise on what exactly should be added to the user docs about this issue, and also the release notes. I can't find any mention of it in the Using Moodle forums (yet!)

          Show
          Helen Foster added a comment - Please can anyone advise on what exactly should be added to the user docs about this issue, and also the release notes. I can't find any mention of it in the Using Moodle forums (yet!)
          Hide
          Mary Cooch added a comment -

          (Housekeeping)10 months on - just asking again if this needs documenting and if so, how, because otherwise perhaps the docs_required label can be removed?

          Show
          Mary Cooch added a comment - (Housekeeping)10 months on - just asking again if this needs documenting and if so, how, because otherwise perhaps the docs_required label can be removed?
          Hide
          Aparup Banerjee added a comment -

          i think it was mainly to say that this was fixed in the release (2.3) and that it wasn't fixed in older ones and was still an issue in older ones just to be aware of (borked email sending functionality described in description). This info is now a bit obsolete now though, nothing to do now imo.

          Show
          Aparup Banerjee added a comment - i think it was mainly to say that this was fixed in the release (2.3) and that it wasn't fixed in older ones and was still an issue in older ones just to be aware of (borked email sending functionality described in description). This info is now a bit obsolete now though, nothing to do now imo.

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: