Unknown VCard properties are lost



  • i’ve skimmed through the relevant standards, and did not find precise
    instructions on how to apply modifications to a vcard, so unless someone
    else finds a wording to that effect, i’ll take a different route of
    argumentation:

    when a user presses save, it is up to the client to phrase the user’s
    request in protocol terms. every other program in the field that i’ve
    tested preserves unknown attributes, so a user’s expectation when he
    saves a contact is that the data he previously entered (eg in evolution)
    does not vanish. as it seems we agree that the server is within its
    rights to accept the PUT body as the new representation (which means
    dropping absent data), the user’s intention can only be phrased by
    taking the original vcard, applying his modifications, and sending that.

    even outside of the field, programs that have much harsher (ie
    unspecified) environments do their best to preserve what they don’t
    recognize (think libreoffice/word, any html editor). consider it a
    matter of doing what the user (who might not even know about vcards and
    their properties) expects.

    implementation-wise, this means either storing the full vcard (or the
    parts you can not 100% restore, but the former is probably easier), or
    fetching the vcard before sending an update, or using PATCH (of which i
    don’t know if many servers support it). it comes with resource costs,
    but the benefit is interoperability between more than just a single
    client application. (were that the goal, things would be much easier
    anyway).


  • developer

    i’ve skimmed through the relevant standards, and did not find precise
    instructions on how to apply modifications to a vcard, so unless someone
    else finds a wording to that effect, i’ll take a different route of
    argumentation: […]

    Accepted. So I’ll consider this as an enhancement request (“nice to have”), and not as a “critical bug” or “terrible violation of standards”.

    implementation-wise, this means either storing the full vcard (or the
    parts you can not 100% restore, but the former is probably easier), or

    We also have to keep in mind the resources on mobile devices are very limited, and there are often out-of-memory exceptions as soon as there are photos etc. in the VCard. Storing the whole original VCard in a column would be the best approach, but I think at least attachments would have to be removed. Does this sound reasonable for you?

    or using PATCH (of which i don’t know if many servers support it).

    This would be the best approach if the CalDAV/CardDAV specifications would have defined PATCH, but unfortunately they didn’t (one might wonder whether they left this out on purpose and why).

    it comes with resource costs, but the benefit is interoperability between more than just a single
    client application.

    Sounds good.

    (were that the goal, things would be much easier anyway).

    For me (personally), it’s a “second goal”: the first and primary one is that I can store my contacts on my own server without depending on Google. Everything else is optional, but of course, it would be good to have.



  • I do not think it is neccessary to store whole original vcard. Storing unknown properties in some unprocessed form should be enough for their reconstruction. That way there will be only one copy of each property, so large chunks of data (like photo) will not cause troubles. And client-specific properties are usualy small, so there will be no significant performance drop.

    I would not remove attachments by default, just wait whether they cause problems first.



  • We also have to keep in mind the resources on mobile devices are
    very limited, and there are often out-of-memory exceptions as soon
    as there are photos etc. in the VCard. Storing the whole original
    VCard in a column would be the best approach, but I think at least
    attachments would have to be removed. Does this sound reasonable for
    you?

    i’m not really familiar with android development, so i don’t know what
    you mean by column.

    from a generic software development point of view, i’d store it
    somewhere “far away” from the usual contact data, as most of the
    operations on contacts are reading, and the original card is only needed
    for the uploading operation. actually, i’d think of it as a cache – you
    can safely drop that information, as long as you store the etag the
    internalized version is based on. if the original vcard is present,
    fine, if not, we have to download it before the editing process can be
    finished.

    in resource constrained environments, attachments are a problem anyway
    – even if not stored, at least they get transferred in when the cards
    are downloaded initially. i’d be happy with attachments or overly large
    images being dropped if there is some kind of confirmation if it
    actually happens (“Uploading these changes will drop an attachment to
    this contact (pictures-of-my-cats.ppt, 20MB). [Save anyway] [Abort
    editing]”).

    This would be the best approach if the CalDAV/CardDAV specifications
    would have defined PATCH, but unfortunately they didn’t (one might
    wonder if they left this out on purpose and why).

    i haven’t seen PATCH in practice so far at all, but sabredav seems to
    implement it[1] using its proprietary update format. looking into might
    be worth it, but i currently can’t spare the time to implement it on
    calypso. anyway, i’m not sure the sabredav approach is suitable for
    vcards, we might as well draft something on our own, but as i said, it’s
    time consuming.

    [1] http://sabre.io/dav/http-patch/

    For me (personally), it’s a “second goal”: the first and primary one
    is that I can store my contacts on my own server without depending on
    Google. Everything else is optional, but of course, it would be good
    to have.

    i think that has been achieved by now 🙂

    best regards
    chrysn



  • rfc2822: of course you would rather see this as a feature request, or nice to have, as opposed to consider this a case of data-loss, because that would be bad.

    I’m not really interested to go into the small print of rfc’s, but I can dig and perhaps I can find one or two lines that underline the main idea I’m trying to convey.

    That’s not the point however. If you are a CardDAV client, and the first thing you do after connecting to a server is to DELETE every vcard that’s already on the system… technically you are still working within the constraints of the protocol and are standards-compliant.

    @chrysn in this regard has it right. This all depends on a users’ expectation. Based on how you choose to proceed in this matter, I would indeed at the very least always warn the user that some data may be lost after every save operation.

    As for PATCH. It’s hard! And it will not solve your problem. Take this (part of a) hypothetical vcard:

    EMAIL;TYPE=WORK;PREF=1:foo@bar.com
    EMAIL;TYPE=HOME;PREF=2:foo@bar.com
    

    In a PATCH, we’d like to update one of these. How can we easily distinguish the two? The answer that really comes down to the following:

    The only way you can semantically patch one of those lines, is if you’re updating exact byte indexes, the literal order of properties in a vcard (assuming they don’t change), or you include the exact line you’re patching, such as:

    -EMAIL;TYPE=WORK;PREF=1:foo@bar.com
    +EMAIL;TYPE=WORK;PREF=1:foo@example.com
    

    Semantic patching is, afaik, very hard… the way the format works is that you can’t really say something like “update property X and set this value” or “find property X with parameter Y and update this value” accurately.

    So as far as I can tell, the only patch mechanisms actually possible, rely on the caller (you) to have a knowledge of the exact structure of the original, which defeats the point of what you’re trying to achieve (because you discard the stuff you don’t support).

    Note that your workaround of putting properties aside you don’t support goes a long way, but only solves part of the problem, as it doesn’t cover parameters, grouping and the other items on this list.

    So yea, I’d be happy to have a chat about the details of the rfc’s and what their intended meaning is, but in the context of this ticket this is meaningless, because step 1 is to accept that it’s not about being compliant to the letter of the specs, it’s admitting you have a problem.


  • developer

    @jkufner:

    I would not remove attachments by default, just wait whether they cause problems first.

    They have caused problems already. Handling the images in the VCards requires special attention and still doesn’t work with large images flawlessly.

    @chrysn:

    i’m not really familiar with android development, so i don’t know what you mean by column.

    The contact information is not stored by DAVdroid itself, but by the Android contacts provider (“the address book”) which itself uses an SQLite database to store the information. The tables and columns must be used exactly by the Android specification so the contacts information is exchangeable between all Android apps. There are some fields/record types that allow sync adapters to store “custom” data – I’d probably use such one for the original VCard.

    i’d be happy with attachments or overly large images being dropped if there is some kind of confirmation if it actually happens (“Uploading these changes will drop an attachment to this contact (pictures-of-my-cats.ppt, 20MB). [Save anyway] [Abort editing]”).

    Normally, a sync adapter like DAVdroid (basically, just a service) should not need a (blocking) user interface (if that’s even possible, haven’t checked yet). For instance, if one app (let’s say, Samsung Kies or some other remote control app) adds a contact to the contacts provider, DAVdroid should sync it to the server without user interaction.

    @bartv2

    I’m not really interested to go into the small print of rfc’s, but I can dig and perhaps I can find one or two lines that underline the main idea I’m trying to convey.

    I have found this:

    »However, if a client needs to make a change to a vCard, it can only change the entire vCard data via a PUT request. There is no way to incrementally make a change to a set of properties within a vCard object resource. [Yes, we already know this. But now the interesting sentence:] As a result, the client will have to cache the entire set of properties on a resource that is being changed.« — RFC 6352 (CardDAV) 9.1. Restrict the Properties Returned

    This can be most reasonably interpreted as that it’s expected behaviour to retain all properties (even if there’s no MUST).

    As for PATCH. It’s hard!

    As it’s not within the scope of CardDAV, i won’t use it until there’s a) at least a draft specification for usage with CardDAV/VCards, and b) support from at least a few servers.

    So yea, I’d be happy to have a chat about the details of the rfc’s and what their intended meaning is, but in the context of this ticket this is meaningless, because step 1 is to accept that it’s not about being compliant to the letter of the specs, it’s admitting you have a problem.

    I admit that you (not you @bartv2 but all who are affected by this issue) have a problem. Personally, I don’t have one because I have enjoyed the removal of hundreds of useless X-Mozilla-*, X-Evolution-* etc. headers at every sync 🙂 Maybe that’s because I’m a minimalist — I honestly didn’t have the idea that someone will be missing these headers. (And yes, I know that DAVdroid itself uses X-DAVdroid-Starred and this will be lost when updating the VCard by other, equally-behaving clients. I didn’t think that somebody would be missing these headers, too.)

    @All:

    I want other users to have a good experience of DAVdroid. But it’s not the best idea to tell me your feedback like “Look there! A f*cking critical bug, and it deletes all data! How dare you!1! Fix it NOW, you’re my open-source slave and have to work when I say so! And by the way, you’ll have to do X and Y and Z, too, otherwise DAVdroid is crap!” when the behaviour just works fine for me, because then I’ll of course solve the conflict of your interests and my interests by asking an independent judge – namely the specs (even if I could simply ignore your feedback as well as the specs – there’s no obligation that I’ll code for you, and by the way, you could also do it yourself if its that important for you).

    So, after considering RFC 6352 9.1 (see above), I think DAVdroid really SHOULD retain all properties (meaning a higher priority than “nice to have” 😀).



  • Thank you for fixing this.



  • Thank you, now i don’t need to worry that changing contacts on my phone will lose information.


  • developer

    You’re welcome – but don’t forget to clean your VCards from time to time… they can only grow and never shrink.



  • lol



  • thanks a lot for fixing this, it makes davdroid my preferred android cal-/carddav client now.

    the attributes in known fields still do get lost (eg EMAIL;TYPE=WORK;X-EVOLUTION-UI-SLOT=1:attribute-test@example.com -> EMAIL;TYPE=work:attribute-test@example.com), and the contact photos get re-encoded (in my test run, the file grew from 1.2k to 1.5k). can we track this here or shall i open another issue for that?

    by the way, your warnings against vcards got me to think; i might add cleanup features to calypso when i find the time to factor out the import features.


  • developer

    the attributes in known fields still do get lost (eg EMAIL;TYPE=WORK;X-EVOLUTION-UI-SLOT=1:attribute-test@example.com -> EMAIL;TYPE=work:attribute-test@example.com)

    Yeah, they do. Attributes could theoretically be stored in the SYNC1 field of the email address row, but at the moment, there are other priorities. Do you really use these properties on VCards which you edit with DAVdroid or did you just test it?

    and the contact photos get re-encoded (in my test run, the file grew from 1.2k to 1.5k). can we track this here or shall i open another issue for that?

    The VCard must be parsed and then generated (each by ez-vcard), so I think re-encoding is unavoidable (given the current functionality of ez-vcard).



  • Ad attributes: I guess separate issue with low priority would be nice, just to keep the bug known. It will get lost here.

    Ad image reencoding: I did a quick look into ez-vcard and I do not see any image transformations in photo property. Only moving blob around. That should not cause loosing image quality. Maybe I missed something (it was only a brief exploration). Isn’t there some hidden reencoding when storing image into Android database?


  • developer

    Ad attributes: I guess separate issue with low priority would be nice, just to keep the bug known. It will get lost here.

    Ah, you mean a low-priority enhancement. Well why not 😛

    Isn’t there some hidden reencoding when storing image into Android database?

    Ah, that may be the case too. Actually, I didn’t look. In both cases, it can’t be changed by DAVdroid.


Log in to reply