Core: Lazy shared storage heII: ghost mounts

Created on 1 Sep 2016  路  6Comments  路  Source: owncloud/core

Found through https://github.com/owncloud/core/pull/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.

Bug sharing sev1-critical

Most helpful comment

<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

All 6 comments

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> 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

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.

Dirtiest fix, hopefully I'll find a better way: https://github.com/owncloud/core/pull/26004

Alternative better approach: https://github.com/owncloud/core/pull/26016
Makes DefaultShareProvider know about filecache and storages table, but I guess that's ok...

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

Was this page helpful?
0 / 5 - 0 ratings