0000370: rbs-schedule.php fails to schedule backup jobs in some timezones

View Issue Details [ Jump to Notes ]

[ Issue History ] [ Print ]

Project

Category

View Status

Date Submitted

Last Update

0000370

ClearCenter

app-remote-backup - Remote Server Backup

public

2011-10-23 23:02

2012-03-29 12:09

Reporter

devferret

Assigned To

dsokoloski

Priority

normal

Severity

major

Reproducibility

always

Status

closed

Resolution

fixed

Platform

OS Version

Product Version

5.2-SP1

Target Version

Fixed in Version

Summary

0000370: rbs-schedule.php fails to schedule backup jobs in some timezones

Description

Symptom:

Backups fail to run. No failure indication appears in the webconfig. Logs contain entries similar to:

   /var/log/suva:Oct 1 00:00:02 satir rbs-schedule[20003]: Nothing to do at this time: 10 < 0 or 10 > 1
   /var/log/suva:Oct 2 00:00:02 satir rbs-schedule[24291]: Nothing to do at this time: 10 < 0 or 10 > 1
   /var/log/suva:Oct 3 00:00:01 satir rbs-schedule[25164]: Nothing to do at this time: 9 < 0 or 9 > 1

Cause:

This occurs due to a timezone handling bug in /var/webconfig/scripts/script.inc.php. This code uses a 'date %Z' command to obtain a short (typically 3 letter) abbreviation for the system timezone, which it then uses to set the timezone for the PHP environment. Unfortunately such abbreviations are not unique (eg. both Australia and the U.S.A. use EST, CDT and others), so the timezone of the PHP environment can be set incorrectly. This leads to /var/webconfig/scripts/rbs-schedule.php deciding it has been invoked at the wrong time, and exiting with the log message given above.

Patch:

I have attached a patch which resolves the immediate issue by using a 'date %Z%z' command to retrieve a UTC offset as well as a 3-letter abbreviation; taken together these do uniquely determine the system timezone. The main logic is in script.inc.php WebConfigScript::SetTimeZone. This patch also cleans up the error handling in that function (what's the point of second-guessing errors from date_default_timezone_set, it's actually counter-productive) and removes some confusing and redundant (probably hangover) code in rbs-scheduler.php.

Additional Information

A discussion of the 3-letter timezone abbreviation issue can be found in the timezone data files (search for "unique" in the Australasia file in http://www.iana.org/time-zones/repository/releases/tzdata2011l.tar.gz [^]).

WRT to the following; apologies if anyone thinks I'm stepping on toes or have no clue, I'm just putting in my two developer cents. No harm intended.

I think a little more work is required to properly fix timezone handling; I can see quite a few problems:
- It seems wrong for WebConfigScript::SetTimeZone to happily set UTC if the real system timezone can't be set.
- It seems wrong to automagically call SetTimeZone in the WebConfigScript constructor, and then eat up any CODE_INVALID_TIMEZONE exception. Doing that means the calling code can't set policy; the scheduler will never see this error.
- Why does the scheduler simply exit if it thinks it has been invoked at the wrong time? Instead of giving up, it should do it's best to perform a backup.
- Deducing the system timezone using /bin/date seems a little kludgy. Doesn't the webconfig know the local timezone? Could it be put in PHP.INI? Or, system setup could save the timezone somewhere. Eg. my gentoo system saves "Australia/Melbourne" in /etc/timezone; init scripts then use that to set up /etc/localtime.

There also seem to be some policy / error handling issues in the scheduler. For a backup, I think correct behaviour is something like "when in doubt, do a backup ; notify the user on any failure". As far as the end result is concerned, an error in the scheduler is no different to an error in the remote backup code (ie. RemoteBackupService) itself; if the backup fails, it fails. There are code paths in rbs-schedule where the backup can fail without (email or webconfig) notification (not least, the RemoteBackupService is constructed outside any try block), perhaps it needs a pair of eyes and a once over.

Ok, so that was maybe, like 42 cents, not two :)

Tags

No tags attached.

Attached Files

fix_backup_timezone_handling.patch [^] (1,973 bytes) 2011-10-23 23:02 [Show Content] [Hide Content]

diff -ur scripts/rbs-schedule.php scripts.new/rbs-schedule.php
--- scripts/rbs-schedule.php	2010-11-10 03:52:56.000000000 +1100
+++ scripts.new/rbs-schedule.php	2011-10-24 15:18:26.000000000 +1100
@@ -35,15 +35,6 @@
 
 set_time_limit(0);
 
-// Set our time zone
-$tz = 'UTC';
-$ph = popen('/bin/date \'+%Z\'', 'r');
-if (is_resource($ph)) {
-	$buffer = chop(fgets($ph, 4096));
-	if (pclose($ph) == 0) $tz = $buffer;
-}
-date_default_timezone_set($tz);
-
 // Create an RBS client
 $rbs = new RemoteBackupService(basename($_SERVER['argv'][0]), false, false, false);
 
diff -ur scripts/script.inc.php scripts.new/script.inc.php
--- scripts/script.inc.php	2010-11-10 03:52:56.000000000 +1100
+++ scripts.new/script.inc.php	2011-10-12 05:19:43.000000000 +1100
@@ -237,23 +237,17 @@
 	public function SetTimeZone()
 	{
 		$tz_abrv = 'UTC';
-		$ph = popen('/bin/date \'+%Z\'', 'r');
+		$tz_offs = 0;
+		$ph = popen('/bin/date \'+%Z*%z\'', 'r');
 		if (is_resource($ph)) {
 			$buffer = chop(fgets($ph, 4096));
-			if (pclose($ph) == 0) $tz_abrv = $buffer;
+			if (pclose($ph) == 0) 
+				list($tz_abrv, $tz_offs) = explode('*', $buffer);
 		}
-		if (!class_exists('DateTimeZone')) {
-			if (!@date_default_timezone_set($tz_abrv)) {
-				if (!date_default_timezone_set('UTC'))
-					throw new ScriptException(ScriptException::CODE_INVALID_TIMEZONE);
-			}
-			return;
-		}
-		$tz_abrv_list = DateTimeZone::listAbbreviations();
-		if (!array_key_exists(strtolower($tz_abrv), $tz_abrv_list))
-			throw new ScriptException(ScriptException::CODE_INVALID_TIMEZONE);
-		if (!date_default_timezone_set($tz_abrv_list[strtolower($tz_abrv)][0]['timezone_id']))
-			throw new ScriptException(ScriptException::CODE_INVALID_TIMEZONE);
+		$tz_name = timezone_name_from_abbr($tz_abrv, $tz_offs * 36);
+		if (@date_default_timezone_set($tz_name)) return;
+		if (date_default_timezone_set('UTC')) return;
+		throw new ScriptException(ScriptException::CODE_INVALID_TIMEZONE);
 	}
 
 	// Lock a file

Relationships

Notes
(0000388) dsokoloski (developer) 2011-10-31 17:05	Source Code Changelog --------------------------------------------------- - Applied time-zone patch from devferret. [fixed tracker 0000370] File Changes --------------------------------------------------- Details: http://code.clearfoundation.com/svn/revision.php?repname=ClearOS&rev=2443 [^] U legacy/webconfig/branches/5.2/scripts/rbs-schedule.php U legacy/webconfig/branches/5.2/scripts/script.inc.php

(0000389) dsokoloski (developer) 2011-10-31 17:10	I've applied the patch for now to resolve the immediate issue of non-unique time-zones. I will revisit the notes here and incorporate some of the suggestions during the next code review/rewrite of RBS.

Issue History
Date Modified	Username	Field	Change
2011-10-23 23:02	devferret	New Issue
2011-10-23 23:02	devferret	File Added: fix_backup_timezone_handling.patch
2011-10-24 15:07	user2	Status	new => assigned
2011-10-24 15:07	user2	Assigned To	=> dsokoloski
2011-10-24 17:37	dsokoloski	Status	assigned => acknowledged
2011-10-31 17:05	dsokoloski	Checkin
2011-10-31 17:05	dsokoloski	Note Added: 0000388
2011-10-31 17:05	dsokoloski	Status	acknowledged => resolved
2011-10-31 17:05	dsokoloski	Resolution	open => fixed
2011-10-31 17:10	dsokoloski	Note Added: 0000389
2011-10-31 17:10	dsokoloski	Status	resolved => feedback
2011-10-31 17:10	dsokoloski	Resolution	fixed => reopened
2011-10-31 17:10	dsokoloski	Status	feedback => resolved
2011-10-31 17:10	dsokoloski	Resolution	reopened => fixed
2012-03-29 12:09	user2	Status	resolved => closed

ClearOS Bug Tracker