nextcloud / calendar

📆 Calendar app for Nextcloud
https://apps.nextcloud.com/apps/calendar
GNU Affero General Public License v3.0
972 stars 240 forks source link

Links from mail or activity to events broken if calendar name contains Umlauts #6073

Closed XueSheng-GIT closed 3 months ago

XueSheng-GIT commented 3 months ago

Steps to reproduce

  1. User2 has turned on notifications for calendars (email+activity stream)
  2. User1 creates a calendar which contains Umlauts or special characters
  3. User1 shares this calendar with User2
  4. User1 creates an event in this shared calendar
  5. User2 receives an email with link and sees the link in the activity stream
  6. User2 clicks on one of those links
  7. Calendar loads and error "Event does not exist" appears

EDIT The same applies if User1 creates an event in his own calendar. User1 will not be able to open that event from activity stream!

Expected behavior

Shared event should just load without error.

Actual behaviour

Event is not loaded, but error "Event does not exist" appears.

Calendar app version

4.7.6

CalDAV-clients used

DAVx5

Browser

Firefox 127.0

Client operating system

Ubuntu 24.04

Server operating system

Ubuntu

Web server

None

Database engine version

None

PHP engine version

PHP 8.1

Nextcloud version

29.0.3.rc2

Updated from an older installed version or fresh install

Updated from an older version

List of activated apps

No response

Nextcloud configuration

No response

Web server error log

No response

Log file

No response

Browser log

No response

Additional info

Follow up issue to https://github.com/nextcloud/calendar/issues/5293 which was fixed by https://github.com/nextcloud/server/pull/45775. But shared calendars with Umlauts or special characters seem to need an additional url encoding. See also further explanation: https://github.com/nextcloud/calendar/issues/5293#issuecomment-2178965867

XueSheng-GIT commented 3 months ago

While digging deeper into this issue, I realized that this does not only affect shared calendars, but also the own calendar. If user1 (owner) of a calendar with Umlauts in its name creates an event, user1 himself will not be able to open that event in the activity stream.

XueSheng-GIT commented 3 months ago

I'm not a programmer but had a quick look at the code. Following lines include $linkData['calendar_uri'] which seems not to be urlencoded. Thus, the base64 encoded string will not match the event. https://github.com/nextcloud/server/blob/3a975d0a89e4576f60d6b9628c3de902e683fdc0/apps/dav/lib/CalDAV/Activity/Provider/Event.php#L79-L87

For example: Link of activity stream: (which fails with "Event does not exist") /remote.php/dav/calendars/user1/persönlich_shared_by_user2/EDCD453A-665E-4CD7-93D5-9DEB27E908F2.ics

Actual link if opened directly in calendar: /remote.php/dav/calendars/user1/pers%c3%b6nlich_shared_by_user2/EDCD453A-665E-4CD7-93D5-9DEB27E908F2.ics

Just adding urlencode does not solve the issue, because the result will be upper case. This will still result in a different base64 encoded string. Thus, strtolower is needed too.

My quick adaption of the above mentioned lines now look like:

if (isset($eventData['link']) && is_array($eventData['link']) && $this->appManager->isEnabledForUser('calendar')) {
        try {
                // The calendar app needs to be manually loaded for the routes to be loaded
                OC_App::loadApp('calendar');
                $linkData = $eventData['link'];
                if ($affectedUser === $linkData['owner']) {
                        $objectId = base64_encode($this->url->getWebroot() . '/remote.php/dav/calendars/' . $linkData['owner'] . '/' . strtolower(urlencode($linkData['calendar_uri'])) . '/' . $linkData['object_uri']);
                } else {
                        // Can't use the "real" owner and calendar names here because we create a custom
                        // calendar for incoming shares with the name "<calendar>_shared_by_<sharer>".
                        // Hack: Fix the link by generating it for the incoming shared calendar instead,
                        //       as seen from the affected user.
                        $objectId = base64_encode($this->url->getWebroot() . '/remote.php/dav/calendars/' . $affectedUser . '/' . strtolower(urlencode($linkData['calendar_uri'])) . '_shared_by_' . $linkData['owner'] . '/' . $linkData['object_uri']);
                }
                $link = [
                        'view' => 'dayGridMonth',
                        'timeRange' => 'now',
                        'mode' => 'sidebar',
                        'objectId' => $objectId,
                        'recurrenceId' => 'next'
                ];
                $params['link'] = $this->url->linkToRouteAbsolute('calendar.view.indexview.timerange.edit', $link);
        } catch (\Exception $error) {
                // Do nothing
        }
}

Some quick tests are positive on my testing instance. As said, I'm not a programmer and don't know whether there's a better solution or whether even more parts must be encoded differently to catch further corner cases.

@st3iny does this sound reasonable to you?