Lazy shared storage heII: ghost mounts #26001

Closed
PVince81 opened this Issue Sep 1, 2016 · 5 comments

Projects

None yet

1 participant

@PVince81
Collaborator
PVince81 commented Sep 1, 2016

Found through #25977

Steps

  1. Create two users "user0" and "user1"
  2. Login as "user0"
  3. Create folders "common/sub"
  4. Share "sub" with "user1"
  5. Delete "common"
  6. Empty trashbin
  7. Login as "user1"
  8. PROPFIND on "sub"
  9. Create a folder "sub"

Expected

404 not found for PROPFIND, folder "sub" can be created.

Actual

<?xml version="1.0" encoding="utf-8"?>
<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">
  <s:exception>InvalidArgumentException</s:exception>
  <s:message>Jail rootPath is null</s:message>
</d:error>

Cannot create folder "sub" as "user1".

Versions

9.1.1RC1 / stable9.1

The problem here is that due to lazy initialization, the mount system thinks that the mount is there until it gets initialized later. Then it turns out it's not supposed to be there but is still registered. So creating a folder with the same name fails.
And querying its existence also causes issues.

@PVince81 PVince81 added this to the 9.1.1 milestone Sep 1, 2016
@PVince81
Collaborator
PVince81 commented Sep 1, 2016

With a local change I'm able to make the PROPFIND return 404.
However the mount manager still thinks that the mount is there and will forbid creating a folder "sub".

Ideally the mount manager should have a way to query mount validity.
And even if it does, should the validity check trigger the lazy init ? Probably not as it would defy the purpose of laziness...

Now a quick but awful hack would be to check all file operations that would target a ghost mount, and add an additional check there.

@PVince81
Collaborator
PVince81 commented Sep 1, 2016
<PVince81> Lazy shared storage hell II: ghost mounts
<PVince81> https://github.com/owncloud/core/issues/26001
<PVince81> the series of horror continues
<PVince81> let me tell you the story, just for venting a bit
<PVince81> once upon a time there was some awful sharing code
<PVince81> which was a million years old and tangled
<PVince81> the sharing code eventually got cleant up and was now using the new APIs, the node API
<PVince81> using the node API (happy?), however, would slow down because it would initialize the shared storage too often
<PVince81> even when only asking for a quick share info, it would setup the shared storages, which made the share API slow from the outside
<PVince81> so this is when the shared storages and mount system was adjusted to be lazy
<PVince81> this means that a storage exists and is mounted until the storage itself says it's actually not valid
<PVince81> but the mount system doesn't handle this properly
<PVince81> and changing the mount system at this stage, in 9.1.1... bääh
<PVince81> removing the laziness ? this would make the share API slow again
<PVince81> will the developers be able to untangle this horror ?
<PVince81> stay tuned
@PVince81
Collaborator
PVince81 commented Sep 1, 2016

Idea: let's see if I can pre-filter the shared mounts in the mount provider.
And instead of using the expensive way from the view to check the owner's storage, do a simple DB query that checks if the target of "file_source" exists or is inside "files/" for the target storage.

@PVince81 PVince81 referenced this issue Sep 1, 2016
Closed

Fix ghost mounts for obsolete shares #26004

8 of 15 tasks complete
@PVince81
Collaborator
PVince81 commented Sep 1, 2016

Dirtiest fix, hopefully I'll find a better way: #26004

@PVince81
Collaborator
PVince81 commented Sep 2, 2016

Alternative better approach: #26016
Makes DefaultShareProvider know about filecache and storages table, but I guess that's ok...

@PVince81 PVince81 closed this in #26016 Sep 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment