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

Error while posting comment in web view #27012

Closed
PVince81 opened this Issue Jan 24, 2017 · 8 comments

Comments

Projects
None yet
2 participants
@PVince81
Member

PVince81 commented Jan 24, 2017

Steps to reproduce

  1. user selects file in shared folder owned by xyz
  2. enters comment and presses post comment

Expected behaviour

comment should be posted

Actual behaviour

error occurs while posting comment

oC 9.1.3 EEe

Server configuration

Operating system:
RHEL 7.2
Web server:
apache 2.4
Database:
mariadb

Browsers:
Chrome / Firefox / IE Edge

log file content:

Exception: {
"Exception":"TypeError","Message":"Argument 1 passed to OC\\Files\\Config\\CachedMountInfo::__construct() must implement interface OCP\\IUser, null given, called in \/var\/www\/html\/owncloud-9.1.3\/lib\/private\/Files\/Config\/UserMountCache.php on line 173",
"Code":0,
"Trace":"
#0 \/var\/www\/html\/owncloud-9.1.3\/lib\/private\/Files\/Config\/UserMountCache.php(173): OC\\Files\\Config\\CachedMountInfo->__construct(NULL, 6, 1829, '\/user1\/files\/...')
#1 [internal function]: OC\\Files\\Config\\UserMountCache->dbRowToMountInfo(Array)
#2 \/var\/www\/html\/owncloud-9.1.3\/lib\/private\/Files\/Config\/UserMountCache.php(206): array_map(Array, Array)
#3 \/var\/www\/html\/owncloud-9.1.3\/lib\/private\/Files\/Config\/UserMountCache.php(260): OC\\Files\\Config\\UserMountCache->getMountsForStorageId(6)
#4 \/var\/www\/html\/owncloud-9.1.3\/apps\/comments\/lib\/Activity\/Listener.php(80): OC\\Files\\Config\\UserMountCache->getMountsForFileId('20675')
#5 \/var\/www\/html\/owncloud-9.1.3\/apps\/comments\/appinfo\/app.php(50): OCA\\Comments\\Activity\\Listener->commentEvent(Object(OCP\\Comments\\CommentsEvent))
#6 [internal function]: OC_App::{closure}(Object(OCP\\Comments\\CommentsEvent), 'OCP\\\\Comments\\\\IC...', Object(Symfony\\Component\\EventDispatcher\\EventDispatcher))
#7 \/var\/www\/html\/owncloud-9.1.3\/3rdparty\/symfony\/event-dispatcher\/EventDispatcher.php(181): call_user_func(Object(Closure), Object(OCP\\Comments\\CommentsEvent), 'OCP\\\\Comments\\\\IC...', Object(Symfony\\Component\\EventDispatcher\\EventDispatcher))
#8 \/var\/www\/html\/owncloud-9.1.3\/3rdparty\/symfony\/event-dispatcher\/EventDispatcher.php(46): Symfony\\Component\\EventDispatcher\\EventDispatcher->doDispatch(Array, 'OCP\\\\Comments\\\\IC...', Object(OCP\\Comments\\CommentsEvent))
#9 \/var\/www\/html\/owncloud-9.1.3\/lib\/private\/Comments\/Manager.php(530): Symfony\\Component\\EventDispatcher\\EventDispatcher->dispatch('OCP\\\\Comments\\\\IC...', Object(OCP\\Comments\\CommentsEvent))
#10 \/var\/www\/html\/owncloud-9.1.3\/lib\/private\/Comments\/Manager.php(484): OC\\Comments\\Manager->insert(Object(OC\\Comments\\Comment))
#11 \/var\/www\/html\/owncloud-9.1.3\/apps\/dav\/lib\/Comments\/CommentsPlugin.php(243): OC\\Comments\\Manager->save(Object(OC\\Comments\\Comment))
#12 \/var\/www\/html\/owncloud-9.1.3\/apps\/dav\/lib\/Comments\/CommentsPlugin.php(119): OCA\\DAV\\Comments\\CommentsPlugin->createComment('files', '20675', Array, 'application\/jso...')
#13 [internal function]: OCA\\DAV\\Comments\\CommentsPlugin->httpPost(Object(Sabre\\HTTP\\Request), Object(Sabre\\HTTP\\Response))
#14 \/var\/www\/html\/owncloud-9.1.3\/3rdparty\/sabre\/event\/lib\/EventEmitterTrait.php(105): call_user_func_array(Array, Array)
#15 \/var\/www\/html\/owncloud-9.1.3\/3rdparty\/sabre\/dav\/lib\/DAV\/Server.php(459): Sabre\\Event\\EventEmitter->emit('method:POST', Array)
#16 \/var\/www\/html\/owncloud-9.1.3\/3rdparty\/sabre\/dav\/lib\/DAV\/Server.php(248): Sabre\\DAV\\Server->invokeMethod(Object(Sabre\\HTTP\\Request), Object(Sabre\\HTTP\\Response))
#17 \/var\/www\/html\/owncloud-9.1.3\/apps\/dav\/lib\/Server.php(178): Sabre\\DAV\\Server->exec()
#18 \/var\/www\/html\/owncloud-9.1.3\/apps\/dav\/appinfo\/v2\/remote.php(29): OCA\\DAV\\Server->exec()
#19 \/var\/www\/html\/owncloud-9.1.3\/remote.php(164): require_once('\/var\/www\/html\/o...')
#20 {main}","File":"\/var\/www\/html\/owncloud-9.1.3\/lib\/private\/Files\/Config\/CachedMountInfo.php","Line":58}

The funny thing is, if the page gets reloaded, the comment appears successfully!

@butonic managed to mitigate the issue with a patch:

Index: lib/private/Files/Config/UserMountCache.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- lib/private/Files/Config/UserMountCache.php	(revision )
+++ lib/private/Files/Config/UserMountCache.php	(revision )
@@ -170,6 +170,10 @@
 
 	private function dbRowToMountInfo(array $row) {
 		$user = $this->userManager->get($row['user_id']);
+		if($user === null) {
+			$this->logger->warning("User '{$row['user_id']}' not found by UserManager, faking User object");
+			$user = new \OC\User\User($row['user_id'], null);
+		}
 		return new CachedMountInfo($user, (int)$row['storage_id'], (int)$row['root_id'], $row['mount_point']);
 	}
 

Tasks

  • reproduce the issue locally
  • write an integration test to reproduce this, if possible
  • make a PR with the patch + unit tests

@PVince81 PVince81 added this to the 9.1.5 milestone Jan 24, 2017

@PVince81 PVince81 self-assigned this Jan 24, 2017

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Jan 24, 2017

Member

From what I see in the stack, the problem happens in the activity app when the comment code is trying to post its activity. It is likely that either there is a deleted/ghost user who has access to that file and it is isn't excluded properly, or some other kind of ghostly... things.

Member

PVince81 commented Jan 24, 2017

From what I see in the stack, the problem happens in the activity app when the comment code is trying to post its activity. It is likely that either there is a deleted/ghost user who has access to that file and it is isn't excluded properly, or some other kind of ghostly... things.

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Jan 25, 2017

Member

Tried with a deleted user as recipient, no exception.

Looking at the stack trace it almost looks like the comment is being posted on a fileid linked to a user that doesn't exist or could not be resolved. How is that even possible ? Maybe someone deleted the share owner (maybe the owner is a zombie/remnant) and someone else kept the web UI open and tried posting a comment on the file still visible in the UI (a refresh would make it disappear).

Let's try that...

Member

PVince81 commented Jan 25, 2017

Tried with a deleted user as recipient, no exception.

Looking at the stack trace it almost looks like the comment is being posted on a fileid linked to a user that doesn't exist or could not be resolved. How is that even possible ? Maybe someone deleted the share owner (maybe the owner is a zombie/remnant) and someone else kept the web UI open and tried posting a comment on the file still visible in the UI (a refresh would make it disappear).

Let's try that...

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Jan 25, 2017

Member

Tested without memcache: deleting the sharer user from LDAP directly removes the share. So trying to post a comment yields another error which makes sense.

Member

PVince81 commented Jan 25, 2017

Tested without memcache: deleting the sharer user from LDAP directly removes the share. So trying to post a comment yields another error which makes sense.

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Jan 25, 2017

Member

Arghhh, while trying to reproduce this I found yet another issue with shared storages and deleted users: #27026

Member

PVince81 commented Jan 25, 2017

Arghhh, while trying to reproduce this I found yet another issue with shared storages and deleted users: #27026

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Jan 25, 2017

Member

I managed to simulate a similar issue by manually setting the field "user_id" to "unexist" in the "oc_mounts" table. Since this is only a cache, it could happen that it contains references to deleted users so this needs to be handled gracefully.

Not sure how to create the situation from scratch, but I think this information is enough to justify the fix from the patch.

Member

PVince81 commented Jan 25, 2017

I managed to simulate a similar issue by manually setting the field "user_id" to "unexist" in the "oc_mounts" table. Since this is only a cache, it could happen that it contains references to deleted users so this needs to be handled gracefully.

Not sure how to create the situation from scratch, but I think this information is enough to justify the fix from the patch.

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Jan 25, 2017

Member

Okay, I found some steps:

  1. Setup OC from scratch (no LDAP)
  2. Create a user "user1"
  3. Login as "user1"
  4. update oc_mounts set user_id='unexist' where user_id='user1';
  5. Refresh the page
  6. Post a comment on "welcome.txt"

"Error posting comment"

Log shows:

{"reqId":"I2EnV\/0Fa6X7VlS+\/fXA","remoteAddr":"127.0.0.1","app":"PHP","message":"Argument 1 passed to OC\\Files\\Config\\CachedMountInfo::__construct() must implement interface OCP\\IUser, null given, called in \/srv\/www\/htdocs\/owncloud\/lib\/private\/Files\/Config\/UserMountCache.php on line 173 and defined at \/srv\/www\/htdocs\/owncloud\/lib\/private\/Files\/Config\/CachedMountInfo.php#58","level":3,"time":"2017-01-25T11:45:34+00:00","method":"POST","url":"\/owncloud\/remote.php\/dav\/comments\/files\/20\/","user":"user1"}
{"reqId":"I2EnV\/0Fa6X7VlS+\/fXA","remoteAddr":"127.0.0.1","app":"PHP","message":"Call to a member function getUID() on null at \/srv\/www\/htdocs\/owncloud\/apps\/comments\/lib\/Activity\/Listener.php#87","level":3,"time":"2017-01-25T11:45:34+00:00","method":"POST","url":"\/owncloud\/remote.php\/dav\/comments\/files\/20\/","user":"user1"}
Member

PVince81 commented Jan 25, 2017

Okay, I found some steps:

  1. Setup OC from scratch (no LDAP)
  2. Create a user "user1"
  3. Login as "user1"
  4. update oc_mounts set user_id='unexist' where user_id='user1';
  5. Refresh the page
  6. Post a comment on "welcome.txt"

"Error posting comment"

Log shows:

{"reqId":"I2EnV\/0Fa6X7VlS+\/fXA","remoteAddr":"127.0.0.1","app":"PHP","message":"Argument 1 passed to OC\\Files\\Config\\CachedMountInfo::__construct() must implement interface OCP\\IUser, null given, called in \/srv\/www\/htdocs\/owncloud\/lib\/private\/Files\/Config\/UserMountCache.php on line 173 and defined at \/srv\/www\/htdocs\/owncloud\/lib\/private\/Files\/Config\/CachedMountInfo.php#58","level":3,"time":"2017-01-25T11:45:34+00:00","method":"POST","url":"\/owncloud\/remote.php\/dav\/comments\/files\/20\/","user":"user1"}
{"reqId":"I2EnV\/0Fa6X7VlS+\/fXA","remoteAddr":"127.0.0.1","app":"PHP","message":"Call to a member function getUID() on null at \/srv\/www\/htdocs\/owncloud\/apps\/comments\/lib\/Activity\/Listener.php#87","level":3,"time":"2017-01-25T11:45:34+00:00","method":"POST","url":"\/owncloud\/remote.php\/dav\/comments\/files\/20\/","user":"user1"}
@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Jan 25, 2017

Member

Note: it is still not clear to me how the bogus entry made it into the database. It might be possible to trigger this with shares from deleted/ghost users where the cache entry still remains.

Member

PVince81 commented Jan 25, 2017

Note: it is still not clear to me how the bogus entry made it into the database. It might be possible to trigger this with shares from deleted/ghost users where the cache entry still remains.

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Jan 25, 2017

Member

Fix is here #27028

Member

PVince81 commented Jan 25, 2017

Fix is here #27028

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment