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

[buteo-sync-plugin-caldav] Avoid '/' character in URLs coming from lo… #25

Merged
merged 1 commit into from Jun 27, 2023

Conversation

dcaliste
Copy link
Contributor

…cal UIDs.

When an incidence is imported, its UID can be anything. If it contains '/' characters, pushing this event to an URL created from the UID will fail. Simply replace '/' with '-' when forging an URL for a locally added
incidence.

@pvuorela , I found another faulty corner case. I imported a event with an UID like 11/07/2023 08:30:00damien.caliste@cea.frEvènement : entretiens vélos personnels0,70554750,533424 (I received it as a meeting appointment from my work).I checked the RFC5545, and it seems that UID can contain '/' characters. Of course pushing such an event to the server at a URL forged from the UID, like /caldav/Y2FsOi8vMC80NQ/11/07/2023... failed. As mentioned in the comment, I first tried to percent encode the UID, which work when sending the event to the server. But the multiget request (to get the etag) to the percent encoded URL returned a 404 error, because it decoded the URL server-side and tried to look for something at /caldav/Y2FsOi8vMC80NQ/11/07/2023.... I'm not sure if it's an issue from the server or if we should double encode the URL in the multiget request. But the simpler solution was to just replace any '/' with something less harmfull.

…cal UIDs.

When an incidence is imported, its UID can be anything.
If it contains '/' characters, pushing this event to an
URL created from the UID will fail. Simply replace '/'
with '-' when forging an URL for a locally added
incidence.
Copy link
Contributor

@pvuorela pvuorela left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems sane.

@pvuorela pvuorela merged commit d64c736 into sailfishos:master Jun 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants