Please provide detailed instructions on how to reproduce in a separate issue. Please also read https://davdroid.bitfire.at/faq/entry/recurring-events
URL should only be updated for 301, 308 redirects when during validation
-
When a new calendar is added, validation updates the URL if the HTTP response is a redirect. The issue is that it does so without looking at the HTTP status code:
https://gitlab.com/bitfireAT/icsx5/blob/master/app/src/main/java/at/bitfire/icsdroid/ui/AddCalendarValidationFragment.kt#L116The URL should only be updated for 301 and 308 (and not for 302 and 307).
This is done better for actually fetching the calendar later. Here, 308 (
== StatusLine.HTTP_PERM_REDIRECT
) is handled. I think 301 should be added here:
https://gitlab.com/bitfireAT/icsx5/blob/master/app/src/main/java/at/bitfire/icsdroid/ProcessEventsTask.kt#L81Maybe it’s easiest to handle all of this only when fetching and not when validating, not sure.
The concrete issue I have is with a “link share” on Seafile installation. When accessing the original URL1, Seafile redirects you to URL2 and generates an access token that is valid for accessing URL2 for only one hour. So when ICSx5 replaces the URL during validation, everything just works for one hour. After one hour, Seafile returns 400 because the access token has expired. (Yes, it should probably return 403 but that’s a different issue.)
-
Thanks for the suggestion!
-
I’m willing to contribute a patch.
Does the following sound good?
- refactor
onRedirect
ofCalendarFetcher
intoonNotModified
(304),onTempRedirect
(302 and 307)onPermRedirect
(301 and 308) - change
AddCalendarValidationFragment
andProcessEventsTask
to overrideonPermRedirect
the and update URL within the method - change
ProcessEventsTask
to overrideonNotModified
to handle 304 like previously
How should I send the patch?
- refactor
-
@real-or-random Thanks, that would be fine! I have sent an invite on gitlab.com; if you accept, you should be able to send a Merge Request from your fork. Otherwise please just post the patch (diff) here.
-
Has there already been submitted a merge request or patch?
I have the same issue with redirection to a temporary calendar, which is no longer accessible after a while. -
At least I didn’t see any merge requests.
-
Sorry, I couldn’t find time so far… This is still on my list, and I’m still personally affected.
I hope I have more time in December.
-
@rfc2822 @mica I’ve created a merge request here:
https://gitlab.com/bitfireAT/icsx5/-/merge_requests/1(I didn’t pass CI but that’s because I don’t have all the repos. I should run in your pipeline, not in mine…)
-
Thanks, I’ll have a look soon.