Ticket #1027 (new defect)

Opened 3 years ago

Last modified 3 years ago

[NEEDINFO] xmlformat/opensync_xmlformat.c:170:E:osync_xmlformat_search_field: Assertion "xmlformat->sorted" failed

Reported by: mkoller Owned by: cstender
Priority: normal Milestone: Plugin Format: vformat 0.40
Component: Format Plugin: vformat: contact Version: 0.39
Severity: critical Keywords:
Cc:

Description

When syncing with kdepim/some-other-plugin-e.g.-file-sync and I only sync contacts, I get this assert.

Part of the backtrace:

#3  0xb7ee03af in osync_xmlformat_search_field (xmlformat=0x831a008, name=0xb7930605 "Organization", error=0xb57821f4)
    at /home/PACKAGES/opensync/opensync/opensync/xmlformat/opensync_xmlformat.c:166
#4  0xb7921761 in handle_kde_organization_attribute (xmlformat=0x831a008, attr=0x8315c20, error=0xb57821f4)
    at /home/PACKAGES/opensync/vformat/src/xmlformat-vcard.c:391
#5  0xb791a402 in handle_attribute (attrtable=0x82f9340, paramtable=0x82f9368, xmlformat=0x831a008, attr=0x8315c20, 
error=0xb57821f4)
    at /home/PACKAGES/opensync/vformat/src/xmlformat-common.c:300
#6  0xb791eed8 in conv_vcard_to_xmlformat (

Change History

comment:1 Changed 3 years ago by dgollub

  • Status changed from new to assigned

This could be related to the multi-level support of xmlfield/xmlformat, which got introduced recently.

This is for sure not a vfomrat-plugin issue.

comment:2 Changed 3 years ago by dgollub

  • Component changed from OpenSync to OpenSync: XMLFormat API

comment:3 Changed 3 years ago by dgollub

  • Severity changed from normal to critical
  • Milestone set to OpenSync 0.39

comment:4 Changed 3 years ago by dgollub

Reproduce with vconvert tool:

vformat/build/tools/vconvert ~/Documents/Crash_Dummy.vcf --to-xmlformat --format-config VCARD_EXTENSION=KDE

Crash_Dummy.vcf

BEGIN:VCARD
CLASS:PUBLIC
EMAIL:foo@bar.de
FN:Crash Dummy
N:Dummy;Crash;;;
ORG:Crash Inc.
UID:POPncHmSO0
VERSION:3.0
END:VCARD

It only fails when the format-config with KDE extension is set. Since only this seems to call osync_xmlformat_search_field.

(Actually i wonder why there is a special ORG handler for KDE vcards at all.. it's not a X- field)

comment:5 Changed 3 years ago by dgollub

Even if the assert() points to line 166, the bad condition is in line 170.

166             osync_assert(name);
167
168             /* Searching breaks if the xmlformat is not sorted (bsearch!).
169                      ASSERT in development builds (when NDEBUG is not defined) - see ticket #754. */
170             osync_assert(xmlformat->sorted);
(gdb) p xmlformat->sorted
$5 = 0
(gdb) p name
$6 = 0xb79ce855 "Organization"

Need to check where the sorted-attribute information get lost. But in testcase the result was sorted.

comment:6 Changed 3 years ago by dgollub

  • Owner changed from dgollub to cstender
  • Status changed from assigned to new
  • Component changed from OpenSync: XMLFormat API to Format Plugin: vformat
  • Summary changed from xmlformat/opensync_xmlformat.c:170:E:osync_xmlformat_search_field: Assertion "xmlformat->sorted" failed to [NEEDINFO] xmlformat/opensync_xmlformat.c:170:E:osync_xmlformat_search_field: Assertion "xmlformat->sorted" failed

This is a vformat-xmlformat conversion bug.

The osync_xmlformat_search_field() requires that the xmlformat is sorted, due to the binary search. Since in the conv_vcard_to_xmlformat() a new xmlformat object get assemble it's not garunteed that the assembling is done in a sorted way. For that reason by default the xmlformat->sorted value is set to 0.

We had in the past several discussion about this sorting state. Currently as temp. solution we sort at the _end_ of the vcard -> conversion. But the "Organization" fields for some extension run those osync_xmlformat_search_field to check if there is already a ORG: entry.

Christopher, is there really need to call _search_field? Why not just create a new Organization XMLField? Or can't we pass-through the XMLField pointer of the Organization node to the attribute handler? This usecase would require that we sort on each "search"... And i really doubt that several ORG: nodes should later share one <Organization> node.

Need to fix the vformat testsuite.. will check the code-coverage if there is _any_ codepath using this case.

Martin, workaround - bfore line xmlformat-vcard.c:391 add:

osync_xmlformat_sort(xmlformat);

comment:7 Changed 3 years ago by dgollub

  • Milestone OpenSync 0.39 deleted

comment:8 Changed 3 years ago by dgollub

Ok - in case of handle_kde_organization_attribute this search seems to be not required at all.

228 calls end up in 228 finding nothing and adding a new ORG: field.

I'll drop the search for handle_kde_organization_attribute. The other search's for the KDE extension - e.g. Office/Department? - seems to be valid. For this a sort would be required or somehow the pointer to the Organization node passed. How many Organization are allowed in a vCard?

           static OSyncXMLField *handle_kde_organization_attribute(OSyncXMLFormat *xmlformat, VFormatAttribute *attr, OSyncError **error)
00000228   {
00000228   	osync_trace(TRACE_INTERNAL, "Handling Organization attribute");
00000228   	OSyncXMLField *xmlfield = NULL;
           
           	//We need to check first if the node already exists.
00000228   	osync_xmlformat_sort(xmlformat);
00000228   	OSyncXMLFieldList *list = osync_xmlformat_search_field(xmlformat, "Organization", error, NULL);
00000228   	if (!list)
00000000   		goto error;
           
00000228   	xmlfield = osync_xmlfieldlist_item(list, 0);
00000228   	osync_xmlfieldlist_free(list);
00000228   	if(xmlfield == NULL) {
00000228   		xmlfield = osync_xmlfield_new(xmlformat, "Organization", error);
00000228   		if(!xmlfield)
00000000   			goto error;
           	}
[...]

comment:9 Changed 3 years ago by dgollub

Ok, i don't touch the xmlformat-plugin. I guess the search is done the extrem rare case that the X-KDE fields are on top. We would need to check how the kdepim code assemble the vCard. My John Doe example seems to be sorted. Which seems to be safe:

BEGIN:VCARD
CLASS:PUBLIC
EMAIL:foo@bar.de
FN:Crash Dummy
N:Dummy;Crash;;;
ORG:Crash Inc.
UID:POPncHmSO0
VERSION:3.0
X-KADDRESSBOOK-X-Office:Lab
X-KADDRESSBOOK-X-Profession:Intern
END:VCARD

This vcard got created by using the "Export" function. Martin, could you check if a contact like this is also via the kdepim3-interface sorted? (Could you also check if your version of kdepim3 also doesn't Export -X-Department? I set this in KAddressbook, but it didn't got exported? kdepim3 bug?)

If the output of the vcard from kdepim is always sorted i would suggest to drop the ORG: KDE-hook and use the standard Hook. Which doesn't use a sort. For the KDE Department and Office hooks i would suggest to an ugly osync_xmlformat_sort for now. Longterm fix would be to pass-through the Office pointer to the office/dep.-attribute hook.

comment:10 follow-up: ↓ 11 Changed 3 years ago by mkoller

In the KDE source the output seems to be written in a sorted way - although this is the current implementation and as it is not specified as a must in the vcard specification, I think it would be dangerous to assume the sorted fields. In my KDE-3.5.10 the Department is not an X- field but is part of the ORG field,

e.g. ORG:Company;Department

which is according to the vcard spec "Organization Name and Organizational Unit"

comment:11 in reply to: ↑ 10 Changed 3 years ago by dgollub

Replying to mkoller:

In the KDE source the output seems to be written in a sorted way - although this is the current implementation and as it is not specified as a must in the vcard specification, I think it would be dangerous to assume the sorted fields.

I wonder if we should sort the parsed vformats before starting the field conversion. This would make it safe and we would avoid to search for all those X-fields which need to get appended to a standardized field.

In my KDE-3.5.10 the Department is not an X- field but is part of the ORG field,

e.g. ORG:Company;Department

which is according to the vcard spec "Organization Name and Organizational Unit"

Ok, then it's broken for kdepim 3.5.9. At least when i'm using the vcard export function from kaddressbook - not the interface it self. Not quite sure if this is the same code. Thinking more about this i doubt this, since otherwise it would get not stored at all. Could you try the kaddressbook export function if you have the same issue with 3.5.10?

comment:12 Changed 3 years ago by mkoller

Ah yes, you're right. When I use the export function, the Department part is lost. I'll check the KDE sources ...

comment:13 Changed 3 years ago by mkoller

ok, fixed the department export in KDE4 kaddressbook (trunk, branch 4.1, branch 4.2)

comment:14 Changed 3 years ago by cstender

  • Component changed from Format Plugin: vformat to Format Plugin: vformat: contact
Note: See TracTickets for help on using tickets.