IMAP - keeping mailing list up-to-date

[Ed_Koren]

Hi there,

I was wondering if anyone could have a look at a small script I wrote that I want to use to keep a mailing list up-to-date. The aim is to periodically execute it as a cron job, picking out the returned mail messages and deleting the original recipient from the mailing list. I am certainly not an advanced PHP programmer and therefore I am not sure if the script is reliable - testing it has been successful though.

// Retrieve headers from mailbox
$headers = @imap_headers($m_connect)
or die("Couldn't retrieve headers");
// Get number of messages
$num_emails = sizeof($headers);
// Loop through headers to locate Returned Mail messages
for($i = 1; $i < $num_emails+1; $i++) {
$mailHeader = @imap_headerinfo($m_connect, $i);
$from = $mailHeader->from[0];
if ($from->mailbox == "MAILER-DAEMON") {
// If returned mail message, get body of message
$b = imap_body($m_connect, $i);
// Remove occurrences of own domain from body
$b = ereg_replace("@mydomain.com","",$b);
// Check if it's a permanent error - not sure if this is a reliable way
if (ereg("permanent", $b) | ereg("fatal", $b)) {
// Fish for failed address
ereg('[a-zA-Z0-9_.-]+@[a-zA-Z0-9-]+.[a-zA-Z0-9-.]', $b, $regs);
$addr = $regs[0];
// Delete address from db
$sql = "DELETE FROM mailinglist WHERE ml_addr = '$addr'";
$result = mysql_query($sql, $db);
// Mark for deletion
imap_delete ($m_connect, $i);
}
}
// Delete all messages marked for deletion
imap_expunge ($m_connect);

}
imap_close($m_connect);

Any suggestions to improve it would be very much appreciated.

Ed

[Moonglobe]

try wrapping it in

 tags, then we'll have a look

[Ed_Koren]

Here it is including php tags...first time posting here, never had a look at the posting guidelines. My apologies for that.

// Retrieve headers from mailbox
$headers = @imap_headers($m_connect);

// Get number of messages
$num_emails = sizeof($headers); 

// Loop through headers to locate Returned mail messages
for($i = 1; $i < $num_emails+1; $i++) { 
$mailHeader = @imap_headerinfo($m_connect, $i);
$from = $mailHeader->from[0];
if ($from->mailbox == "MAILER-DAEMON") {

// If returned mail message, get body of message
$b = imap_body($m_connect, $i);

// Remove occurrences of own domain from body
$b = ereg_replace("@mydomain.com","",$b);

// Check if it's a permanent error
if (ereg("permanent", $b) | ereg("fatal", $b)) {

// Fish for failed address
ereg('[a-zA-Z0-9_\.\-]+@[a-zA-Z0-9\-]+\.[a-zA-Z0-9\-\.]*', $b, $regs);
$addr = $regs[0];

// Delete address from db
$sql = "DELETE * FROM mailinglist WHERE ml_addr = '$addr'";
$result = mysql_query($sql, $db);

// Mark for deletion	
imap_delete ($m_connect, $i);
}
}

// Delete all messages marked for deletion
imap_expunge ($m_connect);

}
imap_close($m_connect);

[LordShryku]

I think he meant the PHP VB Code tags....
Check this page

[Ed_Koren]

Thanks for that, I must appear brainless ;-) Makes the code much more readable indeed. I fixed it in the above post so it would be great if someone could actually comment on the script.

Ed

[Weedpacket]

I guess I should brush up on what IMAP actually does one of these millennia... Too many FLEAs...

I can suggest one improvement:

$b = ereg_replace("@mydomain.com","",$b);

is in a sense incorrect: since . will match any character, the above expression will happily turn "@mydomainacom.com" into ".com". Okay, a rather contrived example, but still, the principle's there.

Fortunately, while the . could just be escaped (i.e., use .), the fact that there isn't anything particular regexy about that expression and is actually just a string means that

$b = str_replace("@mydomain.com","",$b);

would see a marked performance improvement.

Similarly, using strpos instead of ereg to detect plain substrings in longer strings would also be an efficiency gain.

The only other tiddly little change I can see would be turning

for($i = 1; $i < $num_emails+1; $i++)

into

for($i = 1; $i <= $num_emails; $i++)

....saves one addition per iteration.

(And is it "DELETE * FROM" or just "DELETE FROM"?)

All pretty minor, but hey - I get paid for being pedantic.

Phew, and I managed to get through all that without saying "use preg_match() instead!" ..... damn.

[Ed_Koren]

Originally posted by Weedpacket

(And is it "DELETE * FROM" or just "DELETE FROM"?)

Yes, it is "DELETE FROM" indeed ;-) Thanks for the input, I shall take it into account when rewriting the script. I expected a comment on

if (ereg("permanent", $b) | ereg("fatal", $b))

because it seems too simple, although it seems to work just fine.

Anyway, thanks.

Ed

[Moonglobe]

ya that works, but as Weed said, you should replace tem with strpos() calls for a pretty hefty performance gain.