[opendmarc-users] suggested patch for opendmarc-reports-1.2.0

[Barney Wolff]

I had some troubles with opendmarc-reports, which I first installed as
version 1.1.3, recently updated to 1.2.0.

1.  Size limit specification in a mailto URI is not noticed, resulting in
an invalid report destination.

2.  Postfix reacts very nastily to that invalid destination, closing the
SMTP connection.  That causes all subsequent domain reports to fail.

3.  When there is trouble delivering a report, both the zipfile and the
xml file are unlinked, even though keepfiles is set and even though there
may be other usable report destinations.

4.  If the last-update field update fails (unlikely :) the zipfile is left
around.

So I modified opendmarc-reports to avoid these issues, by establishing a
new SMTP connection for each report.  Perhaps overkill, but as the connection
is usually to localhost, I claim the cost is small.  Also, waiting to unlink
the files until done with a domain.

Since AOL has now migrated to gzip, I patched opendmarc-reports to
do it too.

Here's the patch:
#########################################################################
--- opendmarc-reports.1.2.0	2014-03-16 21:56:04.000000000 -0400
+++ opendmarc-reports	2014-03-30 17:48:03.000000000 -0400
@@ -8,6 +8,8 @@
 ### Setup
 ###
 
+$ENV{'PATH'} = '/bin:/usr/bin:/usr/sbin:/usr/local/bin:/usr/local/sbin';
+
 use strict;
 use warnings;
 
@@ -19,7 +21,6 @@
 use Net::Domain qw(hostfqdn hostdomain);
 use Getopt::Long;
 use IO::Handle;
-use IO::Compress::Zip qw(zip);
 use POSIX;
 use MIME::Base64;
 use Net::SMTP;
@@ -346,15 +347,6 @@
 # 	-- update "last sent" timestamp
 #
 
-$smtp = Net::SMTP->new($smtp_server,
-                       'Port' => $smtp_port,
-                       'Helo' => hostfqdn());
-if (!defined($smtp))
-{
-	print STDERR "$progname: open SMTP server $smtp_server:$smtp_port failed\n";
-	exit(1);
-}
-
 foreach (@$domainset)
 {
 	$domain = $_->[0];
@@ -453,7 +445,7 @@
 
 	# construct the temporary file
 	$repfile = $repdom . "!" . $domain . "!" . $lastsent . "!" . time() . ".xml";
-	$zipfile = $repdom . "!" . $domain . "!" . $lastsent . "!" . time() . ".zip";
+	$zipfile = $repfile . '.gz';
 	if (!open($tmpout, ">", $repfile))
 	{
 		print STDERR "$progname: can't create report file for domain $domain\n";
@@ -718,29 +710,18 @@
 		next;
 	}
 
-	# zip the report
-	if (!zip [ $repfile ] => $zipfile)
-	{
-		print STDERR "$progname: can't zip report for domain $domain: $!\n";
-		next;
-	}
-
-	if ($keepfiles)
-	{
-		print STDERR "$progname: keeping report file \"$repfile\"\n";
-	}
-
 	# decode the URI
 	@repuris = split(',', $repuri);
 
 	for $repuri (@repuris)
 	{
+		my ($baseUri,$sizeLim) = split('!',$repuri);
+		if ($sizeLim) {$repuri = $baseUri;}	# TODO check the size limit!
 		$uri = URI->new($repuri);
 		if (!defined($uri))
 		{
-			print STDERR "$progname: can't parse reporting URI for domain $domain\n";
-			unlink($zipfile);
-			unlink($repfile);
+			print STDERR "$progname: can't parse reporting URI $repuri for domain $domain\n";
+			# don't unlink here, there may be other, valid URIs
 			next;
 		}
 
@@ -758,8 +739,7 @@
 				print STDERR "$progname: unknown URI scheme in '$repuri' for domain $domain\n";
 			}
 
-			unlink($zipfile);
-			unlink($repfile);
+			# don't unlink here, there may be other, valid URIs
 			next;
 		}
 		# send/post report
@@ -769,14 +749,22 @@
 			my $datestr;
 			my $report_id;
 
-			if (!open($zipin, $zipfile))
+			if (!open($zipin, "gzip <$repfile|"))
 			{
 				print STDERR "$progname: can't read zipped report for $domain: $!\n";
-				unlink($zipfile);
-				unlink($repfile);
-				next;
+				# we still might be keeping the repfile
+				last;	# break out of repuri loop
 			}
 		
+			$smtp = Net::SMTP->new($smtp_server,
+						   'Port' => $smtp_port,
+						   'Helo' => hostfqdn());
+			if (!defined($smtp))
+			{
+				print STDERR "$progname: open SMTP server $smtp_server:$smtp_port failed\n";
+				last;		# just bust out of repuri loop in case keepfiles
+			}
+
 			$repdest = $uri->opaque;
 
 			$boundary = "report_section";
@@ -805,7 +793,7 @@
 			$mailout .= "generated at " . localtime() . "\n";
 			$mailout .= "\n";
 			$mailout .= "--$boundary\n";
-			$mailout .= "Content-Type: application/zip\n";
+			$mailout .= "Content-Type: application/gzip\n";
 			$mailout .= "Content-Disposition: attachment; filename=\"$zipfile\"\n";
 			$mailout .= "Content-Transfer-Encoding: base64\n";
 			$mailout .= "\n";
@@ -838,19 +826,26 @@
 				print STDERR "$progname: $smtpstatus report for $domain to $repdest ($answer)\n";
 			}
 
-			$smtp->reset();
+			$smtp->quit();
 
 			close($zipin);
 		}
 		else
 		{
 			print STDERR "$progname: unsupported reporting URI scheme " . $uri->scheme . " for domain $domain\n";
-			unlink($zipfile);
-			unlink($repfile);
+			# don't unlink here, there may be other, valid URIs
 			next;
 		}
 	}
 
+	if (!$keepfiles)
+	{
+		unlink($repfile);
+	}
+	else {
+		print STDERR "$progname: keeping report file \"$repfile\"\n";
+	}
+
 	# update "last sent" timestamp
 	if ($doupdate)
 	{
@@ -863,16 +858,8 @@
 			exit(1);
 		}
 	}
-
-	unlink($zipfile);
-	if (!$keepfiles)
-	{
-		unlink($repfile);
-	}
 }
 
-$smtp->quit();
-
 #
 # all done!
 #
#########################################################################
--