vtimezones: explicitly store std and dst TZNAME

In vCalendar 1.0, the DAYLIGHT property must list standard and daylight
saving zone names, for example CET;CEST.

ContextToTzDaylight() used to extract these two names from the
tz_entry::name field. This was rather implicit and not documented, so
the Linux timezone import code got it wrong and stored the original
TZID (like
/freeassociation.sourceforge.net/Tzfile/Australia/Melbourne), leading
to an invalid DAYLIGHT property with too many components. Such an
entry is accepted by Nokia phones in an ADD, but not an UPDATE
(bugs.meego.com #9600).

The code would also have failed for a (hypothetical) VTTIMEZONE
definition with TZNAMEs that contain slashes.

To make the code more robust and easier to understand, this patch
introduces explicit dstName and stdName fields in
tz_entry. VTIMEZONEtoTZEntry() sets them directly, thus allowing the
removal of two parameters in that function.

ContextToTzDaylight() still falls back to splitting the tz_entry::name
field if dstName or stdName are not set when needed, so these new
fields are strictly optional. In particular the code which creates
tz_entry from the builtin table can work as before.

The code which replaces the original TZID in tz_entry::name with a
combination of its TZNAMEs is still active, although it is no longer
needed. It probably was added in the first place to preserve the two
names so that they can be exported again.

4 files changed, 76 insertions, 26 deletions

diff --git a/src/platform_adapters/linux/platform_timezones.cpp b/src/platform_adapters/linux/platform_timezones.cpp
index d65f868..3fdc15b 100644
--- a/src/platform_adapters/linux/platform_timezones.cpp
+++ b/src/platform_adapters/linux/platform_timezones.cpp
@@ -191,12 +191,9 @@ void finalizeSystemZoneDefinitions(GZones* aGZones)
       continue;
     PLOGDEBUGPUTSX(aGZones->getDbgLogger, DBG_PARSE+DBG_EXOTIC, vtimezone);
     tz_entry t;
-    string dstName, stdName;
     if (VTIMEZONEtoTZEntry(
     	vtimezone,
       t,
-      stdName,
-      dstName,
       #ifdef SYDEBUG
         aGZones->getDbgLogger
       #endif
diff --git a/src/sysync/timezones.h b/src/sysync/timezones.h
index d3f30d4..6d16e90 100755
--- a/src/sysync/timezones.h
+++ b/src/sysync/timezones.h
@@ -66,7 +66,19 @@ typedef struct tChangeStruct {
 
 class tz_entry {
         public:
-          std::string name;          /**< name, same as TZID in VTIMEZONE, e.g. CET/CEST or /softwarestudio.org/Tzfile/Europe/Berlin */
+          std::string name;          /**< name, same as TZID in
+                                        VTIMEZONE, e.g. CET/CEST or
+                                        /softwarestudio.org/Tzfile/Europe/Berlin;
+                                        see also dst/stdName */
+          std::string stdName;       /**< optional standard time name, e.g. CEST; must be
+                                        set if "name" is not a concatenation of
+                                        standard and daylight name (CET/CEST);
+                                        the vCalendar 1.0 code relies on that */
+          std::string dstName;       /**< used instead of splitting
+                                        "name" if (and only if)
+                                        stdName is set; may be empty
+                                        in zones without daylight
+                                        saving */
           std::string location;      /**< location string as used in Olson TZID, e.g. Europe/Berlin */
           short       bias;          /**< minutes difference to UTC (west negative, east positive */
           short       biasDST;       /**< minutes difference to bias (not UTC!) */
diff --git a/src/sysync/vtimezone.cpp b/src/sysync/vtimezone.cpp
index 67ba156..8d39f14 100644
--- a/src/sysync/vtimezone.cpp
+++ b/src/sysync/vtimezone.cpp
@@ -470,8 +470,6 @@ static void MultipleSeq( string aText, int &s, int &d )
 
 bool VTIMEZONEtoTZEntry( const char*    aText, // VTIMEZONE string to be parsed
                          tz_entry      &t,
-                         string        &aStdName,
-                         string        &aDstName,
                          TDebugLogger*  aLogP)
 {
   short dBias; // the full bias for DST
@@ -482,12 +480,14 @@ bool VTIMEZONEtoTZEntry( const char*    aText, // VTIMEZONE string to be parsed
                                  t.dynYear= "CUR";
                                  t.biasDST= 0;
                                  t.bias   = 0;
-  if (!GetTZInfo( aText,VTZ_STD, t.std, t.bias, aStdName, -1, aLogP )) {
+                                 t.stdName =
+                                   t.dstName = "";
+  if (!GetTZInfo( aText,VTZ_STD, t.std, t.bias, t.stdName, -1, aLogP )) {
     success = false;
   }
   // default value if not found (which is treated as success by GetTZInfo)
   dBias= t.bias;
-  if (!GetTZInfo( aText,VTZ_DST, t.dst,  dBias, aDstName, -1, aLogP )) {
+  if (!GetTZInfo( aText,VTZ_DST, t.dst,  dBias, t.dstName, -1, aLogP )) {
     // unknown failure, better restore default
     dBias= t.bias;
     success = false;
@@ -513,24 +513,37 @@ bool VTIMEZONEtoInternal( const char*    aText, // VTIMEZONE string to be parsed
   aContext= tctx_tz_unknown;
 
   tz_entry      t;
-  string        stdName,
-                dstName,
-                lName;
+  string        lName;
   timecontext_t lContext;
 
-  if (!VTIMEZONEtoTZEntry( aText, t, stdName, dstName, aLogP )) {
+  if (!VTIMEZONEtoTZEntry( aText, t, aLogP )) {
     PLOGDEBUGPRINTFX(aLogP, DBG_PARSE+DBG_ERROR, ("parsing VTIMEZONE failed:\n%s", aText));
   }
   if (aTzidP) *aTzidP = t.name; // return the original TZID as found, needed to match with TZID occurences in rest of vCalendar
 
-  bool sC= !(stdName=="");
-  bool dC= !(dstName=="");
-
+  bool sC= !(t.stdName=="");
+  bool dC= !(t.dstName=="");
+
+  // This code here creates a new TZID by concatenating
+  // standard and daylight saving time. If a new tz_entry
+  // has to be created, then it will end up having this
+  // artificial TZID (t.name = tName below).
+  // Presumably this is done because the vCalendar 1.0 code
+  // needs these two names and they weren't stored
+  // elsewhere originally. This approach is dangerous (is it
+  // guaranteed that neither stdName nor dstName contains slashes -
+  // otherwise splitting the composed name will fail?)
+  // and confusing (well, it confused me - Patrick).
+  //
+  // As a result of this confusion, the Linux timezone importer
+  // code use the /org.softwarestudio/... TZID as name, which
+  // led to invalid vCalendar properties because splitting that
+  // name into two components failed.
   string tName = t.name;
   if  (sC || dC) tName = ""; // TZID will be replaced in case of unknown
-  if  (sC)       tName+= stdName;
+  if  (sC)       tName+= t.stdName;
   if  (sC && dC) tName+= "/";
-  if        (dC) tName+= dstName;
+  if        (dC) tName+= t.dstName;
 
   bool ok = true;
   bool okM= true;
@@ -568,19 +581,24 @@ bool VTIMEZONEtoInternal( const char*    aText, // VTIMEZONE string to be parsed
   // redundant here, but there is no other way to
   // add the entry
   string new_name;
+
+  // This assignment changes the TZID. The caller needs
+  // to maintain a mapping to deal with this. Perhaps
+  // keep the original name and avoid the mapping?
   t.name = tName;
+
   if (!ok) ok= FoundTZ( t, new_name, aContext, g, true );
 
   if  (ok && t.std.wMonth!=0 &&
              t.dst.wMonth!=0) {
     tz_entry std;
-    std.name = stdName;
+    std.name = t.stdName;
     std.ident= "s"; // standard
     std.bias = t.bias;
     FoundTZ( std, lName,lContext, g, true );
 
     tz_entry dst;
-    dst.name = dstName;
+    dst.name = t.dstName;
     dst.ident= "d"; // daylight saving
     dst.bias = t.bias + t.biasDST;
     FoundTZ( dst, lName,lContext, g, true );
@@ -914,17 +932,42 @@ bool ContextToTzDaylight( timecontext_t  aContext,
 
       timecontext_t                           cc= TCTX_UNKNOWN;
       while (FoundTZ( tCopy, s, cc, g, false, cc )) {
-        if (s.find( "/",0 )!=string::npos) { // search for a time zone with slash in it
+        // search for a time zone with slash in it (which is
+        // interpreted as separator between dstName and stdName) *or*
+        // one which has dstName and stdName set explicitly;
+        // prefer explicit names over splitting name
+        tz_entry zone;
+        if (GetTZ( cc, zone, g, -1 ) &&
+            !zone.stdName.empty() &&
+            !zone.dstName.empty()) {
+          s = zone.stdName + ";" + zone.dstName;
+          found= true; break;
+        } else if (s.find( "/",0 )!=string::npos) {
+          // Assumption here is that s contains exactly one slash,
+          // otherwise s is not valid for stdName;dstName in vCalendar
+          // 1.0.
+          StringSubst( s, "/", ";" );
           found= true; break;
         } // if
       } // while
 
       if (!found) {
-        s = t.name; // create a <x>;<x> string
+        string stdName;
+        if (t.stdName.empty()) {
+          stdName = t.name;
+        } else {
+          stdName = t.stdName;
+        }
+        s = stdName; // create a <x>;<x> string
         s+= ";";
-        s+= t.name;
+        s+= stdName;
       } // if
-    } // if
+    } else if (!t.stdName.empty() && !t.dstName.empty()) {
+      // use explicit zone names instead of splitting s
+      s = t.stdName + ";" + t.dstName;
+    } else {
+      StringSubst( s, "/", ";" );
+    }
 
     if (!dDone) dt= DST_Switch( t, t.bias, year, true  ); // get the switch time/date for DST
     if (!sDone) st= DST_Switch( t, t.bias, year, false ); // get the switch time/date for STD
@@ -951,7 +994,7 @@ bool ContextToTzDaylight( timecontext_t  aContext,
   string                 stdStr;
   TimestampToISO8601Str( stdStr, st, TCTX_UTC );
 
-  StringSubst                         ( s, "/", ";" );
+  // s must be of the format stdName;dstName here, for example CET;CEST or PST;PDT
   aText+= dstStr + ";" + stdStr + ";" + s;
 
   return true;
diff --git a/src/sysync/vtimezone.h b/src/sysync/vtimezone.h
index 5115772..c3707fd 100755
--- a/src/sysync/vtimezone.h
+++ b/src/sysync/vtimezone.h
@@ -30,8 +30,6 @@ class TDebugLogger;
 
 bool VTIMEZONEtoTZEntry( const char*    aText, // VTIMEZONE string to be parsed
                          tz_entry      &t,
-                         string        &aStdName,
-                         string        &aDstName,
                          TDebugLogger*  aLogP);
 
 /*! Convert VTIMEZONE string into internal context value */