From b068259521ebea158e8b664cfbab46d5a65495ff Mon Sep 17 00:00:00 2001
From: Tomas Cohen Arazi <tomascohen@theke.io>
Date: Thu, 5 Apr 2018 16:29:23 -0300
Subject: [PATCH] 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>
---
 C4/Context.pm                                 |  5 ++++
 about.pl                                      | 28 +++++++++++++++++--
 .../intranet-tmpl/prog/en/modules/about.tt    | 20 +++++++++++--
 t/timezones.t                                 | 11 +++++++-
 4 files changed, 57 insertions(+), 7 deletions(-)

diff --git a/C4/Context.pm b/C4/Context.pm
index 07e81854bd..7ce91b6b35 100644
--- 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 91292a5a79..7ea8acd073 100755
--- 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 b47423385a..e109eb0e33 100644
--- 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 c5463dab90..ee64438852 100644
--- 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';
-- 
2.39.5