Hello Beat, Lukas!
While debugging an issue (http://bugs.meego.com/show_bug.cgi?id=9600) I
noticed that SyncEvolution was sending broken vCalendar 1.0 DAYLIGHT
properties.
I tracked it down to the assumption that tz_entry::name is composed of
standard and daylight saving zone names (CET/CEST), which I broken in
the libical timezone import code on Linux.
I'd like to suggest that this assumption gets relaxed such that the
original TZID can be stored together with these two names in a tz_entry.
See attached patches, which I also committed to "master" on
meego.gitorious.org.
Is that okay and suitable for merging back?
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
>From d9f9af69176d776d09f99d85789973fd5d3b10f4 Mon Sep 17 00:00:00 2001
From: Patrick Ohly <patrick.o...@intel.com>
Date: Sun, 12 Dec 2010 21:19:16 +0100
Subject: [PATCH 1/2] 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.
---
src/platform_adapters/linux/platform_timezones.cpp | 3 -
src/sysync/timezones.h | 14 +++-
src/sysync/vtimezone.cpp | 83 +++++++++++++++-----
src/sysync/vtimezone.h | 2 -
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 */
--
1.7.2.3
>From 29a7998a0e27d3a664b3ca143baf201b72ecdc6a Mon Sep 17 00:00:00 2001
From: Patrick Ohly <patrick.o...@intel.com>
Date: Sun, 12 Dec 2010 21:36:10 +0100
Subject: [PATCH 2/2] vtimezone: retain original TZID in imported definitions
VTIMEZONEtoInternal() used to overwrite the original TZID because
there was no other place to store standard and daylight saving TZNAME.
Since there are now explicit fields for that, the code overwriting the
TZID can be removed.
The advantage is that importing and exporting a custom definition
introduces less changes, because now the original TZID will be exported.
---
src/sysync/vtimezone.cpp | 34 +++++-----------------------------
1 files changed, 5 insertions(+), 29 deletions(-)
diff --git a/src/sysync/vtimezone.cpp b/src/sysync/vtimezone.cpp
index 8d39f14..b4519a6 100644
--- a/src/sysync/vtimezone.cpp
+++ b/src/sysync/vtimezone.cpp
@@ -519,32 +519,13 @@ bool VTIMEZONEtoInternal( const char* aText, // VTIMEZONE string to be parsed
if (!VTIMEZONEtoTZEntry( aText, t, aLogP )) {
PLOGDEBUGPRINTFX(aLogP, DBG_PARSE+DBG_ERROR, ("parsing VTIMEZONE failed:\n%s", aText));
}
+ // Telling the caller about the original TZID is necessary because this
+ // code might match that TZID against an existing definition with a different
+ // TZID. Previously it was also necessary when importing the VTIMEZONE, because
+ // the original TZID was overwritten. Now imported definitions retain the
+ // original TZID in t.name.
if (aTzidP) *aTzidP = t.name; // return the original TZID as found, needed to match with TZID occurences in rest of vCalendar
- 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+= t.stdName;
- if (sC && dC) tName+= "/";
- if (dC) tName+= t.dstName;
-
bool ok = true;
bool okM= true;
int s,d;
@@ -582,11 +563,6 @@ bool VTIMEZONEtoInternal( const char* aText, // VTIMEZONE string to be parsed
// 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 &&
--
1.7.2.3
_______________________________________________
os-libsynthesis mailing list
os-libsynthesis@synthesis.ch
http://lists.synthesis.ch/mailman/listinfo/os-libsynthesis
