[Bug 11188] New: Make gather_print_notices.pl die on failed open()
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11188 Bug ID: 11188 Summary: Make gather_print_notices.pl die on failed open() Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: minor Priority: P5 - low Component: Command-line Utilities Assignee: magnus@enger.priv.no Reporter: magnus@enger.priv.no QA Contact: testopia@bugs.koha-community.org Currently, gather_print_notices.pl tries to open a file to write notices to, but it does not check that the open() succeeded. This can result in the following error error, if you point at a location you do not have write access to: print() on closed filehandle $OUTPUT at /usr/share/koha/bin/cronjobs/gather_print_notices.pl line 129. But the script proceeds on its merry way, and marks the "pending" messages it found as "sent", which means you can't re-run the script with another location and gather up the notices you meant to gather up. I'll propose to replace the current "open(...)" with an "open(...) or die()". (I'll probably do a patch for this during training on Monday, so please don't fix it before that :-) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11188 Magnus Enger <magnus@enger.priv.no> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11188 --- Comment #1 from Magnus Enger <magnus@enger.priv.no> --- Created attachment 23181 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23181&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11188 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11188 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #23181|0 |1 is obsolete| | --- Comment #2 from Chris Cormack <chris@bigballofwax.co.nz> --- Created attachment 23849 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23849&action=edit 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 <chris@bigballofwax.co.nz> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11188 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11188 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #23849|0 |1 is obsolete| | --- Comment #3 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 23855 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23855&action=edit [PASSED QA] 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 <chris@bigballofwax.co.nz> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Passes all tests and QA script. Works as described. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11188 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |gmcharlt@gmail.com --- Comment #4 from Galen Charlton <gmcharlt@gmail.com> --- Pushed to master. Thanks, Magnus! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11188 Fridolin SOMERS <fridolyn.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable CC| |fridolyn.somers@biblibre.co | |m --- Comment #5 from Fridolin SOMERS <fridolyn.somers@biblibre.com> --- Patch pushed to 3.14.x, will be in 3.14.3. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11188 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com --- Comment #6 from Tomás Cohen Arazi <tomascohen@gmail.com> --- This patch has been pushed to 3.12.x, will be in 3.12.11. Thanks Magnus! -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org