Mailcomment with mailhandler passthru turned off intercepts all emails

So, if you you look at the Mailhandler settings for these you have something like mailbox1@example.com configured in Mailhandler to use MailComment authentication.

You also have mailbox2@example.com which is currently being authenticated by mailcomment, but you wish it weren't. What is the authentication setting on that mailbox in mailhandler?

The issue as described is also occuring for me. I independently read the code trying to find the problem and came to the same conclusion as Chadwick in #3.

Is there any reason to not check that the $mailbox the message is from is one that this module should operate on at the start of mailcomment_mailhandler()?

@ejort could you try out the idea in #4 and perhaps turn it into a patch - http://drupal.org/patch/create ?

If that works for you I'd be happy to review the setup.

Here is my understanding of the bug:

0. Set mailcomment to not passthru to mailhandler
1. Create a mailhandler mailbox that is mailcomment enabled
2. Create a mailhandler mailbox that is NOT mailcomment enabled
3. Send a mail to mailbox from step 2
4. See that it is rejected by mailcomment

Expected results:
In step 4 it should not be processed at all by mailcomment.

The reason why there's a setting is because of the explanation here http://drupal.org/cvs?commit=192578 ... basically, if you're *only* using mailcomment on your site it prevents mailhandler from ever doing anything else if information is accidentally stripped out of the email meant to be processed by mailcomment. It creates the possibility to bail-out, or not, and leaves it up to the site admin to decide.

The reason why it cannot just check whether the mailbox is a mailcomment mailbox or not is because if the mailbox is a catchall address it's not possible to tell whether or not the email is a mailcomment email address.

For example, if your catchall is *@example.com which you use with mailcomment and you also have an info@example.com address which you use with just mailhandler, if someone sends to foo@example.com you do not know for sure whether they're sending to the catchall or not.

That said, we could assume that if the email is being processed by mailhandler/mailcomment and its full "to" address (foo@example.com) does not match any mailhandler address but its domain (example.com) matches a mailhandler address which is mailcomment enabled, that mailcomment should handle it. Otherwise, mailcomment should not handle it and nor should mailhandler.

As for the order of:

  }
  // If we reach here, there has been an error. Check error code or send a generic one.
  // This part doesn't return a node so it won't be further processed by mailhandler
  if (count($node->mailcomment['params']) > 1 && $header->toaddress == $mbox_name) {
    mailcomment_error($node->mailcomment['params']['error'], $node, $header);
  }
  // If there are no params, return the node back to mailhandler, regardless of to address.
  else {
    if (variable_get('mailcomment_passthru', 0) == 1) {
      return $node;
    }
    else {
      mailcomment_error(MAILCOMMENT_ERROR_PARAMS, $node, $header);
    }

The sequence is important:

* First make sure params existed but error found, if so, send error
* If no params, must not be for mailcomment afterall, so pass through
* If passthru is not enabled then throw an error since mailcomment could not handle the incoming message and mailhandler is not supposed to handle it.

Maybe there's a nicer way to write this though.

Finally, $mbox_name should actually be $matched. $mbox_name is old and removed. This is in the patch.

It would be great to get review of this to make sure I understood the issue completely and that the patch actually works.

Patch in #7 had var names mixed up. Here's a new patch

I applied the patch but nothing happened

Can you provide you complete configuration for Mailhandler box1, Mail handler box 2 and Mailcomment setting...

this patch did not work for me either. (the patch in #8)

Sorry, Mail Comment 6.x-1.x is no longer supported. Please upgrade to 6.x-2.x and re-open if still an issue. Thanks!