Handle notification params replacement being null

[tcitworld]

Unclear how & why this happens, but at least now it's handled.

TypeError: htmlspecialchars() expects parameter 1 to be string, null given in /var/www/nextcloud/apps/notifications/lib/MailNotifications.php:372
Stack trace:
#0 /var/www/nextcloud/apps/notifications/lib/MailNotifications.php(372): htmlspecialchars(NULL)
#1 /var/www/nextcloud/apps/notifications/lib/MailNotifications.php(333): OCA\Notifications\MailNotifications->replaceRichParameters(Array, '{actor} a mis \xC3...')
#2 /var/www/nextcloud/apps/notifications/lib/MailNotifications.php(312): OCA\Notifications\MailNotifications->getHTMLSubject(Object(OC\Notification\Notification))
#3 /var/www/nextcloud/apps/notifications/lib/MailNotifications.php(278): OCA\Notifications\MailNotifications->getHTMLContents(Object(OC\Notification\Notification))
#4 /var/www/nextcloud/apps/notifications/lib/MailNotifications.php(211): OCA\Notifications\MailNotifications->prepareEmailMessage('<uid>', Array, 'fr', 'Europe/Paris')
#5 /var/www/nextcloud/apps/notifications/lib/MailNotifications.php(166): OCA\Notifications\MailNotifications->sendEmailToUser(Object(OCA\Notifications\Model\Settings), Array, 'fr', 'Europe/Paris')
#6 /var/www/nextcloud/apps/notifications/lib/BackgroundJob/SendNotificationMails.php(49): OCA\Notifications\MailNotifications->sendEmails(500, 1653305410)
#7 /var/www/nextcloud/lib/public/BackgroundJob/Job.php(79): OCA\Notifications\BackgroundJob\SendNotificationMails->run(NULL)
#8 /var/www/nextcloud/lib/public/BackgroundJob/TimedJob.php(95): OCP\BackgroundJob\Job->execute(Object(OC\BackgroundJob\JobList), Object(OC\Log))
#9 /var/www/nextcloud/cron.php(150): OCP\BackgroundJob\TimedJob->execute(Object(OC\BackgroundJob\JobList), Object(OC\Log))
#10 {main}

[tcitworld]

Integration tests failure don't seem to be related?

[nickvergessen]

Can you try to catch the case to get the details which notification is a problem?
We need to fix the implementation instead of silencing it.

[tcitworld]

The activity in question that generates the notification seems to be the following:

key	value
activity_id	<an id>
timestamp	<timestamp>
priority	30
type	calendar_event
user	<user id 1>
affecteduser	<user id 2>
app	dav
subject	object_update_event
subjectparams	{"actor":"<user id 1>", "calendar":{"id":<calendar id>,"uri":"<calendar uri>","name":"<calendar name>"},"object":{"id":"<vobject uuid>","name":"<vobject name>","classified":false,"link":{"object_uri":"<object uuid>.ics","calendar_uri":"<calendar uri>","owner":"<user id 1>"}}}
message	null
messageparams	[]
file	null
link	null
object_type	calendar
object_id	<calendar id>

The notification is sent to User 2. User 1 has shared the calendar with user 2, and updated an event in that calendar. The notification itself is properly displayed.

Am I understanding this correctly in thinking this is because actor is not an array but a regular string?

[nickvergessen]

In the database acotr as string is fine.
Will have a look if i can reproduce anything

[nickvergessen]

Can you confirm that the calendar name and object name are real strings (and not null) and displayname of the user 1 is set to a non-empty string?

I still don't get how a null could get in there.

[nickvergessen]

Two issues I stumbled over:

• Save parameters also in the notifications activity#812
• Fix handling events without names in activities server#32580

But nothing that would bring in a null which then throws...

Also I would tend to just go with a check to use the ID if name is null/empty string to help more with debugging, instead of having untranslated parameters afterwards in the strings.

[tcitworld]

Can you confirm that the calendar name and object name are real strings (and not null) and displayname of the user 1 is set to a non-empty string?

That's the issue, user 1's displayname is indeed null.

[nickvergessen]

Which use backend are you using? The Displayname normally falls back to the uid

[tcitworld]

Regular database backend, running on 23.0.4.

[tcitworld]

I can't see how this happens either, following the flow seems to work as expected.

I'll try to reproduce with the actual activity data to make sure.

[nickvergessen]

https://github.com/nextcloud/server/blob/9ec0cb0a90ecdc7f4181ce8399bfe0c3a35842a2/lib/private/User/User.php#L143-L149

Is the user id null as well? Otherwise I can not see how this could happen

[tcitworld]

It's not. Maybe it's something else.

[nickvergessen]

Closing for now