Hello! One of the problems that we have in SyncEvolution is the loss of properties not supported by SyncML servers, like Google: https://bugs.meego.com/show_bug.cgi?id=15029
For example, BDAY is not supported and gets lost in a round-trip sync because the client doesn't know that the server didn't store the field. That's because the Google server does not provide CtCap as part of its DevInf. The attached patches address this problem by allowing a RemoteRule to provide CtCap information. It's not currently in the branch used by SyncEvolution, but I need to add something like it soon. Comments welcome. I'm a bit undecided whether the RemoteRule should provide a DevInf or only CtCap. Right now, only CtCap is used and the rest is ignored, but perhaps that might change someday? -- 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 62df728b87da358b497696ced0500ff221ea0979 Mon Sep 17 00:00:00 2001 From: Patrick Ohly <[email protected]> Date: Wed, 15 Jun 2011 12:24:17 +0200 Subject: [PATCH 1/2] remote rules: added OverrideDevInf Many SyncML servers send DevInf without CtCap inside. For those servers, the engine (often incorrectly) has to assume that they support all fields. This patch adds a configuration element "OverrideDevInf" which helps to address this problem. It may occur in a "RemoteRule". Once that rule matches a DevInf sent by a peer, processing of CtCap continues with the CtCap element of the DevInf inside the remote rule. This allows adding and overriding the CtCap from a peer. "OverrideDevInf" must contain a complete DevInf, although right now only the CtCap inside it really matters. This approach has the advantage that it can reuse the existing and somewhat documented CtCap format for describing capabilities. The downside is that not everything can be encoded in that format, for example, how many components of the ORG field are supported. A possible solution for that is to extend the format in a way that only the Synthesis engine understand and/or standardize such extensions. Right now the "OverrideDevInf" content is only parsed when needed. Beware that syntax errors will only be found when using the rule! --- src/sysync/binfileimplclient.cpp | 4 +- src/sysync/binfileimplclient.h | 2 +- src/sysync/localengineds.cpp | 2 +- src/sysync/syncsession.cpp | 91 +++++++++++++++++++++++++++++++++++++- src/sysync/syncsession.h | 7 +++- 5 files changed, 99 insertions(+), 7 deletions(-) diff --git a/src/sysync/binfileimplclient.cpp b/src/sysync/binfileimplclient.cpp index 9b73c9c..e8be70e 100755 --- a/src/sysync/binfileimplclient.cpp +++ b/src/sysync/binfileimplclient.cpp @@ -2525,7 +2525,7 @@ void TBinfileImplClient::saveRemoteParams(void) // check remote devinf to detect special behaviour needed for some servers. -localstatus TBinfileImplClient::checkRemoteSpecifics(SmlDevInfDevInfPtr_t aDevInfP) +localstatus TBinfileImplClient::checkRemoteSpecifics(SmlDevInfDevInfPtr_t aDevInfP, SmlDevInfDevInfPtr_t *aOverrideDevInfP) { if (fConfigP->fBinfilesActive && aDevInfP) { // check for some specific servers we KNOW they need special treatment @@ -2577,7 +2577,7 @@ localstatus TBinfileImplClient::checkRemoteSpecifics(SmlDevInfDevInfPtr_t aDevIn } } // let session handle other details - return inherited::checkRemoteSpecifics(aDevInfP); + return inherited::checkRemoteSpecifics(aDevInfP, aOverrideDevInfP); } // TBinfileImplClient::checkRemoteSpecifics diff --git a/src/sysync/binfileimplclient.h b/src/sysync/binfileimplclient.h index 5ceeb62..d995fab 100755 --- a/src/sysync/binfileimplclient.h +++ b/src/sysync/binfileimplclient.h @@ -550,7 +550,7 @@ public: uInt32 fRemotepartyID; // - check remote devinf to detect special behaviour needed for some servers. Base class // does not do anything on server level (configured rules are handled at session level) - virtual localstatus checkRemoteSpecifics(SmlDevInfDevInfPtr_t aDevInfP); + virtual localstatus checkRemoteSpecifics(SmlDevInfDevInfPtr_t aDevInfP, SmlDevInfDevInfPtr_t *aOverrideDevInfP); TBinfileDBSyncProfile fProfile; // - remote specific client behaviour flags uInt8 fRemoteFlags; // flags for remote specific behaviour (remotespecs_XXX) diff --git a/src/sysync/localengineds.cpp b/src/sysync/localengineds.cpp index b8289c6..1904670 100644 --- a/src/sysync/localengineds.cpp +++ b/src/sysync/localengineds.cpp @@ -2896,7 +2896,7 @@ localstatus TLocalEngineDS::engInitForSyncOps( // - this is executed only once per session, after that, we'll be fRemoteDevInfLock-ed if (!fSessionP->fRemoteDevInfKnown && !fSessionP->fRemoteDevInfLock) { // detect client specific server behaviour if needed - sta = fSessionP->checkRemoteSpecifics(NULL); + sta = fSessionP->checkRemoteSpecifics(NULL, NULL); fSessionP->remoteAnalyzed(); // analyzed now (accepted or not does not matter) if (sta!=LOCERR_OK) return sta; // not ok, device rejected diff --git a/src/sysync/syncsession.cpp b/src/sysync/syncsession.cpp index 8160c9b..8e7994d 100644 --- a/src/sysync/syncsession.cpp +++ b/src/sysync/syncsession.cpp @@ -26,6 +26,8 @@ #include "platform_thread.h" #endif +#include <xltdec.h> +#include <xltdevinf.h> #ifndef SYNCSESSION_PART1_EXCLUDE @@ -500,6 +502,8 @@ bool TRemoteRuleConfig::localStartElement(const char *aElementName, const char * expectTristate(fForceUTC); else if (strucmp(aElementName,"forcelocaltime")==0) expectTristate(fForceLocaltime); + else if (strucmp(aElementName,"overridedevinf")==0) + expectString(fOverrideDevInfXML); // inclusion of subrules else if (strucmp(aElementName,"include")==0) { // <include rule=""/> @@ -3516,11 +3520,17 @@ localstatus TSyncSession::analyzeRemoteDevInf( )); } // detect remote specific server behaviour if needed - sta = checkRemoteSpecifics(aDevInfP); + SmlDevInfDevInfPtr_t CTCapDevInfP = aDevInfP; + sta = checkRemoteSpecifics(aDevInfP, &CTCapDevInfP); if (sta!=LOCERR_OK) { remoteAnalyzed(); // analyzed to reject goto done; } + // switch to DevInf provided by remote rules + if (CTCapDevInfP != aDevInfP) { + PDEBUGPRINTFX(DBG_REMOTEINFO,("using CTCaps provided in DevInf of remote rule")); + aDevInfP = CTCapDevInfP; + } // Types and datastores may not be changed/added if sync has allready started if (fRemoteDevInfLock) { // Sync already started, in "blind" mode or previously received devInf, @@ -4314,7 +4324,7 @@ bool TSyncSession::SessionLogin( // does not do anything on server level (configured rules are handled at session level) // - NOTE: aDevInfP can be NULL to specify that remote device has not sent any devInf at all // and this is a blind sync attempt (so best-guess workaround settings might apply) -localstatus TSyncSession::checkRemoteSpecifics(SmlDevInfDevInfPtr_t aDevInfP) +localstatus TSyncSession::checkRemoteSpecifics(SmlDevInfDevInfPtr_t aDevInfP, SmlDevInfDevInfPtr_t *aOverrideDevInfP) { #if defined(SYSER_REGISTRATION) || !defined(NO_REMOTE_RULES) localstatus sta = LOCERR_OK; @@ -4426,6 +4436,80 @@ localstatus TSyncSession::checkRemoteSpecifics(SmlDevInfDevInfPtr_t aDevInfP) for(pos=fActiveRemoteRules.begin();pos!=fActiveRemoteRules.end();pos++) { // activate this rule TRemoteRuleConfig *ruleP = *pos; + if (!ruleP->fOverrideDevInfXML.empty()) { + // SMLTK expects full SyncML message + string buffer = + "<SyncML><SyncHdr>" + "<VerDTD>1.2</VerDTD>" + "<VerProto>SyncML/1.2</VerProto>" + "<SessionID>1</SessionID>" + "<MsgID>1</MsgID>" + "<Target><LocURI>foo</LocURI></Target>" + "<Source><LocURI>bar</LocURI></Source>" + "</SyncHdr>" + "<SyncBody>" + "<Results>" + "<CmdID>1</CmdID>" + "<MsgRef>1</MsgRef>" + "<CmdRef>1</CmdRef>" + "<Meta>" + "<Type>application/vnd.syncml-devinf+xml</Type>" + "</Meta>" + "<Item>" + "<Source>" + "<LocURI>./devinf12</LocURI>" + "</Source>" + "<Data>" + ; + buffer += ruleP->fOverrideDevInfXML; + buffer += + "</Data>" + "</Item>" + "</Results>" + "</SyncBody>" + "</SyncML>"; + MemPtr_t xml = (unsigned char *)buffer.c_str(); + XltDecoderPtr_t decoder = NULL; + SmlSyncHdrPtr_t hdr = NULL; + Ret_t ret = xltDecInit(SML_XML, + xml + buffer.size(), + &xml, + &decoder, + &hdr); + if (ret != SML_ERR_OK) { + sta = LOCERR_BADCONTENT; + PDEBUGPRINTFX(DBG_ERROR,("initializing scanner for DevInf in %s failed",ruleP->getName())); + } else { + SmlProtoElement_t element; + VoidPtr_t content = NULL; + ret = xltDecNext(decoder, + xml + buffer.size(), + &xml, + &element, + &content); + if (ret != SML_ERR_OK) { + sta = LOCERR_BADCONTENT; + PDEBUGPRINTFX(DBG_ERROR,("parsing of DevInf in %s failed",ruleP->getName())); + } else if (element != SML_PE_RESULTS || + !((SmlResultsPtr_t)content)->itemList || + !((SmlResultsPtr_t)content)->itemList->item || + !((SmlResultsPtr_t)content)->itemList->item->data || + ((SmlResultsPtr_t)content)->itemList->item->data->contentType != SML_PCDATA_EXTENSION || + ((SmlResultsPtr_t)content)->itemList->item->data->extension != SML_EXT_DEVINF) { + sta = LOCERR_BADCONTENT; + PDEBUGPRINTFX(DBG_ERROR,("parsing of DevInf in %s returned unexpected result",ruleP->getName())); + } else if (aOverrideDevInfP) { + // processing in caller will continue with updated DevInf + *aOverrideDevInfP = (SmlDevInfDevInfPtr_t)((SmlResultsPtr_t)content)->itemList->item->data->content; + } + } + if (decoder) + xltDecTerminate(decoder); + if (sta!=LOCERR_OK) { + AbortSession(sta,true); + break; + } + } // - apply options that have a value if (ruleP->fLegacyMode>=0) fLegacyMode = ruleP->fLegacyMode; if (ruleP->fLenientMode>=0) fLenientMode = ruleP->fLenientMode; @@ -4485,6 +4569,9 @@ localstatus TSyncSession::checkRemoteSpecifics(SmlDevInfDevInfPtr_t aDevInfP) #endif } // for all activated rules PDEBUGENDBLOCK("RemoteRules"); + // something failed in applying remote rules? + if (sta!=LOCERR_OK) + return sta; #endif // NO_REMOTE_RULES // Final adjustments #ifndef NO_REMOTE_RULES diff --git a/src/sysync/syncsession.h b/src/sysync/syncsession.h index 958c7df..1e92321 100755 --- a/src/sysync/syncsession.h +++ b/src/sysync/syncsession.h @@ -155,6 +155,11 @@ public: #ifdef SCRIPT_SUPPORT string fRuleScriptTemplate; // template for rule script #endif + // DevInf in XML format which is to be used instead of the one sent by peer. + // If set, it is evaluated after identifying the peer based on the DevInf + // that it has sent and before applying other remote rule workarounds. + // XML DevInf directly from XML config. + string fOverrideDevInfXML; // list of subrules to activate TRemoteRulesList fSubRulesList; // flag if this is a final rule (if matches, no more rules will be checked) @@ -827,7 +832,7 @@ protected: virtual SmlPcdataPtr_t newHeaderMeta(void); // - check remote devinf to detect special behaviour needed for some clients. Base class // does not do anything on server level (configured rules are handled at session level) - virtual localstatus checkRemoteSpecifics(SmlDevInfDevInfPtr_t aDevInfP); + virtual localstatus checkRemoteSpecifics(SmlDevInfDevInfPtr_t aDevInfP, SmlDevInfDevInfPtr_t *aOverrideDevInfP); // - remote device is analyzed, possibly save status virtual void remoteAnalyzed(void) { /* nop */ }; // SyncML Toolkit interface -- 1.7.2.5
>From 971cc942c40f22bfc40f83857c88b14cd313332c Mon Sep 17 00:00:00 2001 From: Patrick Ohly <[email protected]> Date: Wed, 15 Jun 2011 13:51:54 +0200 Subject: [PATCH 2/2] remote rules + OverrideDevInf: parse DevInf as part of checking the config Now the DevInf is parsed in TSessionConfig::localResolve(). The result is stored in the TRemoteRuleConfig instance. The advantage is that syntax errors are found earlier and for all remote rules, not just the one active during a sync. The downside is somewhat increased memory and CPU consumption. The DevInf in XML format is kept in memory although no longer needed after localResolve(). That might be useful for debugging. --- src/sysync/syncsession.cpp | 158 +++++++++++++++++++++++--------------------- src/sysync/syncsession.h | 2 + 2 files changed, 85 insertions(+), 75 deletions(-) diff --git a/src/sysync/syncsession.cpp b/src/sysync/syncsession.cpp index 8e7994d..332ede1 100644 --- a/src/sysync/syncsession.cpp +++ b/src/sysync/syncsession.cpp @@ -369,6 +369,9 @@ TRemoteRuleConfig::TRemoteRuleConfig(const char *aElementName, TConfigElement *a // config destructor TRemoteRuleConfig::~TRemoteRuleConfig() { + if (fOverrideDevInfBufferP) + smlFreeProtoElement(fOverrideDevInfBufferP); + clear(); } // TRemoteRuleConfig::~TRemoteRuleConfig @@ -421,6 +424,9 @@ void TRemoteRuleConfig::clear(void) fSubRule = false; // normal rule by default // - rules are final by default fFinalRule = true; + // no DevInf by default + fOverrideDevInfP = NULL; + fOverrideDevInfBufferP = NULL; // clear inherited inherited::clear(); } // TRemoteRuleConfig::clear @@ -851,10 +857,82 @@ void TSessionConfig::localResolve(bool aLastPass) // resolve if (aLastPass) { #ifndef NO_REMOTE_RULES - // - resolve rules + // - resolve rules and parse OverrideDevInf TRemoteRulesList::iterator pos; - for(pos=fRemoteRulesList.begin();pos!=fRemoteRulesList.end();pos++) + for(pos=fRemoteRulesList.begin();pos!=fRemoteRulesList.end();pos++) { (*pos)->Resolve(aLastPass); + + if (!(*pos)->fOverrideDevInfXML.empty()) { + // SMLTK expects full SyncML message + string buffer = + "<SyncML><SyncHdr>" + "<VerDTD>1.2</VerDTD>" + "<VerProto>SyncML/1.2</VerProto>" + "<SessionID>1</SessionID>" + "<MsgID>1</MsgID>" + "<Target><LocURI>foo</LocURI></Target>" + "<Source><LocURI>bar</LocURI></Source>" + "</SyncHdr>" + "<SyncBody>" + "<Results>" + "<CmdID>1</CmdID>" + "<MsgRef>1</MsgRef>" + "<CmdRef>1</CmdRef>" + "<Meta>" + "<Type>application/vnd.syncml-devinf+xml</Type>" + "</Meta>" + "<Item>" + "<Source>" + "<LocURI>./devinf12</LocURI>" + "</Source>" + "<Data>" + ; + buffer += (*pos)->fOverrideDevInfXML; + buffer += + "</Data>" + "</Item>" + "</Results>" + "</SyncBody>" + "</SyncML>"; + MemPtr_t xml = (unsigned char *)buffer.c_str(); + XltDecoderPtr_t decoder = NULL; + SmlSyncHdrPtr_t hdr = NULL; + Ret_t ret = xltDecInit(SML_XML, + xml + buffer.size(), + &xml, + &decoder, + &hdr); + if (ret != SML_ERR_OK) { + fRootElementP->setError(true, "initializing scanner for DevInf failed"); + } else { + smlFreeProtoElement(hdr); + SmlProtoElement_t element; + VoidPtr_t content = NULL; + ret = xltDecNext(decoder, + xml + buffer.size(), + &xml, + &element, + &content); + if (ret != SML_ERR_OK) { + fRootElementP->setError(true, "parsing of OverrideDevInf failed"); + } else if (element != SML_PE_RESULTS || + !((SmlResultsPtr_t)content)->itemList || + !((SmlResultsPtr_t)content)->itemList->item || + !((SmlResultsPtr_t)content)->itemList->item->data || + ((SmlResultsPtr_t)content)->itemList->item->data->contentType != SML_PCDATA_EXTENSION || + ((SmlResultsPtr_t)content)->itemList->item->data->extension != SML_EXT_DEVINF) { + fRootElementP->setError(true, "parsing of DevInf returned unexpected result"); + if (content) + smlFreeProtoElement(content); + } else { + (*pos)->fOverrideDevInfP = (SmlDevInfDevInfPtr_t)((SmlResultsPtr_t)content)->itemList->item->data->content; + (*pos)->fOverrideDevInfBufferP = content; + } + } + if (decoder) + xltDecTerminate(decoder); + } + } #endif TLocalDSList::iterator pos2; for(pos2=fDatastores.begin();pos2!=fDatastores.end();pos2++) @@ -4436,79 +4514,9 @@ localstatus TSyncSession::checkRemoteSpecifics(SmlDevInfDevInfPtr_t aDevInfP, Sm for(pos=fActiveRemoteRules.begin();pos!=fActiveRemoteRules.end();pos++) { // activate this rule TRemoteRuleConfig *ruleP = *pos; - if (!ruleP->fOverrideDevInfXML.empty()) { - // SMLTK expects full SyncML message - string buffer = - "<SyncML><SyncHdr>" - "<VerDTD>1.2</VerDTD>" - "<VerProto>SyncML/1.2</VerProto>" - "<SessionID>1</SessionID>" - "<MsgID>1</MsgID>" - "<Target><LocURI>foo</LocURI></Target>" - "<Source><LocURI>bar</LocURI></Source>" - "</SyncHdr>" - "<SyncBody>" - "<Results>" - "<CmdID>1</CmdID>" - "<MsgRef>1</MsgRef>" - "<CmdRef>1</CmdRef>" - "<Meta>" - "<Type>application/vnd.syncml-devinf+xml</Type>" - "</Meta>" - "<Item>" - "<Source>" - "<LocURI>./devinf12</LocURI>" - "</Source>" - "<Data>" - ; - buffer += ruleP->fOverrideDevInfXML; - buffer += - "</Data>" - "</Item>" - "</Results>" - "</SyncBody>" - "</SyncML>"; - MemPtr_t xml = (unsigned char *)buffer.c_str(); - XltDecoderPtr_t decoder = NULL; - SmlSyncHdrPtr_t hdr = NULL; - Ret_t ret = xltDecInit(SML_XML, - xml + buffer.size(), - &xml, - &decoder, - &hdr); - if (ret != SML_ERR_OK) { - sta = LOCERR_BADCONTENT; - PDEBUGPRINTFX(DBG_ERROR,("initializing scanner for DevInf in %s failed",ruleP->getName())); - } else { - SmlProtoElement_t element; - VoidPtr_t content = NULL; - ret = xltDecNext(decoder, - xml + buffer.size(), - &xml, - &element, - &content); - if (ret != SML_ERR_OK) { - sta = LOCERR_BADCONTENT; - PDEBUGPRINTFX(DBG_ERROR,("parsing of DevInf in %s failed",ruleP->getName())); - } else if (element != SML_PE_RESULTS || - !((SmlResultsPtr_t)content)->itemList || - !((SmlResultsPtr_t)content)->itemList->item || - !((SmlResultsPtr_t)content)->itemList->item->data || - ((SmlResultsPtr_t)content)->itemList->item->data->contentType != SML_PCDATA_EXTENSION || - ((SmlResultsPtr_t)content)->itemList->item->data->extension != SML_EXT_DEVINF) { - sta = LOCERR_BADCONTENT; - PDEBUGPRINTFX(DBG_ERROR,("parsing of DevInf in %s returned unexpected result",ruleP->getName())); - } else if (aOverrideDevInfP) { - // processing in caller will continue with updated DevInf - *aOverrideDevInfP = (SmlDevInfDevInfPtr_t)((SmlResultsPtr_t)content)->itemList->item->data->content; - } - } - if (decoder) - xltDecTerminate(decoder); - if (sta!=LOCERR_OK) { - AbortSession(sta,true); - break; - } + if (ruleP->fOverrideDevInfP && aOverrideDevInfP) { + // processing in caller will continue with updated DevInf + *aOverrideDevInfP = ruleP->fOverrideDevInfP; } // - apply options that have a value if (ruleP->fLegacyMode>=0) fLegacyMode = ruleP->fLegacyMode; diff --git a/src/sysync/syncsession.h b/src/sysync/syncsession.h index 1e92321..1ecbd67 100755 --- a/src/sysync/syncsession.h +++ b/src/sysync/syncsession.h @@ -160,6 +160,8 @@ public: // that it has sent and before applying other remote rule workarounds. // XML DevInf directly from XML config. string fOverrideDevInfXML; + SmlDevInfDevInfPtr_t fOverrideDevInfP; + VoidPtr_t fOverrideDevInfBufferP; // list of subrules to activate TRemoteRulesList fSubRulesList; // flag if this is a final rule (if matches, no more rules will be checked) -- 1.7.2.5
_______________________________________________ os-libsynthesis mailing list [email protected] http://lists.synthesis.ch/mailman/listinfo/os-libsynthesis
