Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Contact sync abort on single vcard error #1351

Open
2 tasks done
timedin-de opened this issue Mar 19, 2025 · 4 comments
Open
2 tasks done

Contact sync abort on single vcard error #1351

timedin-de opened this issue Mar 19, 2025 · 4 comments
Assignees
Labels
bug Something isn't working need info Further information needed to continue

Comments

@timedin-de
Copy link

Problem scope

  • I'm sure that this is a DAVx⁵ problem.

App version

  • I'm using the latest available DAVx⁵ version.

Android version and device/firmware type

Android 15 (Google Pixel 6a)

Steps to reproduce

  1. Apply the linked patch to your synced nextcloud: nextcloud/server#51478
  2. Create an invalid contact in Android (e.g., with an invalid email).
    1.5. Add a valid new contact
  3. Start a sync with DAVx⁵.
  4. Observe that the sync process gets canceled due to the invalid contact. The valid contact will not get synced

Actual result

Currently, when a put/post request fails, the entire sync process is aborted, and changes to other contacts (different http-requests) are not synced.

Expected result

I would expect DAVx⁵ to skip the invalid contact and continue syncing the remaining valid contacts instead of aborting the entire sync process.

Maybe: Extract the error message and show it directly in the notification

Further info

  • Although in this case its about nextcloud, the same errror would happen with other carddav servers that do server-side validation.
  • Related issue: nextcloud/server#51478
@timedin-de timedin-de added the bug Something isn't working label Mar 19, 2025
@ArnyminerZ
Copy link
Member

I have tried reproducing, and for me it is working as expected. Steps:

  1. Add a Nextcloud account to DAVx⁵
  2. Synchronize an address book
  3. Add a contact with an invalid email address (added a contact with "invalid" as email)
  4. Synchronize

And everything is working well. What is your issue? Does the PR mentioned have to be merged on Nextcloud before so that this error occurs?

We cannot implement "fixes" for things that still do not happen. Nextcloud could simply never merge the PR, and nothing would have been updated. Also, without reproduction steps, we cannot verify whether this is an issue or not.

@timedin-de
Copy link
Author

timedin-de commented Mar 20, 2025

And everything is working well. What is your issue? Does the PR mentioned have to be merged on Nextcloud before so that this error occurs?

We cannot implement "fixes" for things that still do not happen. Nextcloud could simply never merge the PR, and nothing would have been updated. Also, without reproduction steps, we cannot verify whether this is an issue or not.

Sorry if i didn't make the point clear.

Yes for my reproduction steps, it's needed to merge my mentioned nextcloud PR. But the Problem I see doesn't need nextcloud by itself, just a example server.

My Problem is a unclear error handling, if (any Server) would reject a new/updated CardDav (theoretically the same for CalDav) all sync for that server is aborted.

Instead I think, when it updates to server, and the server returns a 4xx, it should just abort that one Contact/Calender. #

Current: DavX⁵ Updates a VCard ----> PUT to DavServer ---> DavServer Returns error 4xx ----> DavX⁵ stops ALL syncronisation

Expected: DavX⁵ Updates a VCard ----> PUT to DavServer ---> DavServer Returns error 4xx ----> DavX⁵ skips this VCard, continues with next vcard

@rfc2822 rfc2822 added the need info Further information needed to continue label Apr 7, 2025
@rfc2822 rfc2822 self-assigned this Apr 7, 2025
@rfc2822
Copy link
Member

rfc2822 commented Apr 7, 2025

Create an invalid contact in Android (e.g., with an invalid email).

To understand that correctly: what is an invalid contact / invalid email? Invalid in which sense? For CardDAV, it would be relevant whether the contact / email is a valid vCard. The vCard generated by DAVx5 should never contain syntactically invalid fields (like emails) that make the vCard unparseable.

If it's about server-side data validation: As far as I know, WebDAV/CardDAV doesn't specify this behavior and how it should be done. So I have to investigate further. However clients (also other clients) can't be expected to react in a specific way. However the correct usage of status codes may make things more easy.


Current: DavX⁵ Updates a VCard ----> PUT to DavServer ---> DavServer Returns error 4xx ----> DavX⁵ stops ALL syncronisation

It depends on the actual status code. 400 is a Bad Request. It leaves the synchronization in an undefined state, as DAVx5 doesn't know what happened.

I don't think 400 is the correct status code, because 400 means that the message body is syntactically incorrect [https://www.rfc-editor.org/rfc/rfc4918#section-11.2].

Expected: DavX⁵ Updates a VCard ----> PUT to DavServer ---> DavServer Returns error 4xx ----> DavX⁵ skips this VCard, continues with next vcard

But that would mean that either

  • the vCard would be uploaded at every sync, forever (imagine it contains a big photo and is uploaded every 15 min – it will drain battery and data usage)
  • or we keep an unsynchronized state (vCard not uploaded corretly) as "synchronized". People may be confused because DAVx5 didn't show an error although synchronization was not successful.

What do you think?

@timedin-de
Copy link
Author

timedin-de commented Apr 7, 2025

To understand that correctly: what is an invalid contact / invalid email? Invalid in which sense? For CardDAV, it would be relevant whether the contact / email is a valid vCard. The vCard generated by DAVx5 should never contain syntactically invalid fields (like emails) that make the vCard unparseable.

Invalid to the EMail RFC, like spaces at start/end, no @, unable to send messages to.... I know the whole CardDav Scheme has no such validation for any fields.

I don't think 400 is the correct status code, because 400 means that the message body is syntactically incorrect [https://www.rfc-editor.org/rfc/rfc4918#section-11.2].

Ok I'll discuss it again.

or we keep an unsynchronized state (vCard not uploaded corretly) as "synchronized". People may be confused because DAVx5 didn't show an error although synchronization was not successful.

I'd always still show an error, the question is just how, currently the user just sees an error (yes on detailed inspection they can see the corresponding VCard, but at first it just looks like a broken server, which is wrong on 4XX).

I already realize it's not as straightforward as I thought, considering the RFCs etc. Weird that no (common) system has something similar implemented. But I think it would be a helpful somewhat important feature to have (and a starting point for other to continue).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working need info Further information needed to continue
Projects
None yet
Development

No branches or pull requests

3 participants