ADD readlink support for linux (POC,needs remerge, needs more features)

[basos9]

Support for following symbolic links is added to the linux (and possibly macOS, but no tested) client.

Here follows a use case description of an upload and download scenario.

UPLOAD

When the client (uploader) encounters a symlink (in a platform that supports this and also the platform client support this), the client would sync the contents.

• If it is a file symlink sync the file
• If it is directory symlink sync the contents of the directory.
• If we do not support it, we print a nice exclamation mark and ignore this (present behavior)
• If the link is broken, we print a nice exclamation mark (error level) and temporarily ignore this resource

For the sake of example lest say that I create the directory dir-LNK (witch points to /data/dir). The client will sync a directory named dir-LNK (without anything on the server to denote this is/was a symlink)

DOWNLOAD

This was the first client (uploader lets say) scenario. Now, when a new client (downloader) registers to sync with this folder, a normal sync should occur, agnostic of symlinks. More specifically, there are two possibilities:

• an empty root, do the syncing, without any symlink knowledge,
  create directory named dir-LNK inside the nextcloud root.
• an existing root with symlink setup manually.
  I create a symlink in the nextcloud root dir-LNK which points to /externalstorage/dir
  When sync begins, just read the conents for dir-LNK and do the sync (i.e. do not treat symlink specifically
  • If the link is broken, we print a nice exclamation mark (error level) and temporarily ignore this resource

So in general, we do not treat symlinks specifically when they exist, except for the case when the link is broken, when we print an error and ignore the resource. This means that there is no need to ever write a symlink. In other words all the symlink hassle is a client issue. The server is agnostic of symlink.

ADDITIONS

After the implementation I read this discussion.

So this is a proof of concept solution with 2nd comment of callegar: i.e. client dereference. As is see there should be the following improvements:

1. make client symlink dereferencing optional and opt in (for users who know what they are doing)
2. FIX, in current patch (52fcd8dbc8bdc00a9f1e8199b427c97c60272164) only links in level one are followed. E.g ./mylink is followed, but ./folder/onotherlink is not
3. test/fix symlink loops
4. test, document cross filesystem links

TODO TEST

• Testing for various add, remove, rename scenarios
  FOLDER

  • Rename
  • Delete
  • Relink
    FILE
  • Rename
  • Delete
  • Relink
    OTHER

• Change link from file to folder and vice versa.

• Testing for windows (nothing should change)

• Testing for macOS (symlinks should be supported)

[Banderi]

This sounds amazing, just the one thing I felt missing that I would love to have! Would it be at all possible to have a nightly/testing binary for this before it gets added to the master for the automated builds? I've tried to wrap my head around CMake but it's always proven to be my worst nightmare...

[basos9]

Here is a testing build for anybody caring to test the present functionality

Run nextcould-run.sh

[garethmcc]

Is there any chance this will be available soon. This single change will make the world of difference for my own use of Nextcloud

[jcklpe]

I just wanted to add that I'm VERY VERY much looking forward to this feature being added. Thank you baros!

[detrout]

The way I had tried to use a symlink with nextcloud was to provide an alias. Specifically I was tired of looking for a photo that's in InstantUploads so I added a symlink that points to the camera file.

In the case of a relative symlink pointing to another file in the sametree it might be nice to keep it as a symlink.

[jcklpe]

also I don't know if this issue might be relevant or not? nextcloud/server#11879

[rickdoesdev]

Edit: duh; I just realised the file changed in this PR specifically was for linux handling.

How difficult would it be, do you think, to apply similar updates for windows csync_vio_local_win.cpp
Given windows does support symlinks natively and has done for quite some time?

[basos9]

@rcuddy
Have you tried this on windows ? Maybe it works out of the box (for the case we are discussing here, of not handling sym links very specifically, i.e. resolve the link when syncing).

For example: you can create a directory symlink inside the nextcloud root, pointing somewhere.outside, and see what happens.

For the other commends, this is still a proof of concept. It needs testing for various cases and implementation of opt in option.

@jcklpe it seems it isn't

[rickdoesdev]

@basos9 I may have misunderstood the intent of the change, apologies if so.

Symlink created on windows; C:/nextcloud/file-lnk.txt -> C:/nextcloud/file.txt results in a client sync message saying symlinks aren't supported and it doesn't even try to send them up the wire.

If a symlink exists on the server within the data directory the web ui sees both the original file and the link file. Editing them via the web ui shows changes reflect instantly in both files on the web ui / server. Unfortunately when the windows desktop client pulls them down it pulls them down as actual files, it doesn't preserve the link relationship. This is what I was hoping you were trying to fix!

[basos9]

As you mention two cases:

The first scenario is related to this one. And it should just copy 2 files on server (file-lnk.txt and file.txt) with the same content.

In the second it seems you want to sync symbolic links (i.e. treat them specifically), this is discussed in here.

[detrout]

I didn't think windows supported symlinks either... but apparently it was added with vista and users can be allowed to create them in win10. General docs https://docs.microsoft.com/en-us/windows/security/threat-protection/security-policy-settings/create-symbolic-links Mklink command https://docs.microsoft.com/en-us/windows-server/administration/windows-commands/mklink On April 22, 2019 6:06:19 AM PDT, Rick <[email protected]> wrote: @basos9<https://github.com/basos9> I may have misunderstood the intent of the change, apologies if so. Symlink created on windows; C:/nextcloud/file-lnk.txt -> C:/nextcloud/file.txt results in a client sync message saying symlinks aren't supported and it doesn't even try to send them up the wire. If a symlink exists on the server within the data directory the web ui sees both the original file and the link file. Editing them via the web ui shows changes reflect instantly in both files on the web ui / server. Unfortunately when the windows desktop client pulls them down it pulls them down as actual files, it doesn't preserve the link relationship. This is what I was hoping you were trying to fix! — You are receiving this because you commented. Reply to this email directly, view it on GitHub<#1067 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AAHOBPU6IQXZDZZAM4CU62DPRWZ4XANCNFSM4GUFKBXQ>.

[k7hoven]

I went through all this some time ago, and sorry if this has already been said, but just to make sure: I don't think nextcloud should ever "follow symlinks" by default. By default, symlinks should be synced as symlinks.

Personally, I have felt the need for both of the two features. However, it should be known locally if it is the case that a specific symlink should be followed and not synced as a plain symlink. The configuration of that should be local.

[xrobin]

Under Linux the expected behavior is to follow symlinks. While I agree it might be a good idea to allow disabling it, I would have it turned on by default.

Regarding the change itself: csync/csync.h defines an enum value CSYNC_STATUS_INDIVIDUAL_IS_SYMLINK which is used throughout to mark and disable symlinks. Shouldn't it be changed as well?

[alexeymuranov]

Under Linux the expected behavior is to follow symlinks

Behaviour of what program? Expected by whom? It is not an expected by me behaviour of a backup or version control or synchronisation software.

rm -rf does not follow symlinks (except the ones explicitly listed as arguments, i think), does it behave unexpectedly?

[xrobin]

Behaviour of what program

Bash, Python, my Gnome desktop environment, my file manager, most programs I can think about won't care if a file is a symlink or has a symlink somewhere in the path... they will just follow it and, to me, operate exactly as if it was a normal file/folder. Maybe show a different icon, that's about it. The Dropbox client follows symlinks too.

cp might be a relevant exception indeed.

Expected by whom?

Not sure, hard to answer. At least me. My best guess would be "everyone who's not a C programmer", but that might be a bit of a stretch.

[alexeymuranov]

Well, Nextcloud is neither a programing language, nor a desktop environment... I only see an analogy with Dropbox, but i do not see yet why Dropbox's behaviour in this case is the "correct" one.

[basos9]

It seems that there are use cases and supporters for both strategies. So in the far future the ideal would be both to implemented and configurable per sync root (i think). Also we should agree what the default would be if any. But I would like to see implementation first (tools), yes for both scenarios, since people use them, and then decide about policy.

My opinion about defaults:

• client derefencing (type B, this patch when complete) should be off by default (despite being my use case). And I understand @xrobin 's reasoning. But there are security and possibly technical issues that make this a power user, opt in, feature.
• client sync links (type A). We should first have a reference implementation with proper sanity checks (links pointing inside sync root, link strategy configuration change corner cases, ...) and test it for a while. If all goes smoothly this could be the default indeed. In the reasoning that since we will have done so much work to handle all symlink schenarios, it's more convenient to enable by default the most safe strategy (type A), than nothing (type C, ignore symlinks, present situation).

For clarification I copy my comment from another discussion.

... I understand that there are different use cases that might benefit from either implementation. I just restate the symbolic link processing strategies:

TYPE A (SLINK): No dereferencing 
TYPE B (SDER). Client dereferencing
TYPE C (SIGN). Ignore symlinks

Some opinions:
I came because I needed the type B (sder). I implemented a poc. It is a poc since it does not address the issues you mention (loop check, file system notifications).
I agree that the multiple sync folders functionality, partially solves this issue, but again it has the problem of copying the data in the synced folder under the root folder. With symbolic links you can preserve your documents organization and just symlink the stuff you want to publish under root, without wasting space.
Regarding security, indeed, this should be added as an advanced opt in option, targeted to power users, who want this conveniency.
Also I can see that it should be more practical to have this option per synced root (per client).
Note, that this is a client implementation. The server does not know anything. Also clients that are on platforms no supporting symlink, they too, not do any special processing.

Regarding type A (slink), I can see use cases, but I think it should be limited to symbolic links targeting paths inside the sync root. To distribute links that link outside the root is not so practical, since:

a. The paths environment might be different from client to client (e.g. client a has folder /opt/data/, client b does not). If you need a client to modify a path outside the sync root, with type B (since you know what you are doing) you can add a symlink and only to the environments you want to (e.g. only on client a and not on b)
b. There is no cross platform meaning for e.g…" C:\data" vs “/home/data”

Also I consider this to be more difficult to implement, since you need to modify client and server to be aware of a new file type. In addition, client should consider all platforms specifically. Also you need to sanitize that links will be pointing inside the sync root.

And like you said, if they are to be implemented both, they should be mutually exclusive. Also the client should handle various corner cases

• client a type A, client b type B =>
  handle name clashes, on both ends, ignore if type mismatch,
  b skips remote links,
• client a type A, client b no link processing
  b skip remote links
• client a type A, then change setting to no processing
  what do we do on remote? Do we delete the links ? Just leave them ? (maybe this suggests that link setting should be a remote setting shared by all clients ?)
  Check what is the present behavior with ignored files for inspiration.

Maybe more cases ...

[k7hoven]

I agree about much of @basos9 's last message, except that I would make the configuration local. This would also remove many of the corner cases.

What I mean is that there could for instance be a configuration file called .nextcloud_follow_symlinks (or .nextcloud_dereference_symlinks) in the directory in which the link is located. This file would contain the names of the links that should be followed. This file itself, however, would not be synced.

I think this way there will also be fewer problems with the listed corner cases with different per-client configurations.

[stale]

This request did not receive an update in the last 4 weeks. Please take a look again and update the issue with new details, otherwise the issue will be automatically closed in 2 weeks. Thank you!

[sebschlicht]

I've read through some GitHub issues and Nextcloud pages. Looks like the merging is blocked due to a missing review. What is the current state of this feature bundle? Personally, I can not use Nextcloud on my desktop without having (dereferencing) symbolic links working but I am curious about its state at a whole.

I've learned about three use cases, i.e.

• ignore symlinks as before
• treat them as-is (potentially limited to the Nextcloud root)
• not treat them differently than actual files (a.k.a. dereferencing)

If the first behavior would be the default, nothing would change for users.
This allows to implement the other two behaviors independently, by incrementally extending the UI and the business logic.

As far as I've understood, @basos9 worked out a poc implementing the third behavior.

• What is the overall progress of this poc?
• Is there any chance to merge it into Nextcloud?
• Is the second behavior required before merging or can these behavior indeed be implemented individually?

[mgallien]

@basos9 Do you still plan to work on this or should we just close it ?
As a first step, could you rebase on latest mater ?

[basos9]

Hi, unfortunately there is no time from my side for the time being. I would love to contribute to a future version if somebody does not follow. So we could close and reopen in the future.
Thanks

[ukos-git]

Could we leave an issue for this open?

[basos9]

I opened up an issue for this feature request #3335