Summary: | Koha::Logger does not correctly handle instantiation errors | ||
---|---|---|---|
Product: | Koha | Reporter: | Jonathan Druart <jonathan.druart> |
Component: | Architecture, internals, and plumbing | Assignee: | Jonathan Druart <jonathan.druart> |
Status: | CLOSED INVALID | QA Contact: | Marcel de Rooy <m.de.rooy> |
Severity: | normal | ||
Priority: | P5 - low | CC: | jweaver, m.de.rooy |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: | http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14597 | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Bug Depends on: | 14167 | ||
Bug Blocks: | |||
Attachments: |
Bug 14906: Koha::Logger - handle permission issues
[SIGNED-OFF] Bug 14906: Koha::Logger - handle permission issues |
Description
Jonathan Druart
2015-09-28 09:08:30 UTC
Created attachment 42904 [details] [review] Bug 14906: Koha::Logger - handle permission issues Koha::Logger should log in the regular log files if the paths are not correctly set (does not exist, no permission to write, etc.). Test plan: Set a path without write permission for the intranet in the log4perl configuration file. The one for opac should be correct. Try to write 1 line in the OPAC file. Without this patch, nothing is logged, and nothing is warned in the regular Koha log file. With this patch, the line in the OPAC log file will be appened and the Koha log file will have a line with a permission issue. Created attachment 42918 [details] [review] [SIGNED-OFF] Bug 14906: Koha::Logger - handle permission issues Koha::Logger should log in the regular log files if the paths are not correctly set (does not exist, no permission to write, etc.). Test plan: Set a path without write permission for the intranet in the log4perl configuration file. The one for opac should be correct. Try to write 1 line in the OPAC file. Without this patch, nothing is logged, and nothing is warned in the regular Koha log file. With this patch, the line in the OPAC log file will be appened and the Koha log file will have a line with a permission issue. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> First chmod your opac logfile to 400. Second, try this test please: use Koha::Logger; my $logger = Koha::Logger->get({ interface => 'opac' }); $logger->warn('L4'); Without this patch, you will have no warning since the log is not writable. With your patch, the program will crash (on the following call on line 132 after calling _check_conf): Log::Log4perl->init_once($conf); The intention of the code in Logger.pm was just to prevent such a crash. This patch reintroduces it. Failed QA (In reply to Jonathan Druart from comment #0) > Also, the _check_conf subroutine will fail (return undef) if at least 1 path > is not correctly defined, which is not necessary: Yes it is. I get Logger configuration error - Wrong permission for file /home/koha/var/log/opac-error.log at t.pl line 289. Can't open /home/koha/var/log/opac-error.log (Permission denied) at /usr/share/perl5/Log/Log4perl/Appender/File.pm line 124. Forget last comment. Marcel, what do you suggest? (In reply to Jonathan Druart from comment #6) > Forget last comment. > > Marcel, what do you suggest? If one of those two logfiles (opac, intranet) is not writable, log4perl will crash nicely. The goal was to prevent such a crash. So I would suggest to add the warnings about the unwritable logfile and still return undef. So do not call log4perl and no additional log message. Note that this situation should be quite unusual. Probably both files are writable or both are not. May I ask why this was resolved invalid? It's a rather surprising silent failure that I had to find with a debugger, and anything besides silence would be preferable. Yes, this is unlikely in production, but it would still be nice to fix. (In reply to Jesse Weaver from comment #8) > May I ask why this was resolved invalid? It's a rather surprising silent > failure that I had to find with a debugger, and anything besides silence > would be preferable. Yes, this is unlikely in production, but it would still > be nice to fix. Please check the earlier comments. The solution proposed was not adequate. On itself no reason to close the report, I agree. If you have a better solution, please reopen. |