Folders with custom icons not deletable on Windows

[meztup]

Syncing a folder with the read-only attribute set between Windows clients works as expected. However, when deleting the folder on one client, the other goes "Out of Sync" in the GUI with "access denied" in the Syncthing console.

The folder must then be manually deleted on the second machine (using explorer or "attrib" and "del" commands in the console.)

This seems similar to Issue #1610 only with folders rather than files.

Note: Windows sets the read-only attribute on folders automatically when you set a custom icon for said folder--that's how I initially discovered the issue. However, manually creating a read-only folder results in exactly the same behavior from Syncthing.

Syncthing Version: v0.14.11
OS Version: Windows 7

[canton7]

So, "Read-only folders are read-only"?

[calmh]

This surprises me as we have a test that verifies that we can remove read only directories in read only directories on Windows:

https://github.com/syncthing/syncthing/blob/master/lib/osutil/osutil_test.go#L99

and we actually appear to be using that code to remove directories from the puller:

https://github.com/syncthing/syncthing/blob/master/lib/model/rwfolder.go#L696

although not, for example, for removing stale tempfiles in it first so there is some inconsistency we should clean up...

Is this generally reproducible by Windows folks?

[meztup]

Maybe os.remove should be osutil.remove?

[AudriusButkevicius]

We should replace it everywhere, yes.

[meztup]

Oops.

osutil.remove was removed back in August (18cc7a6) because Go had apparently fixed os.remove

EDIT:
I just went back and tested Syncthing v0.14.4 from August 10th (release prior to the above changes) and it worked perfectly.

Also, it looks like the test might be relying on an error occurring -- perhaps os.remove itself is failing silently.

[calmh]

There's something else going on here beyond the read only bit. Read only directories can be deleted just fine. I tried to make a test for this, but if I create a read only folder, either from code or by creating it in Explorer and setting the read only bit:

it deletes just fine by our normal procedure. But setting a custom icon does something else. Note the "1 file" supposedly in the directory now:

That file isn't visible in cmd nor Explorer, but deleting the directory now indeed returns access denied. Maybe we manage to sync this custom icon anyway and this screws up the following delete.

Needs someone who likes getting involved in Windows internals, which is not me beyond this point. :)

[AudriusButkevicius]

I think there should be a folder.jpg that's not visible in there.

[calmh]

Yeah I guess the only odd thing is the "access denied" vs the more reasonable "not empty" then. But this issue seems much more niche than I initially thought, and I'm leaving it for someone who cares more to figure out. :)

[wweich]

Setting a custom icon will create a hidden desktop.ini file. You can see it with dir /a or by unchecking "hide system files" in the Windows Explorer settings.
Deleting that file will unlock the folder.

[Ferroin]

Expanding on what @wweich said: Properties like custom icons get put into a file called desktop.ini that has the 'system' attribute bit set, which means that it is hidden unless you have the 'Hide Protected Operating System Files' option disabled in your Folder Settings. The file itself is essentially equivalent to an OS X resource fork for a directory. It originated back when Windows still used FAT as the default filesystem (and thus didn't have alternative data streams to stuff the data in), and has stuck around because Microsoft are too lazy and obsessed with making ancient software continue working to update it to do things right.

Deletion of folders with this file in it works from the Windows file manager because it checks for this file and deletes it before deleting the folder if there is nothing else in the folder. Syncthing should probably do the same.

[meztup]

@calmh Setting the attribute using the properties dialog only makes the files within the folder read-only. The folder itself is still just a regular folder. Creating a custom icon changes the attibute of the actual folder. Try using attrib from the command-line to create a "real" read-only folder.

[wweich]

Deletion of folders with this file in it works from the Windows file manager because it checks for this file and deletes it before deleting the folder if there is nothing else in the folder.

Actually, I couldn't delete the folder with custom icon in Windows Explorer until I remove the desktop.ini file manually.

[AudriusButkevicius]

I tried os.RemoveAll, which deletes the desktop.ini, but the folder is still not deletable.
I think we should raise this as a bug in the standard library.

[Nnnes]

Since nobody else seems to have submitted this as a Go issue yet: golang/go#26295

[TigersWay]

Sorry to bump this old unsolved issue, but ...
Read-only folders simply do not exist on Windows. The attribute exists but has nothing to do with reading only, but stating the fact - on Windows only - that these folders are special/customized.

It seems just plain wrong then to sync that attribute or its changes (from Windows only again)!

[calmh]

We don't and that's not the problem here.

I also suspect the standard library fix (linked from the issue above) misses the point as it just does a chmod of the directory before remove, which as far as I remember does not solve this problem with the custom icon.

[Nnnes]

@calmh Now that Go 1.14 is out, I can confirm you are correct that the standard library fix does not solve the problem for Syncthing. But why not?

I have an otherwise empty test folder with a custom icon C:\sync\test_folder_with_icon. When synced to a Windows machine that is running Syncthing built with Go 1.14 (syncthing v1.4.0-rc.11+38-g20aaa592 "Fermium Flea" (go1.14 windows-amd64)) and then deleted on the original machine, Syncthing fails to delete the folder, just as before. However, I can successfully delete the very same folder (regardless of whether or not the desktop.ini has already been deleted) using a simple Go program built with Go 1.14:

package main

import (
	"os"
)

func main() {
	err := os.RemoveAll("C:/sync/test_folder_with_icon")
	if err != nil {
		panic(err)
	}
}

My Go knowledge is limited. What is the difference between what Syncthing does and what the short program above does? It seems to me that Syncthing uses os.RemoveAll() to delete directories.

[AudriusButkevicius]

We do Remove, not RemoveAll, as RemoveAll would delete ignored files, hence we rely on Remove to tell us that the directory is not empty and preventing us from deleting it.

[calmh]

syncthing/lib/model/folder_sendrecv.go

Line 1826 in 20aaa59

func (f *sendReceiveFolder) deleteDirOnDisk(dir string, snap *db.Snapshot, scanChan chan<- string) error {

[john8329]

Hopefully it's not an issue to add details to a months old discussion, but I'm having a similar problem. I'm using Google Drive to sync my folder to a windows machine, and Syncthing to "bridge" the syncing to other workstations. Google drive puts custom icons on every folder synchronised, and I'm having frequent issues described by "Access is denied."

The files and folders have "everyone" set to full control, so it's not a permission thing. Oddly if I delete the folders and restore them, they stop being a problem, but that has to be manual. I'm still researching this, but it appears to happen systematically with this setup:

• Windows machine with syncthing and google drive, syncthing reads and writes to the google drive folder (ignoring and deleting .DS_Store and desktop.ini). Permissions are ignored for sync
• macOS machine with just syncthing, adding a .mindnode bundle (which is a folder with other folders) makes it sync correctly (it's added to a google drive SHARED folder, so it adds the icons at windows side)
• when removing the bundle, it fails to delete it at the windows side, giving "Access is denied." errors on every folder contained by it, files are removed
• If I delete the folders manually everything's okay

Hope it could help reproducing what I think is a quite nasty bug. It doesn't seem to happen outside Drive, but I presume it's linked to the custom icons, when Drive isn't syncing the issue doesn't arise.

I can confirm the problem is 100% reproducible on demand.

[djjaeger82]

Hopefully it's not an issue to add details to a months old discussion, but I'm having a similar problem. I'm using Google Drive to sync my folder to a windows machine, and Syncthing to "bridge" the syncing to other workstations. Google drive puts custom icons on every folder synchronised, and I'm having frequent issues described by "Access is denied."

I'm having this exact same issue. My mother has a desktop and a laptop she uses in her house. I use syncthings to sync files between the two machines locally. And then on the desktop I also backup the same folders to google drive. I am constantly getting this access denied error, I believe due to the google drive and custom icons it imposes. If I uninstall google drive then all of a sudden syncthings can do its job without issue.

Has there been any fix to this problem? Any workaround? Please help. I want to have local sync + cloud backup both.

[calmh]

I haven't tried but maybe an ignore pattern like (?d)desktop.ini would work (ignore desktop.ini and remove it if it's in the way of directory deletion).

[AlexandreAlvesDB]

As stated in Go repository : golang/go#26295

RemoveAll should be used instead of Remove for windows directories because Go returns an error in one case and not in the other when removing the same directory because of this issue with "custom" folders

Also preventing with an ignore desktop.ini does not work.

Example :

syncthing/lib/model/folder_sendrecv.go

Line 2039 in 452daad

if err := f.mtimefs.Remove(dirsToDelete[len(dirsToDelete)-1-i]); err != nil && delErr == nil {

Changing it to RemoveAll should fix this long-standing issue for send and receive folders

Most of the tests I saw in the syncthing repo are done with RemoveAll instead of Remove but most of the code used in production uses Remove

[calmh]

We should look at what RemoveAll does under the hood to handle this and adopt the relevant parts. (We want the remove to fail if there are unexpected items in the directory, as a safety measure, so we can't just start using RemoveAll everywhere.)

[AlexandreAlvesDB]

I opened a pull request on Go repository to patch Remove : golang/go#56615
So I guess you will not need to modify the code to fix the issue when the patch will get in a public release

[calmh]

The "read only" part is a red herring, your chmod will do nothing to enable the removal of that directory. Removing desktop.ini should resolve it, but if I remember correctly the next level of problem is that the desktop.ini file isn't visible to us or isn't removable of a combination of the two. (If it were visible, the suggested (?d)desktop.ini pattern would work, because that means "delete that file before deleting the directory".)

[AlexandreAlvesDB]

If it really is a red herring as you think, you have to use RemoveAll as my first proposition to remove the desktop.ini and the folder
If the bug was still open, it was for a reason, you have probably not read the thread but even without desktop.ini, os.Remove in Golang doesn't work for folder who had an icon, that's why I proposed the patch

[AlexandreAlvesDB]

Hello, I've seen my code added to the repo but I've not been added to the Authors file

[calmh]

@AlexandreAlvesDB sorry about that, the script I was hoping to pick that up wasn't smart enough, I've fixed it in the PR above.

[AlexandreAlvesDB]

@AlexandreAlvesDB sorry about that, the script I was hoping to pick that up wasn't smart enough, I've fixed it in the PR above.

No problem :)