nextcloud / server

☁️ Nextcloud server, a safe home for all your data
https://nextcloud.com
GNU Affero General Public License v3.0
27.37k stars 4.07k forks source link

Critical: renaming files inside shared nested folder duplicates file #9957

Closed dtygel closed 4 years ago

dtygel commented 6 years ago

Precondition:

Users U, S1 and S2 must have desktop sync client installed (Ubuntu) and configured to share all remote files/folders.

Steps to reproduce

  1. User U creates a folder A
  2. User U creates a folder B inside A
  3. User U shares folder A with users S1 and S2
  4. User U shares folder B with users S1 and S2
  5. User U creates a file F in folder B (in desktop)
  6. Wait until all users' desktop sync clients finish sync those files and folders
  7. User U renames file F to F2
  8. Wait until all users' desktop sync clients finish syncing the files

Expected behaviour

All users should see a file named F2 in folder B.

Actual behaviour

All users (U, S1 and S2) see both files F and F2 in folder B.

Server configuration

Operating system: Ubuntu 18.04

Web server: Apache2

Database: Mysql

PHP version: 7

Nextcloud version: 13 (from branch stable13)

Updated from an older Nextcloud/ownCloud or fresh install: Fresh install

Where did you install Nextcloud from: Github

Signing status:

Signing status `integrity checker has been disabled. Integrity cannot be verified`

List of activated apps:

App list ``` Enabled: - comments: 1.3.0 - dav: 1.4.7 - federatedfilesharing: 1.3.1 - federation: 1.3.0 - files: 1.8.0 - files_sharing: 1.5.0 - files_trashbin: 1.3.0 - files_versions: 1.6.0 - lookup_server_connector: 1.1.0 - oauth2: 1.1.1 - provisioning_api: 1.3.0 - sharebymail: 1.3.0 - systemtags: 1.3.0 - theming: 1.4.5 - twofactor_backupcodes: 1.2.3 - updatenotification: 1.3.0 - workflowengine: 1.3.0 Disabled: - admin_audit - encryption - files_external - testing - user_ldap ```

Nextcloud configuration:

Config report ``` { "system": { "instanceid": "***REMOVED SENSITIVE VALUE***", "passwordsalt": "***REMOVED SENSITIVE VALUE***", "secret": "***REMOVED SENSITIVE VALUE***", "trusted_domains": [ "localhost" ], "datadirectory": "***REMOVED SENSITIVE VALUE***", "overwrite.cli.url": "http:\/\/localhost\/nc13_stable", "dbtype": "mysql", "version": "13.0.4.0", "dbname": "***REMOVED SENSITIVE VALUE***", "dbhost": "***REMOVED SENSITIVE VALUE***", "dbport": "", "dbtableprefix": "oc_", "mysql.utf8mb4": true, "dbuser": "***REMOVED SENSITIVE VALUE***", "dbpassword": "***REMOVED SENSITIVE VALUE***", "installed": true } } ```

Are you using external storage, if yes which one: No

Are you using encryption: No

Are you using an external user-backend, if yes which one: No

Client configuration

Browser: Firefox

Operating system: Ubuntu 18.

nextcloud-bot commented 6 years ago

GitMate.io thinks possibly related issues are https://github.com/nextcloud/server/issues/6491 (Duplicate Shared folders), https://github.com/nextcloud/server/issues/8628 (renaming shared folder - no actualization), https://github.com/nextcloud/server/issues/553 (Issues after renaming an already shared folder...), https://github.com/nextcloud/server/issues/9152 (Problem renaming folders), and https://github.com/nextcloud/server/issues/3502 (Duplicated files and folders when moving from one shared folder to another (can reproduce it)).

pawlosck commented 6 years ago

I think, it can be connected with my issue -> https://github.com/nextcloud/server/issues/8301

dtygel commented 6 years ago

We have a proposal to deal with this issue. If there is interest in the community, we can propose a Pull Request. Briefly, the proposal is as follows:

When preparing the list of files/folders to be shown to the user in root (/), we check if there is a direct share A that is also shared to the same user through a parent shared folder F.

We have a working implementation of this solution, but we know we should change the Share Class for correctly querying the database: we've temporarily created a query in the view lib/private/Files/View.php, but we know that it should be in lib/private/Share/Share.php. The query we've created gets a list of filecaches based on a list of fileids.

If you'd like to see it working, it's available in this commit: https://github.com/coletivoEITA/server/commit/7c619a33d450f927106a51db095ef1e20e4e145f

Is it of interest? If so, we can change the implementation to have the query inside Share.php

pawlosck commented 6 years ago

Sharing process in nextcloud is too complicated, unintuitive and dangerous for data. Why it can't be normal like in Windows? /nextcloud_server1/user/dir /nextcloud_server2/user/dir It's the most intuitive and understandable by most non technical people. It should be completely redesigned.

MorrisJobke commented 6 years ago

cc @nextcloud/sharing

jospoortvliet commented 6 years ago

@pawlosck the sharing model Nextcloud uses is the same as in Dropbox, Google Drive and OneDrive and they use that simpler model because it is easier to understand and deal with than having these expansive user directories that were invented in the '90's. Of course, people who are still used to those find the 'new' model weird and complicated, a classic example of how you can never make everybody happy I guess.

Meanwhile, I do like the idea of @dtygel as it allows users to 'upgrade' the share permissions without getting duplicate shares. @jancborchardt what do you think, is this the right approach?

jospoortvliet commented 6 years ago

@jancborchardt @nextcloud/sharing given @dtygel is willing to do a PR we should really give some thinking to this... Of course, I understand everybody is focusing on finishing and releasing Nextcloud 14 now.

@dtygel you coming to the Nextcloud conference?

viniciuscb commented 6 years ago

hi @jospoortvliet , I worked in pair with @dtygel in this patch. We are from a cooperative in Brazil (eita.coop.br) and one of our main products is a Nextcloud-based solution, that we would like to present in the conference.

We have interest in participating and would like to apply for the 80% support for travel and accommodations.

rullzer commented 6 years ago

Well a PR is welcome of course.

With code it is easier to discuss but my fear is mainly that this reduces scalability a lot. As you need to traverse up for all shares. And we have customers that have close to 1.5k shares for a single user.

But as said. Please submit a PR and we can have a look and discuss.

skjnldsv commented 4 years ago

As the version of the software you've reported this for has reached end of life, I will close this ticket. If this is still happening after an upgrade to the latest version, feel free to reopen

Freshou commented 7 months ago

Est-ce que le bug pourrait être dans le NextCloud Client Mac avec une erreur d'interprétation des Groups Folder ?!?!? J'ai refait des tests avec les log en mode debug sur le serveur... Et je n'ai absolument rien côté serveur...

Mais quand je demande l'archive de déboggage du NextCloud Client j'ai ceci au moment ou j'effectue le rennommage :

2024-03-18 15:05:35:840 [ info nextcloud.sync.discovery /var/folders/yr/9dx0mtfj7kdf4725tmcz6md80000gp/T/macos-21208/src/libsync/discovery.cpp:474 ]: Processing "0300 - LAS Comptabilité/test David--.pdf" | (db/local/remote) | valid: true/false/true | mtime: 1707760237/0/1707760237 | size: 84331/0/84331 | etag: "2edf741481fbeb2075128501b8d84055"//"2edf741481fbeb2075128501b8d84055" | checksum: "MD5:337154430db00d63039a72c3f86e2ec2"//"" | perm: "WDNVRM"//"WDNVRm" | fileid: "37574027oc98lfxeupo5"//"37574027oc98lfxeupo5" | type: CSyncEnums::ItemTypeFile/CSyncEnums::ItemTypeSkip/CSyncEnums::ItemTypeFile | e2ee: false/false | e2eeMangledName: ""/"" | file lock: not locked//not locked | file lock type: ""//"0" | metadata missing: /false/ 2024-03-18 15:05:35:840 [ info nextcloud.sync.discovery /var/folders/yr/9dx0mtfj7kdf4725tmcz6md80000gp/T/macos-21208/src/libsync/discovery.cpp:1682 ]: "0300 - LAS Comptabilité/test David--.pdf 2 1 0" 2024-03-18 15:05:35:840 [ info nextcloud.sync.discovery /var/folders/yr/9dx0mtfj7kdf4725tmcz6md80000gp/T/macos-21208/src/libsync/discovery.cpp:474 ]: Processing "0300 - LAS Comptabilité/test David--2.pdf" | (db/local/remote) | valid: false/true/false | mtime: 0/1707760237/0 | size: 0/84331/0 | etag: ""//"" | checksum: ""//"" | perm: ""//"" | fileid: ""//"" | type: CSyncEnums::ItemTypeSkip/CSyncEnums::ItemTypeFile/CSyncEnums::ItemTypeFile | e2ee: false/false | e2eeMangledName: ""/"" | file lock: not locked// | file lock type: ""//"" | metadata missing: /false/ 2024-03-18 15:05:35:840 [ warning nextcloud.sync.discovery /var/folders/yr/9dx0mtfj7kdf4725tmcz6md80000gp/T/macos-21208/src/libsync/discovery.cpp:494 ]: File "0300 - LAS Comptabilité/test David--2.pdf" was modified before the last sync run and is not in the sync journal and server 2024-03-18 15:05:35:841 [ info nextcloud.sync.discovery /var/folders/yr/9dx0mtfj7kdf4725tmcz6md80000gp/T/macos-21208/src/libsync/discovery.cpp:1388 ]: checking checksum of potential rename "0300 - LAS Comptabilité/test David--2.pdf" "MD5:337154430db00d63039a72c3f86e2ec2" "MD5:337154430db00d63039a72c3f86e2ec2" 2024-03-18 15:05:35:841 [ info nextcloud.sync.discovery /var/folders/yr/9dx0mtfj7kdf4725tmcz6md80000gp/T/macos-21208/src/libsync/discovery.cpp:1435 ]: Move without permission to rename base file, source: true , target: true , targetNew: true , isExternalStorage: true 2024-03-18 15:05:35:841 [ info nextcloud.sync.discovery /var/folders/yr/9dx0mtfj7kdf4725tmcz6md80000gp/T/macos-21208/src/libsync/discovery.cpp:1682 ]: "0300 - LAS Comptabilité/test David--2.pdf 8 1 0" 2024-03-18 15:05:35:841 [ info nextcloud.sync.discovery /var/folders/yr/9dx0mtfj7kdf4725tmcz6md80000gp/T/macos-21208/src/libsync/discovery.cpp:1458 ]: Undid remove instruction on source "0300 - LAS Comptabilité/test David--.pdf" 2024-03-18 15:05:35:841 [ info nextcloud.sync.discovery /var/folders/yr/9dx0mtfj7kdf4725tmcz6md80000gp/T/macos-21208/src/libsync/discovery.cpp:83 ]: STARTING "0300 - LAS Comptabilité/Transactions - Accueil" OCC::ProcessDirectoryJob::ParentNotChanged "0300 - LAS Comptabilité/Transactions - Accueil" OCC::ProcessDirectoryJob::NormalQuery 2024-03-18 15:05:35:842 [ info nextcloud.sync.discovery /var/folders/yr/9dx0mtfj7kdf4725tmcz6md80000gp/T/macos-21208/src/libsync/discovery.cpp:474 ]: Processing "0300 - LAS Comptabilité/Transactions - Accueil/Revenus clients" | (db/local/remote) | valid: true/db/db | mtime: 1710741364/0/0 | size: 0/0/0 | etag: "65f7d774047b6"//"" | checksum: ""//"" | perm: "DNVCKRm"//"" | fileid: "02506031oc98lfxeupo5"//"" | type: CSyncEnums::ItemTypeDirectory/CSyncEnums::ItemTypeSkip/CSyncEnums::ItemTypeFile | e2ee: false/false | e2eeMangledName: ""/"" | file lock: not locked// | file lock type: ""//"" | metadata missing: /false/ 2024-03-18 15:05:35:842 [ info nextcloud.sync.engine /var/folders/yr/9dx0mtfj7kdf4725tmcz6md80000gp/T/macos-21208/src/libsync/syncengine.cpp:783 ]: #### Discovery end #################################################### 478 ms 2024-03-18 15:05:35:842 [ info nextcloud.sync.engine /var/folders/yr/9dx0mtfj7kdf4725tmcz6md80000gp/T/macos-21208/src/libsync/syncengine.cpp:833 ]: #### Reconcile (aboutToPropagate) #################################################### 478 ms 2024-03-18 15:05:35:842 [ info nextcloud.sync.statustracker /var/folders/yr/9dx0mtfj7kdf4725tmcz6md80000gp/T/macos-21208/src/libsync/syncfilestatustracker.cpp:239 ]: Investigating "0300 - LAS Comptabilité" OCC::SyncFileItem::NoStatus CSyncEnums::CSYNC_INSTRUCTION_UPDATE_METADATA OCC::SyncFileItem::Down 2024-03-18 15:05:35:842 [ info nextcloud.sync.statustracker /var/folders/yr/9dx0mtfj7kdf4725tmcz6md80000gp/T/macos-21208/src/libsync/syncfilestatustracker.cpp:239 ]: Investigating "0300 - LAS Comptabilité/test David--.pdf" OCC::SyncFileItem::NoStatus CSyncEnums::CSYNC_INSTRUCTION_NONE OCC::SyncFileItem::Up 2024-03-18 15:05:35:842 [ info nextcloud.sync.statustracker /var/folders/yr/9dx0mtfj7kdf4725tmcz6md80000gp/T/macos-21208/src/libsync/syncfilestatustracker.cpp:239 ]: Investigating "0300 - LAS Comptabilité/test David--2.pdf" OCC::SyncFileItem::NoStatus CSyncEnums::CSYNC_INSTRUCTION_NEW OCC::SyncFileItem::Up 2024-03-18 15:05:35:842 [ info nextcloud.sync.engine /var/folders/yr/9dx0mtfj7kdf4725tmcz6md80000gp/T/macos-21208/src/libsync/syncengine.cpp:840 ]: #### Reconcile (aboutToPropagate OK) #################################################### 479 ms 2024-03-18 15:05:35:842 [ info nextcloud.sync.engine /var/folders/yr/9dx0mtfj7kdf4725tmcz6md80000gp/T/macos-21208/src/libsync/syncengine.cpp:894 ]: #### Post-Reconcile end #################################################### 479 ms

Freshou commented 7 months ago

Les lignes suspectes sont :

`2024-03-18 15:05:35:841 [ info nextcloud.sync.discovery /var/folders/yr/9dx0mtfj7kdf4725tmcz6md80000gp/T/macos-21208/src/libsync/discovery.cpp:1435 ]: Move without permission to rename base file, source: true , target: true , targetNew: true , isExternalStorage: true

2024-03-18 15:05:35:841 [ info nextcloud.sync.discovery /var/folders/yr/9dx0mtfj7kdf4725tmcz6md80000gp/T/macos-21208/src/libsync/discovery.cpp:1682 ]: "0300 - LAS Comptabilité/test David--2.pdf 8 1 0"

2024-03-18 15:05:35:841 [ info nextcloud.sync.discovery /var/folders/yr/9dx0mtfj7kdf4725tmcz6md80000gp/T/macos-21208/src/libsync/discovery.cpp:1458 ]: Undid remove instruction on source "0300 - LAS Comptabilité/test David--.pdf"`

Selon moi !