-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
fix(settings): make trailing slash for caldav/carddav redirects optional #46079
Conversation
#43939 moved the CalDAV/CardDAV redirect checks from the frontend to a new backend API. Since the backend does not send an authentication header, checking for the expected response code 207 of the DAV endpoint does not work anymore, hence the URL of the last redirect is checked instead. This URL is expected to contain a trailing slash, which was not required before, since the DAV endpoint works properly without it (when authenticated). While a trailing slash in the redirect does no harm, it causes many setups to throw an admin panel warning, while in fact the redirects work properly. Furthermore, the proposed "/.well-known/carddav" => "/remote.php/dav/" redirect leads to double slashes, when doing a request to "/.well-known/carddav/", which seems more wrong then right. This change makes the trailing slash optional, hence old and adjusted setups won't throw the warning anymore, and the DAV endpoint works well in both cases. Signed-off-by: MichaIng <micha@dietpi.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
/backport to stable29 |
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
Summary
#43939 moved the CalDAV/CardDAV redirect checks from the frontend to a new backend API.
Since the backend does not send an authentication header, checking for the expected response code 207 of the DAV endpoint does not work anymore. Hence the URL of the last redirect is checked instead.
This URL is expected to contain a trailing slash, which was not required before, since the DAV endpoint works properly without it (when authenticated).
While a trailing slash in the redirect does no harm, it causes many setups to throw an admin panel warning, while in fact the redirects work properly. Furthermore, the proposed
"/.well-known/carddav" => "/remote.php/dav/"
redirect leads to double slashes, when doing a request to"/.well-known/carddav/"
, which seems more wrong than right.This change makes the trailing slash optional, hence old and adjusted setups won't throw the warning anymore, and the DAV endpoint works well in both cases.
The motivation behind the change is to prevent admins from doing practically unnecessary webserver config changes, after upgrading to Nextcloud 29, to mute the too strict setup warning.
TODO
Checklist