Nextcloud Windows client does upload & delete instead of moves and renames

[joholm2]

How to use GitHub

• Please use the 👍 reaction to show that you are affected by the same issue.
• Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
• Subscribe to receive notifications on status change and new comments.

Expected behaviour

1. Renaming a file within a sync folder should result in renaming the file on the server.
2. Moving a file within a sync folder should result in moving the file on the server.
3. Renaming a folder within a sync folder should result in renaming the folder on the server with no action on the contents of the folder.

Actual behaviour

Windows client recreates the file or folder arrangement on the server by uploading all the folders and files then deletes the originals on the server.

Steps to reproduce

1. In Windows file explorer navigate to a sync folder.
   2.1 Select a file and rename it; or...
   2.2 Drag and drop the file to another folder in the same sync region; or...
   2.3 Rename a folder containing folders and/or files
2. Check the Activity log

Client configuration

Client version: Version 3.2.1 (Windows)

Operating system: Windows 10 Pro

Installation path of client: Default

Server configuration

Nextcloud version: 21.0.1, Ubuntu 20.04.2

Storage backend (external storage): SMB, Raid, Western Digital

Logs

1. Client logfile:
   See log files attached in next post...

2. Server logfile: nextcloud log (data/nextcloud.log):
   No message are added to the server log during the tests.

[joholm2]

I can see the cause of the issue now in the log.

I carried out a simple test....

1. Created a file "Test_abc.txt"
2. Renamed it to "Test_abc_renamed"
   This worked fine with a "move" on server
   log file 20210506_0134_owncloud.log.0.txt
3. Renamed the file back to "Test_abc.txt"
   This failed and uploaded Test_abc.txt and deleted Test_abc_renamed.txt
   log file 20210506_0134_owncloud.log.1.txt

20210506_0134_owncloud.log.0.txt
20210506_0134_owncloud.log.1.txt

Here is a screenshot of the web view of "Activity"; at the bottom step 1, then moving upwards, step 2 and the top two steps the failed rename back, which results in an upload and delete.

The error is in the webdav access to the server when searching for the file to be renamed in step 3 above.

When doing the first "move" we can see in the log the reference to the file to move on the server is correct.
Note that it includes the Root of the Sync folders....
.
Line 1: Line 2872: 2021-05-06 01:34:48:589 [ info nextcloud.sync.accessmanager ]: 6 "MOVE" "https://[myhost]/nextcloud/remote.php/dav/files/[user]/DJB@NC/Data/TestFolder/TestSub1/Test_abc.txt" has X-Request-ID "85ace896-5f31-4efe-b42a-eb00ef9a5661"

But later when looking for the "Test_abc_renamed.txt" file on the sever, the Root of the Sync folders is missing from the path...

Line 21: Line 2820: 2021-05-06 01:35:10:615 [ info nextcloud.sync.accessmanager ]: 6 "PROPFIND" "https://[myhost]/nextcloud/remote.php/dav/files/[user]/Data/TestFolder/TestSub1/Test_abc_renamed.txt" has X-Request-ID "0d410f2f-7917-44a7-8455-52b7df8c43d5"

Since the path is incorrect the server reports "not found" and the client decides it can't do a rename/move.; instead it does an upload and delete.

I don't know why it would create the incorrect path, but hopefully it should be easy to fix.
Maybe you can figure out what introduced this bug because it's been working fine up until very recently.

[allexzander]

@joholm2

Are you sure nothing is missing from repro steps?

Here are my results when following your repro-steps:

Line 150: 2021-05-06 14:43:57:023 [ info sync.discovery ]:	Rename detected (up)  "New folder (2)/New Text Document.txt"  ->  "New folder (2)/test_file.txt"
Line 156: 2021-05-06 14:44:08:719 [ info sync.discovery ]:	Rename detected (up)  "New folder (2)/test_file.txt"  ->  "New folder (2)/test_file_renamed.txt"
Line 155: 2021-05-06 14:44:15:017 [ info sync.discovery ]:	Rename detected (up)  "New folder (2)/test_file_renamed.txt"  ->  "New folder (2)/test_file.txt"

3 renames were detected successfully.

In your logs, however, I can see the following line:

2021-05-06 01:35:10:719 [ info sync.discovery ]:	Can't rename because the etag has changed or the directory is gone "Data/TestFolder/TestSub1/Test_abc_renamed.txt"
2021-05-06 01:35:10:719 [ info sync.discovery ]:	Discovered "Data/TestFolder/TestSub1/Test_abc.txt" CSyncEnums::CSYNC_INSTRUCTION_NEW OCC::SyncFileItem::Up CSyncEnums::ItemTypeFile

which makes me think that something else is definitely happening in parallel with renaming.

BTW, have you tried the same on version 3.1.3? Is it also present there?

[joholm2]

Hi Allexzander.

It is clear in the logs that the client is creating faulty "paths" with the Root folder missing in some of the webdav calls to the server.

Please can you follow up the cause of that.

(Please remove the "needs info" tag. There is nothing missing in the repro steps.)

PS. I didn't see this trouble previously using 3.1.3.
(I tried to revert to 3.1.3 but it crashed every time it tried to sync so I ended up reinstalling 3.2.1)

[joholm2]

Hi Allexzander. a question to you...

Which versions of client and sever did you use when trying to reproduce the issue?

You should use Windows client 3.2.1 and Linux server 21.0.1

[allexzander]

Hi Allexzander.

It is clear in the logs that the client is creating faulty "paths" with the Root folder missing in some of the webdav calls to the server.

Please can you follow up the cause of that.

(Please remove the "needs info" tag. There is nothing missing in the repro steps.)

PS. I didn't see this trouble previously using 3.1.3.
(I tried to revert to 3.1.3 but it crashed every time it tried to sync so I ended up reinstalling 3.2.1)

@joholm2 It crashed because there is an incompatibility between the new and old version's database. However, you can still test this scenario with 3.1.3 if you temporarily rename C:/Users/<your_user_name>/AppData/Roaming/Nextcloud folder to C:/Users/<your_user_name>/AppData/Roaming/~Nextcloud (or whatever, just so it is not named Nextcloud). Then, you can run 3.1.3 and check if the issue is there too.

The test with 3.1.3 is now even more important since you've mentioned your server version is 21.x and is installed on Linux, while my server is 20.x on Windows.

I just want to exclude the chance of this being a server-side issue. Thx.

I am going to keep the needs info label for now, as I still require these test cases from you because I was not able to reproduce the issue locally (will try later with a newer version of the server under Linux instead of Windows).

Hard to say, why the path created is incorrect, but, thx for the hint, I may need to look into it as well.

[joholm2]

Hard to say, why the path created is incorrect, but, thx for the hint, I may need to look into it as well.

From what I can see it is the the faulty path created by the client, which results in a "Not found" from the server (not the sever's fault) - which then leads to the client deciding that it can not do a rename or move and so resorts instead to a new upload.

So (forgive me for being "pushy" ) but I'd suggest that investigating the faulty path IS the most important thing here.

Ok. I'l try 3.1.3 soon and let you know.

[joholm2]

@allexzander

I uninstalled Windows client 3.2.1, removed the Nextcloud folder you mentioned and reinstalled 3.1.3.

I see that without its database it has to start from scratch and it was going to takes hours to re-sync. So I created a simple sync folder structure and file for testing, with a few folders, sub-folders and files.

I tried a variety of update, renames, moves of both folders and files - but I couldn't "break" the client - according to the "Activity" log it always did the correct thing.

To make a direct comparison with 3.2.1 I reduced it to a simple test with just the one file....

/Test@NC/Test_folder_01/Test_sub_01/Test_abc.txt

The test steps were to rename the file to "Test_abc_renamed.txt" and after sync has completed rename it back to "Test_abc.txt".

Client 3.1.3 carried out renames as it should.

Then I uninstalled 3.1.3, removed the Nextcloud folder, installed 3.2.1 and set it to sync the exact same folders Test@NC

The test steps were the same again; to rename the file to "Test_abc_renamed.txt" and after sync has completed rename it back to "Test_abc.txt".

Client 3.2.1 did a rename first and then an upload and delete.

Is I had seen before that the error was occurring in "PROPFIND" calls to the sever I filtered the logs for those events.

In 3.1.3 you can see that for the two step rename, in each case it works down the folder hierarchy and stops at the folder containing the renamed file....

In 3.2.1, it not only checks the folder hierarchy but even does a little more. For the first rename (which works fine) it checks a path to, but not including the sync folder and in the second rename (which goes wrong) it tries to check the path to the actual file which has been renamed - but the path IS MISSING THE ROOT FOLDER. This results in a "Not found" response from the server and the client abandons the rename attempt. Instead it resorts to upload and delete.

I'll add the log files if you need them; but I think it would be good if someone could check on why PROPFIND is getting the wrong path when trying to check a file.

[joholm2]

@allexzander

Here is another hint...

Owncloud had the same issue and found the bug back in February.

It was in a branch of code where it was deemed necessary to check the renamed file's "etag", but the root folder was missing from the URL they created.

Could it be that Nextcloud 3.2.1 was updated to include the same (erroneous) check?

Are you able to find the piece of code show in the screenshot below?

[joholm2]

@allexzander

Hi again,

I took a look myself and found the code here https://github.com/nextcloud/desktop/blob/master/src/libsync/discovery.cpp

Sure enough it has the same bug.

_remoteFolder is missing.

Are you able to confirm and make a correction?

[allexzander]

Hello @joholm2

At the moment I am overloaded with critical issues. Sorry, but, I am not able to work on multiple things at the same time. I will have a look at it once I am freer. Alternatively, I'll see if some of my teammates could make this correction.

As the last option, we encourage the community to contribute to bug fixes/improvements. So, if you feel like you up to it, go ahead and create a PR and I'll make sure it is reviewed and merged sooner.

[joholm2]

Hi, if the "tags" mean anything then could someone please remove the "Needs info" tag on this thread?
What more info could you want? I have shown you logs and even pointed to the likely mistake in the code.
I don't know "the ropes" here, but if someone can make the change I suggested above and create a trial build I'll be happy to test it.

[allexzander]

Probably related to #3296

[joholm2]

Thanks a lot @allexzander !

[allexzander]

Likewise. Thank you @joholm2. I've realized that the issue can be reproduced on my side when selecting a non-root remote sync folder as a local root. This is the important detail that was missing. Now, the PR is ready for review, and, we'll most likely put it into 3.2.2.

[joholm2]

Hi, I wasn't aware that there were situations where the remote folder could be root. Since we have several remote folders with different access permissions they are all starting on a path beginning with the synch folder name. Is there another kind of setup?
But most interesting - will there be a test build we can try out before official release?

[allexzander]

@joholm2
On my test server, I don't have many files, so, I set a remote root folder to a local sync folder root by default. Makes sense to choose custom roots if you have different access rights, etc.

The test build can be downloaded from here https://cloud.nextcloud.com/s/g6acjp9HGNPweL5

[joholm2]

Hi,

Thanks. I installed the version on the link above (shows up as Version 3.2.50 in settings).

I tested simple renaming and moving of files and folders and all appears to be working fine now. No unexpected uploads and deletes.

I tested renaming of folders containing several files, and still ok; with the expected renaming of folder on server and no unwanted uploads.

Checked logs and all webdav URLs referencing files and folders on the sever now have the correct path, including the sync folder name on the server.

Thanks for the great support!

[allexzander]

@joholm2 Great, thank you for helping us to improve the sync client. Feel free to contribute more if needed.

[biva]

@joholm2 are you sure you don't have the issue anymore? On my side, I still have the issue, even on 3.2.4 (see #3240 (comment) to reproduce the issue)

Thanks!

[joholm2]

HI @biva,
Yes the fault I found and reported in this post has been solved. It was a missing remote folder prefix in the path used to check the remote copy of the file. With the latest builds I see the correct path being used.

Unfortunately having solved one issue it has simply revealed another slightly less common issue.

I have a small bunch of folders and files I used for testing this issue and now I can rename and move them around and Nextcloud client does the correct update on the server pretty flawlessly.

But if I then resume my normal work, I have some files created recently by a document scanner, and invariably when I rename them Nextcloud client does an erroneous upload and delete. But this time it is a different bug.

I'm not sure why, but Nextcloud client decides to compare checksums of the local and remote "rename candidates", and whilst the checksum digits are identical, one of them has an extra piece of text at the end... "". This leads to the comparison failing and Nextcloud decides it is not a move, but rather a new file and a deleted file - so does an upload and delete.

2021-07-13 22:42:30:091 [ info sync.discovery ]: checking checksum of potential rename "[File path]" "SHA1:fe347769ef00d4ebfe066398e18ce54af6595deb" "SHA1:fe347769ef00d4ebfe066398e18ce54af6595deb</checksum>"
2021-07-13 22:42:30:091 [ info sync.discovery ]: Not a move, checksums differ

I have reported this bug in my more recent post...
"Inconsistent trailing text in checksums causes move and renames to be rejected" #3381

I took a look at your issue and it seems you are "stressing" the sync process a bit more than I am. Mine is done at a leisurely pace, allowing the Client to keep up easily. I think it's important to get the "easy" situations working properly first otherwise there is no sound footing for testing stress situations.

[biva]

Thanks for your feedback @joholm2
Indeed, I'm "stressing" the sync process, but that's only to reproduce a bug that we see randomly: we're in Africa, with a poor internet connection. I think that, in this case, "stressing" the sync process is equivalent to a "normal" behavior with a poor internet connection. Does it make sense?