Preserve symbolic links from sync clients

[taminob]

• Resolves: Syncing symbolic links as reference desktop#250
• Related to: Feature/sync unix symlinks as links desktop#6205

Summary

This PR aims to allow clients to upload/download symlinks to/from the server.
I try to provide an MVP to make some progress with the 5 years old issue regarding symlink synchronization.

TODO

• Allow upload of symlinks via single-file upload (PUT request)
  • Extract from ChunkingV2Plugin to own plugin
  • Evaluate if symlinks should be stored as virtual files instead of actual symlinks on server
• Allow upload of symlinks via bulk upload (POST request)
• Allow download of symlinks
• Add symlink file type to server response for querying files
• Investigate necessary UI changes to either indicate or ignore symlinks

Open discussions

• Database columns in oc_symlinks to identify file: fileid vs storage+path
• Use new header field OC-File-Type in requests to indicate symlink or encode in Content-Type via unique symlink mimetype

Checklist

• Code is properly formatted
• Sign-off message is added to all commits
• Tests (unit, integration, api and/or acceptance) are included
• Screenshots before/after for front-end changes
• Documentation (manuals or wiki) has been updated or is not required
• Backports requested where applicable (ex: critical bugfixes)

Disclaimer

This PR is still in a WIP state, do not test this on a production Nextcloud server.
Also, everything in there is still subject to change and hacky solutions and code full with TODOs will appear in this PR.

[taminob]

@juliushaertl @ArtificialOwl @icewind1991 @sorbaugh

I think I got the first draft for this feature and would appreciate your thoughts on the implementation. Please bear with me if its not the most idomatic code, this is my first time working with PHP - any hints on how to improve it are welcome.

After experimenting with different server representations of symlinks, I finally settled on creating a new database table (oc_symlinks) and storing the actual symlink as a regular file (containing the symlink target). Especially the latter was helpful in not having to mess too much with the internal structure of Nextcloud and the 3rd party library sabre-dav. The checks if a path is a symlink are performed at the boundaries in the HTTP handlers.
Currently, the symlinks are identified via a combination of internalPath, name and numericStorageId. name is contained in the internalPath, so it could probably also be dropped, but I wanted to keep it consistent with the oc_filecache for now (although a path deduplication with parentPath+name for the parentPath could probably reduce the database size by quite a bit). I considered simply using the fileId of the file, but I'm not sure if this information is permanent since it's in something called "cache" which is re-written with each occ files:scan --all (and this could also make unmarking the source of a move operation harder).

Current limitations:

• symlinks restored from the trash bin will be re-created as regular files with the target path as their content; an additional database column (inTrash) might be a solution
• symlinks are not indicated in the Web UI; their preview/icon is determined by their name - this could actually prove trickier than thought because \OC\Files\Type\Detection::detectPath() is used throughout the code base with only the filename (which is insufficient to identify an entry in oc_symlinks) easier than thought via PreviewController - current solution might need improvements; I chose the link.svg for now, but there are definitely better icons - suggestions are welcome, I'm not the best UI designer :)
• copied symlinks are copied as regular files and not marked as symlinks in oc_symlinks (and thus interpreted as regular files)

If you don't see any major issues with this approach (please also point out potential other limitations that I've overlooked), I'll add according unit tests for the new code and rewrite the git history to ~6 commits (or would you prefer smaller or even fewer commits?).

Also feel free to ping me if something is not as intuitive as I thought or you have any other questions.

[taminob]

@juliushaertl @ArtificialOwl @icewind1991 @sorbaugh @szaimen ping

[taminob]

Hi @artonge, thanks for your reply - great to know at least someone is alive in this project.
Your summary of the implementation looks correct. Do you see a general issue with this approach or is this the right way to go?

1. Is the '{DAV:}symlink' property a proper WebDAV property? Can you link to its documentation?

I don't think it is - I wasn't able to find anything for symlinks.
The closest thing could be {DAV:}reftarget from https://greenbytes.de/tech/webdav/rfc4437.html, but not sure if it is a good idea to use it without following this RFC.
What do you think should be used instead? Is there some convention for custom properties or should they be avoided in general?

2. Same thing for the [`{DAV:}resourcetype` property](https://github.com/nextcloud/server/pull/41321/files#diff-1d6ed4804160da213bc58248990a6936622ce53100a23535d207dba153f7e759R77)

It is already in use in the code base, e.g. https://github.com/nextcloud/3rdparty/blob/d7b9f6f5f0513adc3ed652eb84b1822fb5b53032/sabre/dav/lib/DAV/CorePlugin.php#L548 (see also https://github.com/dmfs/davwiki/wiki/DAV::resourcetype)

3. Are we using the [`oc-file-type` header](https://github.com/nextcloud/server/pull/41321/files#diff-2ae5e0e3cbbc9591fb1530ddeec96098b0130c3fcafb1fe75c07aca882d7672eR103) somewhere else ?

No, that is a new header field - see the second point in "Open Discussions" in the PR description. But I think for the mime type we'll also have to define a custom string since I'm not aware of any "official" symlink mime type.

Also thanks for pointing out the metadata feature, I wasn't aware of that. That should simplify the code and looks pretty useful.
Unfortunately, I currently don't have the time to implement this improvement. I might have some spare time to experiment with it, but I might have to push the actual finalization of this PR back to August when I should have some more time again. But let's discuss already on how to implement the feature so that the implementation can go as quickly as possible then.

[artonge]

Hey @taminob, after discussing the PR internally, we think that this feature could cause too many issues, especially when dealing with compatibility between Windows, MacOS and Linux. Server side, this could be developed as an app to not have it in core, but client side there is no app system, and I fear that they probably won't accept to cover such use case.

[f1d094]

Hey @taminob, after discussing the PR internally, we think that this feature could cause too many issues, especially when dealing with compatibility between Windows, MacOS and Linux. Server side, this could be developed as an app to not have it in core, but client side there is no app system, and I fear that they probably won't accept to cover such use case.

@artonge First...I should point out that the lack of proper symlink synchronization is already causing a lot of issues and should be considered a bug, not a feature. As currently implemented Nextcloud actively breaks multi-client Linux/Mac Nextcloud setups. Secondly...I'd like to stress that if implemented correctly, it should be invisible to Windows users and transparently supported on Linux. Allow me to make the case:

1. On the Nextcloud server, there should exist a special filetype, that filetype is "POSIX-SymLink" or whatever.
2. The special filetype is ignored by all Windows clients. Full stop.
3. On Mac/Linux systems, it is sent client-side as a symlink. It doesn't matter where it points to, it could be a broken link, or it could point to /etc/shadow...that is not the concern of the client. A link is a link is a link. I can point my links to the moon and if there is no path there, what is the harm? I've just created a broken symlink. The client should never-ever resolve the link or care where it points. It is just a basic filetype.
4. Deleting the file on the nextcloud server, deletes the symlinks on clients that sync the symlink, as it would any other ordinary file.

Why is this at all complicated? Can you provide a use-case where the above would break any given install or cause comlications? Please explain.

We, as a team of Mac and Linux users, are incredibly frustrated with Nextcloud because of this very broken aspect of the way Nextcloud handles POSIX filesystems. If you claim to support Linux as a client, symlinks must be properly supported. They are a critcial and integral part of computing-on-Linux.

[f1d094]

1. On the Nextcloud server, there should exist a special filetype, that filetype is "POSIX-SymLink" or whatever.

To further clarify...on the Nextcloud server and through the web interface, the symlink filetype would:

1. Just be a text file
2. Not be editable on the web server. Preview of the file only allows viewing the text contents of the file...which would be a text representation of the path where the symlink points.
3. The only file handling options on the web server should be to move/delete the file

YES...moving the file may have the effect of breaking the symlink when it is synchronized to the clients if they are relative links. But such is the life of the symlink user...that is our problem to deal with. Whether or not a symlink points to anything useful is of no concern of the file-sync software...it should sync my broken links if I tell it to. Allow us to shoot our own foot off if we so desire. Please and Thank You.

[f1d094]

@artonge The above is the correct way symlink handling should be implemented. Based on earlier conversations with @taminob, this is also more-or-less how he implemented it. If this understanding is incorrect, then the conversation should instead be focused on how to fix the implementation and not to simply refuse support for a core system filetype.

[AkechiShiro]

@artonge this could be a Linux feature only or under a checkbox/specific manual configuration too and not in an app on the side.

@f1d094 @taminob I think I might fork Nextcloud just for this feature, I'll try to build a CI/CD to synchronize commits if there is just simply no way forward with this PR as this feature is highly necessary for me, because I believe this is just plain wrong to not support this feature. Feel free to let me know, if there would be any support towards this effort if we do need to come down this road sadly.

[f1d094]

@f1d094 @taminob I think I might fork Nextcloud just for this feature, I'll try to build a CI/CD to synchronize commits if there is just simply no way forward with this PR as this feature is highly necessary for me, because I believe this is just plain wrong to not support this feature. Feel free to let me know, if there would be any support towards this effort if we do need to come down this road sadly.

@AkechiShiro: I agree. Unfortunately, doing a fork is a very heavy lift and especially frustrating because this is something that should have been built-in from day one. Symlinks are very simple, both in concept and in execution. I am embarrassed on behalf of the Nextcloud team that nobody seems to have a grasp of filesystem basics. It is supposed to be at the core of what they do.

That said, if we were to do a fork, I would only be interested in supporting whatever the minimum use-case is to make symbolic links sync properly on Linux/Mac. I've never done something similar, but I imagine with a CI/CD setup we could simply maintain a running patch set for the server and the client binaries.

IIRC, @taminob is out of bandwidth to do further development until August...do you have the capacity to pull his fork and test what he's already commited?

I'm already working two jobs...I take time to come here and advocate because this feature is very important to me. Unfortunately I don't have any more room for another project and was counting on the Nextcloud devs to come to their senses.

[f1d094]

@artonge this could be a Linux feature only or under a checkbox/specific manual configuration too and not in an app on the side.

@AkechiShiro I would disgree that this should be an optional "Linux feature only". It should just be native...and transparent. Symlinks are just another file, more-or-less. The fact that they only serve a function in relation to a POSIX filesystem is irrelevant.

They should be ignored by Windows systems because there simply isn't a clean way to translate them to/from Windows<->Unix. Any attempt here just adds tons of needless complexity. Just look at all the ridiculous nonsense that the SAMBA project has to deal with. No-Thank-You. Let's keep it simple. As long as the server recognizes them and will send them to/from valid clients (linux/mac) is all that matters.

[AkechiShiro]

You are correct @f1d094 I just wanted to try and at least get my foot in the door officially for this feature as @artonge seems to be concerned about multi-platform support and potential issue that could rise maybe a symlink in NTFS is not the same as on ext4 (symlink vs LNK or maybe I'm just wrong) and an abstraction is already handling these differences.

EDIT : Read your answer fully, I plainly agree

Regarding the fork, yes but I can't keep working without symlinks on my end, so I'll push this effort alone at first and see where it goes, if I can end up with something more polished, more tested, more focused on user's wanted feature than the current Nextcloud situation which is frankly totally dissapointing, we have to deal with, well that will be at least a win despite the hard difficulty of maintaining such a fork.

I'm working one job on my side but I do have time to work on a side project, if people donate and find the fork better than Nextcloud, then maybe this will help me focus on it full time. Maybe we can fix user centric issues, instead of putting AI in Nextcloud or making Nextcloud a replacement for .

[AkechiShiro]

Unfortunately, doing a fork is a very heavy lift and especially frustrating because this is something that should have been built-in from day one. Symlinks are very simple, both in concept and in execution. I am embarrassed on behalf of the Nextcloud team that nobody seems to have a grasp of filesystem basics. It is supposed to be at the core of what they do.

Given the current reaction of Nextcloud devs to this issue and others issues, I've started loosing hope Nextcloud will definitely get better with time, hence the motivation towards making a fork.

I don't mean to spite Nextcloud devs, they do work hard on a lot of features and aim to do their best but sometimes I just plainly don't understand what is currently prioritized on the project and why it is prioritized, I feel there is no real prioritization being done or proper organization on the project at the current scale, they are operating (really wanted user features can get ignored, let's take grouping user contacts for instance, no one works on it, I believe).

[f1d094]

Maybe we can fix user centric issues, instead of putting AI in Nextcloud or making Nextcloud a replacement for .

@AkechiShiro Well...if you decide to really do it, you should call the new fork:

Nixcloud: Works well with *nix!

:)

[taminob]

Hey @taminob, after discussing the PR internally, we think that this feature could cause too many issues, especially when dealing with compatibility between Windows, MacOS and Linux. Server side, this could be developed as an app to not have it in core, but client side there is no app system, and I fear that they probably won't accept to cover such use case.

Hi @artonge, thanks for the feedback. So if this was designed as an app (in apps/ like e.g. files_trashbin), would you be willing to accept this? I don't really see an issue with the client side since the feature can be hidden behind a feature checkbox. Did you ask the maintainers of the client apps if this is actually impossible? It is exactly the same for the end-to-end encryption app which isn't even in this repository but in a separate one.

@f1d094 @AkechiShiro I don't think a Nextcloud server fork is worth it for this since it's much easier to apply these simple changes as a patch on top of the latest official Nextcloud releases. If this actually is rejected, it makes more sense to fork the Nextcloud desktop application since the changes there are much more invasive and might lead to more merge conflicts in the future when applying as a patch.

[artonge]

o if this was designed as an app (in apps/ like e.g. files_trashbin), would you be willing to accept this?

Yes, if this is an external app, then no need for our approval :)

Did you ask the maintainers of the client apps if this is actually impossible?

No, you can try to ping them again in the desktop client PR?

[taminob]

Yes, if this is an external app, then no need for our approval :)

I'd like it to be part of the Nextcloud server repository (like the integrated weather_status, files_trashbin, ... apps) or at least in the nextcloud namespace (like the E2E encryption app) - otherwise I think it would be kind of weird to support it in the official desktop app.

No, you can try to ping them again in the desktop client PR?

👍

[susnux]

Just be a text file

Just want to mention that this is a pretty standard way to handle this, e.g. git is also handling symlinks exactly like this.

[AkechiShiro]

@taminob do you believe we should fork or let this be an external app ?

I just want to know what's your opinion on this ?

I don't think it's worth putting efforts alone on merging this feature in Nextcloud in a fork if the feature is going to be in an external app.

EDIT : As you've said a patch should be enough for this feature, so most likely just patching the package of Nextcloud would be enough to provide this feature hence a fork is not needed AFAIK.

[taminob]

@taminob do you believe we should fork or let this be an external app ?

I just want to know what's your opinion on this ?

I don't think it's worth putting efforts alone on merging this feature in Nextcloud in a fork if the feature is going to be in an external app.

EDIT : As you've said a patch should be enough for this feature, so most likely just patching the package of Nextcloud would be enough to provide this feature hence a fork is not needed AFAIK.

You're right in your edit, I don't think a fork of Nextcloud server makes sense. I'd probably prefer having this as an app than a patch, but both should be feasible - since I've never written a Nextcloud app, I still have to check how exactly this could be implemented.
But I think that should be fairly straightforward and I will test this together with the improvements using the metadata API soon.

The desktop client will be far more challenging if the changes get rejected there and if that happens, a fork of the desktop app might become the only option (a scenario I'd like to avoid if possible).