From 6b09a1c19558d58fd3d1d323f89416455fda5706 Mon Sep 17 00:00:00 2001 From: Magnus Enger Date: Sat, 23 Nov 2013 22:57:03 +0100 Subject: [PATCH] Bug 11188 - Make gather_print_notices.pl die on failed open() Problem: If you tell gather_print_notices.pl to write output to a location you do not have write access to, it will silently fail to write the data, but still mark unsent messages as sent. Solution: This patch adds two lines of defense: 1. Check that the location given for the output is writable 2. use "open() or die" instead of just "open()" when writing the output The first measure should catch most of the potential errors, but I guess a directory can be writable, but the open() still can fail because the disk is full or something similar. To test: - Make sure you have some unsent messages in the message_queue table, that do not have an email adress - Apply the patch - Run the script, pointing at a location you do not have access to write to. Check that the script exits with an appropriate error message, and that the unsent messages are still unsent. Do this both with and without the -s option. - To fake passing the first line of defence, comment out line 62 and put this in instead: if ( !$output_directory || !-d $output_directory ) { - Run the script again as above, check you get an appropriate error and that the message queue is not touched - Reset line 62 to how it was - Run the script against a directory you do have access to write to and check that output is produced as expected and that messages are marked as sent - Sign off Signed-off-by: Chris Cormack Signed-off-by: Katrin Fischer Passes all tests and QA script. Works as described. Signed-off-by: Galen Charlton (cherry picked from commit e8a24c35f64cb461a80f768b505bbf16c49dfeff) Signed-off-by: Fridolin Somers (cherry picked from commit ac5a13eaa34b2377d39b362ed0326f200b5365e5) Signed-off-by: Tomas Cohen Arazi --- misc/cronjobs/gather_print_notices.pl | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/misc/cronjobs/gather_print_notices.pl b/misc/cronjobs/gather_print_notices.pl index 2d0555750e..248716e339 100755 --- a/misc/cronjobs/gather_print_notices.pl +++ b/misc/cronjobs/gather_print_notices.pl @@ -59,9 +59,9 @@ usage(0) if ($help); my $output_directory = $ARGV[0]; -if ( !$output_directory || !-d $output_directory ) { +if ( !$output_directory || !-d $output_directory || !-w $output_directory ) { print STDERR -"Error: You must specify a valid directory to dump the print notices in.\n"; +"Error: You must specify a valid and writeable directory to dump the print notices in.\n"; usage(1); } @@ -87,9 +87,10 @@ if ($split) { foreach my $branchcode ( keys %messages_by_branch ) { my @messages = @{ $messages_by_branch{$branchcode} }; - open $OUTPUT, '>', - File::Spec->catdir( $output_directory, + my $output_file = File::Spec->catdir( $output_directory, "holdnotices-" . $today->output('iso') . "-$branchcode.html" ); + open $OUTPUT, '>', $output_file + or die "Could not open $output_file: $!"; my $template = C4::Templates::gettemplate( 'batch/print-notices.tmpl', 'intranet', @@ -112,9 +113,11 @@ if ($split) { } } else { - open $OUTPUT, '>', - File::Spec->catdir( $output_directory, + my $output_file = File::Spec->catdir( $output_directory, "holdnotices-" . $today->output('iso') . ".html" ); + open $OUTPUT, '>', $output_file + or die "Could not open $output_file: $!"; + my $template = C4::Templates::gettemplate( 'batch/print-notices.tmpl', 'intranet', -- 2.39.5