Bug 18674: TZ error handling

This patch adds C4::Context->timezone bad timezone handling.
The calculated 'effective' timezone is tested with the right tool and a
fallback to 'local' is added. A warning is printed in the logs.

A test for this is added to about.pl too, along with the right warning
messages in case of problems.

Tests are added for both invalid TZ and to make sure the warning is
raised.

To test:
- Apply the patch
- Run:
  $ kshell
 k$ prove t/timezones.t
=> SUCCESS: All tests pass

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

diff --git a/C4/Context.pm b/C4/Context.pm
index 07e8185..7ce91b6 100644 (file)
--- a/C4/Context.pm
+++ b/C4/Context.pm
@@ -97,6 +97,7 @@ use POSIX ();
 use DateTime::TimeZone;
 use Module::Load::Conditional qw(can_load);
 use Carp;
+use DateTime::TimeZone;
 
 use C4::Boolean;
 use C4::Debug;
@@ -981,6 +982,10 @@ sub timezone {
     my $self = shift;
 
     my $timezone = C4::Context->config('timezone') || $ENV{TZ} || 'local';
+    if ( !DateTime::TimeZone->is_valid_name( $timezone ) ) {
+        warn "Invalid timezone in koha-conf.xml ($timezone)";
+        $timezone = 'local';
+    }
 
     return $timezone;
 }

diff --git a/about.pl b/about.pl
index 91292a5..7ea8acd 100755 (executable)
--- a/about.pl
+++ b/about.pl
@@ -23,6 +23,7 @@
 use Modern::Perl;
 
 use CGI qw ( -utf8 );
+use DateTime::TimeZone;
 use List::MoreUtils qw/ any /;
 use LWP::Simple;
 use XML::Simple;
@@ -61,10 +62,31 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
     }
 );
 
+my $config_timezone = C4::Context->config('timezone');
+my $config_invalid  = !DateTime::TimeZone->is_valid_name( $config_timezone );
+my $env_timezone    = $ENV{TZ};
+my $env_invalid     = !DateTime::TimeZone->is_valid_name( $env_timezone );
+my $actual_bad_tz_fallback = 0;
+
+if ( $config_timezone ne '' &&
+     $config_invalid ) {
+    # Bad config
+    $actual_bad_tz_fallback = 1;
+}
+elsif ( $config_timezone eq '' &&
+        $env_timezone    ne '' &&
+        $env_invalid ) {
+    # No config, but bad ENV{TZ}
+    $actual_bad_tz_fallback = 1;
+}
+
 my $time_zone = {
-    actual      => C4::Context->timezone(),
-    config      => C4::Context->config('timezone'),
-    environment => $ENV{TZ},
+    actual                 => C4::Context->timezone(),
+    actual_bad_tz_fallback => $actual_bad_tz_fallback,
+    config                 => $config_timezone,
+    config_invalid         => $config_invalid,
+    environment            => $env_timezone,
+    environment_invalid    => $env_invalid
 };
 $template->param( 'time_zone' => $time_zone );
 

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/about.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/about.tt
index b474233..e109eb0 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/about.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/about.tt
@@ -112,10 +112,24 @@
             <tr><th scope="row"><b>Warning</b> </th><td>Error message from Zebra: [% ( errZebraConnection ) %] </td></tr>
             [% END %]
             <tr>
+              [% timezone_config_class = (time_zone.config_invalid) ? 'status_warn' : '' %]
+              [% timezone_env_class    = (time_zone.env_invalid)    ? 'status_warn' : '' %]
               <th scope="row">Time zone: </th>
-              <td>Used: <span class="status_ok">[% time_zone.actual %]</span> |
-                  Config: [% IF time_zone.config != '' %][% time_zone.config %][% ELSE %]<span>Undefined</span>[% END %] |
-                  ENV: [% IF time_zone.environment != '' %][% time_zone.environment %][% ELSE %]<span>Undefined</span>[% END %]
+              <td>Used: <span>[% time_zone.actual %]</span>
+                          [% IF time_zone.actual_bad_tz_fallback %]
+                             <span>(This is a fallback value due to a bad configuration)</span>
+                          [% END %]
+                           |
+                  Config: [% IF time_zone.config != '' %]
+                            <span class="[% timezone_config_class %]">[% time_zone.config %]</span>
+                          [% ELSE %]
+                            <span>Undefined</span>
+                          [% END %] |
+                  Environment (TZ):  [% IF time_zone.environment != '' %]
+                          <span class="[% timezone_env_class %]">[% time_zone.environment %]</span>
+                        [% ELSE %]
+                          <span>Undefined</span>
+                        [% END %]
               </td>
             </tr>
         </table>

diff --git a/t/timezones.t b/t/timezones.t
index c5463da..ee64438 100644 (file)
--- a/t/timezones.t
+++ b/t/timezones.t
@@ -2,9 +2,12 @@ use Modern::Perl;
 
 use C4::Context;
 
-use Test::More tests => 3;
+use Test::More tests => 5;
+use Test::Warn;
 use t::lib::Mocks;
 
+use DateTime::TimeZone;
+
 $ENV{TZ} = q{};
 t::lib::Mocks::mock_config( 'timezone', q{} );
 is( C4::Context->timezone, 'local',
@@ -23,3 +26,9 @@ is(
     'Antarctica/South_Pole',
     'Got correct timezone using config, overrides env'
 );
+
+t::lib::Mocks::mock_config( 'timezone', 'Your/Timezone' );
+warning_is {
+    is( C4::Context->timezone, 'local', 'Invalid timezone falls back to local' ); }
+    'Invalid timezone in koha-conf.xml (Your/Timezone)',
+    'Invalid timezone raises a warning';