Definitely a great idea, since most linux distributions already bundle this
information and update it frequently.
BTW, your implementation seems to require some security checks for timezone
names like "../../../etc/passwd".

Nuno

----- Original Message -----
From: "Joe Orton" jorton@redhat.com
To: derick@php.net; internals@lists.php.net
Sent: Wednesday, January 09, 2008 2:15 PM
Subject: [PHP-DEV] [PATCH] date/timelib: use system timezone database

Hi folks. It's a bit of a maintenance headache for distributions when
packages include their own copy of the timezone database, since this
needs to be updated frequently.

I've prepared a patch to allow the timelib code to use the system
timezone database directly; would something like this be acceptable, any
thoughts? This passes the ext/date tests, and reduces the PHP binary
size by about 300K to boot.

Notes:

1. I've not implemented timelib_timezone_builtin_identifiers_list() here
   since it doesn't seem to be used, is it there for third-party
   extensions? It could be implemented by iterating through the directory.

2. there's no general way that I can find to obtain the database
   version, so I just invented a string here.

joe

Index: ext/date/lib/parse_tz.c

RCS file: /repository/php-src/ext/date/lib/parse_tz.c,v
retrieving revision 1.35
diff -u -r1.35 parse_tz.c
--- ext/date/lib/parse_tz.c 31 Dec 2007 07:12:08 -0000 1.35
+++ ext/date/lib/parse_tz.c 9 Jan 2008 14:04:28 -0000
@@ -20,6 +20,14 @@

#include "timelib.h"

+#ifdef HAVE_SYSTEM_TZDATA
+#include <sys/mman.h>
+#include <sys/stat.h>
+#include <limits.h>
+#include <fcntl.h>
+#include <unistd.h>
+#endif

#include <stdio.h>

#ifdef HAVE_STRING_H
@@ -27,7 +35,10 @@
#else
#include <strings.h>
#endif

+#ifndef HAVE_SYSTEM_TZDATA
#include "timezonedb.h"
+#endif

#if (defined(APPLE) || defined(APPLE_CC)) &&
(defined(BIG_ENDIAN) || defined(LITTLE_ENDIAN))

if defined(LITTLE_ENDIAN)

@@ -202,6 +213,86 @@
}
}

+#ifdef HAVE_SYSTEM_TZDATA

+#ifdef HAVE_SYSTEM_TZDATA_PREFIX
+#define ZONEINFO_PREFIX HAVE_SYSTEM_TZDATA_PREFIX
+#else
+#define ZONEINFO_PREFIX "/usr/share/zoneinfo"
+#endif

+static const timelib_tzdb timezonedb_system = { "0.system", 0, NULL,
NULL };

+/* Return the mmap()ed tzfile if found, else NULL. On success, the

• • length of the mapped data is placed in *length. */
    +static char *map_tzfile(const char *timezone, size_t *length)
    +{
• char fname[PATH_MAX];
• struct stat st;
• char *p;
• int fd;
• snprintf(fname, sizeof fname, ZONEINFO_PREFIX "/%s", timezone);
• fd = open(fname, O_RDONLY);
• if (fd == -1) {
• return NULL;
• } else if (fstat(fd, &st) != 0 || st.st_size < 21) {
• close(fd);
• return NULL;
• }
• *length = st.st_size;
• p = mmap(NULL, st.st_size, PROT_READ, MAP_SHARED, fd, 0);
• close(fd);
• return p != MAP_FAILED ? p : NULL;
  +}

+const timelib_tzdb *timelib_builtin_db(void)
+{

• return &timezonedb_system;
  +}

+const timelib_tzdb_index_entry
*timelib_timezone_builtin_identifiers_list(int *count)
+{

• *count = 0;
• return NULL;
  +}

+int timelib_timezone_id_is_valid(char *timezone, const timelib_tzdb
*tzdb)
+{

• char fname[PATH_MAX];
• snprintf(fname, sizeof fname, ZONEINFO_PREFIX "/%s", timezone);
• return access(fname, R_OK) == 0 ? 1 : 0;
  +}

+timelib_tzinfo *timelib_parse_tzfile(char *timezone, const timelib_tzdb
*tzdb)
+{

• char *tzf, *orig;
• timelib_tzinfo *tmp;
• size_t len;
• orig = map_tzfile(timezone, &len);
• if (orig == NULL) {
• return NULL;
• }
• tmp = timelib_tzinfo_ctor(timezone);
• tzf = orig + 20;
• read_header(&tzf, tmp);
• read_transistions(&tzf, tmp);
• read_types(&tzf, tmp);
• munmap(orig, len);
• return tmp;
  +}
  +#else

static int seek_to_tz_position(const unsigned char **tzf, char *timezone,
const timelib_tzdb *tzdb)
{
int left = 0, right = tzdb->index_size - 1;
@@ -258,6 +349,7 @@

return tmp;
}
+#endif

static ttinfo* fetch_timezone_offset(timelib_tzinfo *tz, timelib_sll ts,
timelib_sll *transition_time)
{
Index: ext/date/lib/timelib.m4

RCS file: /repository/php-src/ext/date/lib/timelib.m4,v
retrieving revision 1.4
diff -u -r1.4 timelib.m4
--- ext/date/lib/timelib.m4 3 Jul 2005 23:30:52 -0000 1.4
+++ ext/date/lib/timelib.m4 9 Jan 2008 14:04:28 -0000
@@ -78,3 +78,17 @@

dnl Check for strtoll, atoll
AC_CHECK_FUNCS(strtoll atoll strftime)

+PHP_ARG_WITH(system-tzdata, for use of system timezone data,
+[ --with-system-tzdata[=DIR] to specify use of system timezone
data],
+no, no)

+if test "$PHP_SYSTEM_TZDATA" != "no"; then

• AC_DEFINE(HAVE_SYSTEM_TZDATA, 1, [Define if system timezone data is
  used])
• if test "$PHP_SYSTEM_TZDATA" != "yes"; then

•  AC_DEFINE_UNQUOTED(HAVE_SYSTEM_TZDATA_PREFIX, "$PHP_SYSTEM_TZDATA",
  

•                     [Define for location of system timezone data])
  

• fi
  +fi

2008/1/9, Joe Orton jorton@redhat.com:

Hi folks. It's a bit of a maintenance headache for distributions when
packages include their own copy of the timezone database, since this
needs to be updated frequently.

+1000 :-D

--
http://www.kissofjudas.net/

The reason we do not use the system database is because it is
inconsistent and likely is updated less frequently then the one
included with PHP.

-1.

Hi folks. It's a bit of a maintenance headache for distributions when
packages include their own copy of the timezone database, since this
needs to be updated frequently.

I've prepared a patch to allow the timelib code to use the system
timezone database directly; would something like this be acceptable,
any
thoughts? This passes the ext/date tests, and reduces the PHP binary
size by about 300K to boot.

Notes:

1. I've not implemented timelib_timezone_builtin_identifiers_list()
   here
   since it doesn't seem to be used, is it there for third-party
   extensions? It could be implemented by iterating through the
   directory.

2. there's no general way that I can find to obtain the database
   version, so I just invented a string here.

joe

Index: ext/date/lib/parse_tz.c

RCS file: /repository/php-src/ext/date/lib/parse_tz.c,v
retrieving revision 1.35
diff -u -r1.35 parse_tz.c
--- ext/date/lib/parse_tz.c 31 Dec 2007 07:12:08 -0000 1.35
+++ ext/date/lib/parse_tz.c 9 Jan 2008 14:04:28 -0000
@@ -20,6 +20,14 @@

#include "timelib.h"

+#ifdef HAVE_SYSTEM_TZDATA
+#include <sys/mman.h>
+#include <sys/stat.h>
+#include <limits.h>
+#include <fcntl.h>
+#include <unistd.h>
+#endif

#include <stdio.h>

#ifdef HAVE_STRING_H
@@ -27,7 +35,10 @@
#else
#include <strings.h>
#endif

+#ifndef HAVE_SYSTEM_TZDATA
#include "timezonedb.h"
+#endif

#if (defined(APPLE) || defined(APPLE_CC)) &&
(defined(BIG_ENDIAN) || defined(LITTLE_ENDIAN))

if defined(LITTLE_ENDIAN)

@@ -202,6 +213,86 @@
}
}

+#ifdef HAVE_SYSTEM_TZDATA

+#ifdef HAVE_SYSTEM_TZDATA_PREFIX
+#define ZONEINFO_PREFIX HAVE_SYSTEM_TZDATA_PREFIX
+#else
+#define ZONEINFO_PREFIX "/usr/share/zoneinfo"
+#endif

+static const timelib_tzdb timezonedb_system = { "0.system", 0,
NULL, NULL };

+/* Return the mmap()ed tzfile if found, else NULL. On success, the

• • length of the mapped data is placed in *length. */
    +static char *map_tzfile(const char *timezone, size_t *length)
    +{
• char fname[PATH_MAX];
• struct stat st;
• char *p;
• int fd;
• snprintf(fname, sizeof fname, ZONEINFO_PREFIX "/%s", timezone);
• fd = open(fname, O_RDONLY);
• if (fd == -1) {

• return NULL;
  

• } else if (fstat(fd, &st) != 0 || st.st_size < 21) {

• close(fd);
  

• return NULL;
  

• }
• *length = st.st_size;
• p = mmap(NULL, st.st_size, PROT_READ, MAP_SHARED, fd, 0);
• close(fd);
• return p != MAP_FAILED ? p : NULL;
  +}

+const timelib_tzdb *timelib_builtin_db(void)
+{

• return &timezonedb_system;
  +}

+const timelib_tzdb_index_entry
*timelib_timezone_builtin_identifiers_list(int *count)
+{

• *count = 0;
• return NULL;
  +}

+int timelib_timezone_id_is_valid(char *timezone, const timelib_tzdb
*tzdb)
+{

• char fname[PATH_MAX];
• snprintf(fname, sizeof fname, ZONEINFO_PREFIX "/%s", timezone);
• return access(fname, R_OK) == 0 ? 1 : 0;
  +}

+timelib_tzinfo *timelib_parse_tzfile(char *timezone, const
timelib_tzdb *tzdb)
+{

• char *tzf, *orig;
• timelib_tzinfo *tmp;
• size_t len;
• orig = map_tzfile(timezone, &len);
• if (orig == NULL) {

• return NULL;
  

• }
• tmp = timelib_tzinfo_ctor(timezone);
• tzf = orig + 20;
• read_header(&tzf, tmp);
• read_transistions(&tzf, tmp);
• read_types(&tzf, tmp);
• munmap(orig, len);
• return tmp;
  +}
  +#else

static int seek_to_tz_position(const unsigned char **tzf, char
*timezone, const timelib_tzdb *tzdb)
{
int left = 0, right = tzdb->index_size - 1;
@@ -258,6 +349,7 @@

return tmp;
}
+#endif

static ttinfo* fetch_timezone_offset(timelib_tzinfo *tz, timelib_sll
ts, timelib_sll *transition_time)
{
Index: ext/date/lib/timelib.m4

RCS file: /repository/php-src/ext/date/lib/timelib.m4,v
retrieving revision 1.4
diff -u -r1.4 timelib.m4
--- ext/date/lib/timelib.m4 3 Jul 2005 23:30:52 -0000 1.4
+++ ext/date/lib/timelib.m4 9 Jan 2008 14:04:28 -0000
@@ -78,3 +78,17 @@

dnl Check for strtoll, atoll
AC_CHECK_FUNCS(strtoll atoll strftime)

+PHP_ARG_WITH(system-tzdata, for use of system timezone data,
+[ --with-system-tzdata[=DIR] to specify use of system
timezone data],
+no, no)

+if test "$PHP_SYSTEM_TZDATA" != "no"; then

• AC_DEFINE(HAVE_SYSTEM_TZDATA, 1, [Define if system timezone data
  is used])
• if test "$PHP_SYSTEM_TZDATA" != "yes"; then

•  AC_DEFINE_UNQUOTED(HAVE_SYSTEM_TZDATA_PREFIX,  
  

"$PHP_SYSTEM_TZDATA",

•                     [Define for location of system timezone  
  

data])

• fi
  +fi

--

Ilia Alshanetsky

It's a bit of a maintenance headache for distributions when
packages include their own copy of the timezone database, since this
needs to be updated frequently.

There is a PECL extension to provide those updates:
http://pecl.php.net/package/timezonedb
It is a drop-in replacement to update the rules.

I've prepared a patch to allow the timelib code to use the system
timezone database directly; would something like this be acceptable, any
thoughts?

Quite a few actually.

Notes:

1. I've not implemented timelib_timezone_builtin_identifiers_list() here
   since it doesn't seem to be used, is it there for third-party
   extensions? It could be implemented by iterating through the directory.

It's not used (anymore). However, there is a PHP function
timezone_identifiers_list that uses the index information to enumerate
all entries. Your patch prevents this function from working as both
struct elements index_size and index are set to 0/null.

2. there's no general way that I can find to obtain the database
   version, so I just invented a string here.

Not having the version creates another problem, as the PECL timezonedb
extension will then always override the builtin database. The
extension uses the version number to see if the extension provides a
later rules set. With your patch, this extra check will not work
correctly anymore if PHP's built-in version is newer (which would happen
if PHP got upgraded).

Some other issues:

• This patchs allows accessing any file on the filesystem (because the
  path is not sanitized).
• Opening a file is less quick than having the data in memory.
• The system's format of tzfiles does not necessarily have to be the
  same as the one that PHP reads. On my (Debian) system the tz provided
  data is already 64bit read. Once I update the extension to use this
  extra data, reading the system tz files won't work anymore.
• It would be possible to use a "wrong set" of tzdata, resulting in bugs
  like http://bugs.php.net/bug.php?id=35958

The reason to bundle the database is to have a operating system
independent source of tzdata, which is something that this patch
destroys. I am because of this, and all the other issues, quite against
adding this patch - especially because there is already a solution for
this.

regards,
Derick

--
Derick Rethans
http://derickrethans.nl | http://ezcomponents.org | http://xdebug.org