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

Don't repair RFC-conform UIDs by default #527

Closed
equaeghe opened this Issue Dec 5, 2016 · 4 comments

Comments

Projects
None yet
2 participants
@equaeghe

equaeghe commented Dec 5, 2016

Why is @ not in the list of safe chars for UIDs? (See PR #231.) If one follows the RFC's recommendations (see https://tools.ietf.org/html/rfc5545#section-3.8.4.7), then an @ is bound to be there. Now what happens when doing a repair, e.g., due to some duplicate UID, then almost all entries will have their UID changed. In case the entry creator used a creation date as the start of the UID (as recommended), this actually causes convenient info to be lost. Namely, if one looks at the entries in the directory listing, one won't be able to use a nice ordering.

I'd like to request putting @ in the list of safe chars for UIDs.

I'm aware that @ is a reserved character and that means that in principle it should be encoded in URLs, but this consideration should not affect restrictions imposed on the UID.

@untitaker

This comment has been minimized.

Member

untitaker commented Dec 5, 2016

Some servers break with @ in UIDs.

I'd like to request putting @ in the list of safe chars for UIDs.

Why? What do you gain from this?

untitaker added a commit that referenced this issue Dec 5, 2016

@untitaker

This comment has been minimized.

Member

untitaker commented Dec 5, 2016

I'm aware that @ is a reserved character and that means that in principle it should be encoded in URLs,

Also this is wrong, it's a totally safe character in URL paths in theory.

In the past vdirsyncer attempts to make href and UID equal when uploading items, and I don't see a reason to give up this invariant.

@equaeghe

This comment has been minimized.

equaeghe commented Dec 5, 2016

Some servers break with @ in UIDs.

I guess you're referring to Issue #526. Well, that is a server bug, no? If the server can't deal with UIDs in a form that is recommended by the RFC…

Why? What do you gain from this?

  • More informative directory listings are preserved (in case server uses RFC recommendation for UID)
  • Information about where the entry was created is stored in the UID, which may be useful for analysing issues.
  • vdirsyncer's repair functionality not touching entries that don't need to be touched and therefore being less invasive (less chance of messing things up) and running quicker
  • Other clients not losing their UID-based state; my phone's common correspondent list was reset after doing a repair to fix a single duplicate UID (DAVdroid as client) which is quite bothersome, so this did affect me.

Also, we don't how do the myriad of clients react when they see all the UIDs have changed. I guess they think all existing entries are gone and an equal number of new ones appeared. Again, any state based on UID will be lost. And keeping state based on UID is not crazy, as I think UIDs are assumed to be ‘somewhat constant’ next to being unique.

[@ is] a totally safe character in URL paths in theory.

I think I agree with you here, https://tools.ietf.org/html/rfc3986#section-2.2 says:

If data for a URI component would conflict with a reserved character's purpose as a delimiter, then the conflicting data must be percent-encoded before the URI is formed.

Given that @ would not conflict when in the path part of a URL, it does not need to be encoded.

@untitaker

This comment has been minimized.

Member

untitaker commented Dec 5, 2016

Well, that is a server bug, no? If the server can't deal with UIDs in a form that is recommended by the RFC…

Unfortunately those servers are sufficiently popular to make this such a practical issue such that the principle of only following the RFC is (in my view) outweighed.

Or rather, they may not even be that popular, but I got enough bugreports about this to be annoyed enough to add this fixer.

Information about where the entry was created is stored in the UID, which may be useful for analysing issues.

Vdirsyncer already maintains the invariant that href == uid, so this advantage is still there.

Other clients not losing their UID-based state; my phone's common correspondent list was reset after doing a repair to fix a single duplicate UID (DAVdroid as client) which is quite bothersome, so this did affect me.

That is a very good argument. I didn't realize that I would actually break anything with doing "too much" repair. I think we should have this repair routine disabled by default then.

Given that @ would not conflict when in the path part of a URL, it does not need to be encoded.

The argument is even simpler. If you look at the ABNF for pchar, you'll see that @ is explicitly allowed.

@untitaker untitaker changed the title from Why is @ not in list of safe chars for UIDs? to Don't repair RFC-conform UIDs by default Dec 5, 2016

@untitaker untitaker closed this in 96bf21c Dec 6, 2016

@untitaker untitaker removed the ready label Dec 6, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment