owncloud sharing notification emails without shared resource

[kalletabur]

Our team has experienced weird notification emails - email subject doesn't have shared resource name and link (named "View it!") is broken

Email:
Email subject: John Doe shared »« with you
Body:
Hey there,
just letting you know that John Doe shared with you.
View it!
Cheers!

Links:
https://serverURL/index.php/apps/files?dir=
and
https://serverURL/index.php/apps/files?dir=%2F&scrollto=

Such emails are not sent everyday but still on regular bases over month already. These emails started to appear after upgrade from 6 to 7.
Maybe related to issue #11757

ownCloud log (data/owncloud.log)

{"reqId":"542a8145d6f1f","app":"mail","message":"Mail from John Doe ([email protected]) to: Jane Doe([email protected]) subject: John Doe shared \u00bb\u00ab with you","level":0,"time":"2014-09-30T13:09:20+03:00","method":"POST","url":"\/index.php\/core\/ajax\/share.php"}
{"reqId":"542a8145d6f1f","app":"PHP","message":"Undefined offset: 0 at \/var\/www\/owncloud\/lib\/private\/share\/mailnotifications.php#90","level":3,"time":"2014-09-30T13:09:20+03:00","method":"POST","url":"\/index.php\/core\/ajax\/share.php"}

Server configuration

Debian 7.6 + nginx + php5-fpm (5.4.4-14) + MySQL + ownCloud 7.0.2-1

[karlitschek]

@nickvergessen any idea?

[nickvergessen]

No idea here.
Codewise it looks like they are sharing something they can't see.
Maybe some kind of race-condition with stuff being unshared while someone tries to reshare:
https://github.com/owncloud/core/blob/master/lib/private/share/mailnotifications.php#L89-L90

But I guess we could throw an exception there, if empty($items), before trying to use the first entry of the item.

[popov-d]

I also have this bug. And even a fix.

The broken links and log messages with "Undefined offset" happen, when sharing a file or directory to a group, not a personal user. Debian 7.7 + Apache 2.2.22 + php5.4.34-0+deb7u1 + mysql-server-5.5 + 7.0.2-1 (installed and updated from repository download.opensuse.org).

A small patch on /var/www/owncloud/lib/private/share/share.php:

@@ -320,10 +320,11 @@
                                                FROM
                                                `*PREFIX*share`
                                                WHERE
-                                               `item_source` = ? AND `item_type` = ? AND `share_with` in (?)'
+                                               `item_source` = ? AND `item_type` = ? AND `share_with` in (' .
+                                               implode(',', array_fill(0, count($groups), '?')) . ')'
                                        );

-                       $result = \OC_DB::executeAudited($query, array($itemSource, $itemType, implode(',', $groups)));
+                       $result = \OC_DB::executeAudited($query, array_merge(array($itemSource, $itemType), $groups));

                        while ($row = $result->fetchRow()) {
                                $shares[] = $row;

Group list passwed to SQL as a string like "group1,group2,..." which should be substituted to the "in (?)" part of select statement. That seems to work when list is empty or contains only one element. But when there are 2 or more (recipient user is member of 2 or more groups), it fails silently. I think SQL searches for one group with name "group1,group2,..." and finds nothing. It happens in OC\Share\Share::getItemSharedWithUser(). The calling code in mailnotifications.php finds nothing where it expects an array with at least one element [0], so we get a log message.

My version converts a sequence of groups into a sequence of SQL parameter of the same length. I should note that I tested it only with MySQL and I am not sure it will work in all environments. Nor am I sure it is a best solution. But hope it shows the idea at least. Works ok for my server.

Checking for empty($items) won't fix the bug completely, it may just suppress log warning, but will still break functionality. Is far as I see, the general process is:

• A share is created and written to oc_share during previous operation.
• User tells to notify a group about this share.
• Group is expanded into a list of users.
• For each user
  • The share is searched in DB to get back file_target, permissions, and expiration. First field is used to generate links, failure to search leads to broken links. Share is search by id number, item type and user. This operation works when sharing to a separate user.
  • If previous shearch fails, DB table is search by all groups of current user. Required group is among them, so we must find the share if code is not broken. Again file_target (path to shared file in common case) is taken from found record.
  • If second search fails, in broken case, function returns empty array. But it can't be empty if a request was generated for a really existing share and a really existing group, who's member was taken at current iteration. So this is a bug.

[LukasReschke]

@popov-d That's great. Thanks for helping debugging this!

May I ask you to create a Pull Request with your suggested changes? - We can then easier review it and if we agree that this is the right approach merge it right away. Thank you!

[popov-d]

Sorry, I am a sysadmin, not a developer. Unfortunately I am not used with git, just pulled from repositories to me several times. It will take some time for me to learn, not today anyway.

[LukasReschke]

@popov-d No problem. What needs to be done is:

1. Fork the repository: https://help.github.com/articles/fork-a-repo/
2. Submit a pull request: https://help.github.com/articles/using-pull-requests/

If you need any help feel free to ask! - We're also always at #owncloud-dev on the Freenode IRC. (but it might take some time until someone answers ;-))

[popov-d]

It seems I created something, #12030. Not sure I've done everything correctly, but my changes are there. Hope my commit won't break anything else.

[LukasReschke]

Thanks a lot. That's correct that way. - We just require your license statement on this PR and then we can review this :-)

[popov-d]

As I understand, you mean this: http://owncloud.org/contribute/agreement/ . Just sent a scanned image to [email protected] accordingly.

[LukasReschke]

Yes. Either that (preferred) or state that the contribution is MIT licensed. But this approach is preferred as this will also work for future contributions.

[kalletabur]

I tried that patch (12030) but it failed. Is the issue still under investigation?

[popov-d]

Sorry for a delay, I was on vacation. But just before it my production machine received an update to 7.0.3-1 from a repository on download.opensuse.org (I use Debian). The fix was already there, although slightly changed. After update I re-tested by case, everyting worked ok.

I think you would update your installation the same way instead of patching. Additionally, you will get other fixes. Code around my original patch has also changed, so applying it directly may create errors.

[DeepDiver1975]

issue has been solved meanwhile - #12030 (comment)