-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Check whether a message is an auto reply based on configuration #12
base: master
Are you sure you want to change the base?
Conversation
68803f2
to
b2bd01f
Compare
b2bd01f
to
af5fc9e
Compare
So will auto replies still show up in itop and we can filter on the notification/triggers to prevent mail ping-pongs? Or will these E-Mails never show up in iTop (be suppressed) ? |
In detail this PR will just add a new method which will allow to detect auto generated / replied mails. |
Hello Lars, I'll check this PR on friday, would you update/rebase you branch on master in the meantime? Thanks |
Might also be worth checking out the open issue on my fork: jbostoen/itop-jb-mail-to-ticket-automation-v2#21 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will be reviewed this afternoon
classes/emailmessage.class.inc.php
Outdated
foreach ($aAutoReplyHeaderPatterns as $sHeader => $sPattern) { | ||
if(array_key_exists(strtolower($sHeader), $this->aHeaders)) | ||
{ | ||
IssueLog::Info("Header \"$sHeader\" exists with the value: \"" . $this->aHeaders[strtolower($sHeader)] . "\""); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IssueLog::Info("Header \"$sHeader\" exists with the value: \"" . $this->aHeaders[strtolower($sHeader)] . "\""); | |
IssueLog::Debug("Header \"$sHeader\" exists with the value: \"" . $this->aHeaders[strtolower($sHeader)] . "\""); |
classes/emailmessage.class.inc.php
Outdated
if (preg_match($sPattern, $this->aHeaders[strtolower($sHeader)])) | ||
{ | ||
// Current mail is an auto reply | ||
IssueLog::Info("Given mail is an autoreply!"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IssueLog::Info("Given mail is an autoreply!"); | |
IssueLog::Debug("Given mail is an autoreply!"); |
Hello Lars, We reviewed this PR today, seems like the RFC was updated. We wonder the header value, you only check for "auto-replied", how about "auto-generated"; wasn't it relevant in your use cases? |
d0fccfa
to
0ce8062
Compare
Hi Guillaume, the idea of this PR(s) was to prevent email "ping pong" between iTop and a mail server. "Ping pong" means: iTop is sending a mail (i.e. "Hey you are an unknown caller"), Mail server ist sending an auto-reply (i.e. "Hey this is an auto-reply to your message"), iTop is sending again ("Hey your are an unknown caller"), and so on and so on. From my point of view this "endless loop" should happen with auto-replies but not auto generated mails, since they are generated not in a reply to a mail from iTop. (correct me if I am wrong). There might be a good reason to handle some auto generated mails in iTop, or at least handle them different then auto replies. You could follow my explanation? |
Thanks for the feedback, it is kind of how we understood it as well but as we didn't collect much experience on that matter yet, we didn't know if some mail servers would flag an out of office response as "auto-generated" for example. So we wanted to know what you encountered in your experience. So it's all good for us (technically), the PRs will be functionally reviewed next week. :) |
Accepted during functional once the following modification has been made: Empty the default We don't have enough experience yet on the headers and their values when it comes to the different mail servers out there (Exchange, GMail, ...). Anyhhow, the feature will be documented and people will be able to activate it. |
I would suggest to only empty it in the code, but keep in the module. That way it will not change behaviour for existing users, but it will get enabled for new users.. |
$aAutoReplyHeaderPatterns = array( | ||
'auto-submitted' => '/^auto-replied.*$/i', | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$aAutoReplyHeaderPatterns = array( | |
'auto-submitted' => '/^auto-replied.*$/i', | |
); | |
$aAutoReplyHeaderPatterns = []; |
With the new method it is possible to detect auto replies or auto generated mail by their header (see also rfc3834)
This would be pretty useful in order to prevent sending automatic mails in reply to such messages (loop prevention).
See also corresponding PR in itop-standard-email-synchro.