Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FileView can not open symlinked folders #1476

Closed
buhtz opened this issue Jul 16, 2023 · 1 comment · Fixed by #1650
Closed

FileView can not open symlinked folders #1476

buhtz opened this issue Jul 16, 2023 · 1 comment · Fixed by #1650
Assignees

Comments

@buhtz
Copy link
Member

buhtz commented Jul 16, 2023

This issue needs further investigation. I won't touch it yet because I'm in the middle of refactoring qt/app.py while I uncovered this issue.

Reproduce:

  • Select "Now" in the timeline left to show the current state ot the filesystem and not any of the snapshots.
  • Double click on a folder which is a symlink.
  • Nothing will happen. The folder does not open.

The relevant code in the GUI is here:

backintime/qt/app.py

Lines 1338 to 1353 in 1003d36

def openPath(self, rel_path):
rel_path = os.path.join(self.path, rel_path)
full_path = self.sid.pathBackup(rel_path)
if os.path.exists(full_path) and self.sid.canOpenPath(rel_path):
if os.path.isdir(full_path):
self.path = rel_path
self.path_history.append(rel_path)
self.updateFilesView(0)
else:
# prevent backup data from being accidentally overwritten
# by create a temporary local copy and only open that one
if not isinstance(self.sid, snapshots.RootSnapshot):
full_path = self.tmpCopy(full_path, self.sid)
self.run = QDesktopServices.openUrl(QUrl('file://' + full_path))

In there the self.sid.canOpenPath() does return False.

It is not a serious bug. But I have the feeling that touching this will cause some more trouble and much more need to refactor. There might be a big workload hidden behind it. 😄

@aryoda
Copy link
Contributor

aryoda commented Dec 21, 2023

The idea is that symbolic links shall only be opened if the link points to a folder inside of the current snapshot:

backintime/common/snapshots.py

Lines 2447 to 2465 in 2f7b609

def canOpenPath(self, path):
"""
``True`` if path is a file inside this snapshot
Args:
path (str): path from local filesystem (no snapshot path)
Returns:
bool: ``True`` if file exists
"""
fullPath = self.pathBackup(path)
if not os.path.exists(fullPath):
return False
if not os.path.islink(fullPath):
return True
basePath = self.pathBackup()
target = os.readlink(fullPath)
target = os.path.join(os.path.abspath(os.path.dirname(fullPath)), target)
return target.startswith(basePath)

I have tested this by creating a symbolic link in the source folder and point to a sub folder within the source folder

source/data/
source/link_to_data/

Edit: Stupid me, I had the "expert option" _Copy links (dereference symbolic links) activated so I have to retest all this...

Test results:

  1. After taking a snapshot the linked folder is contained as materialized folder (= no longer a link): It contains the same files as the originally linked folder (as duplicated copies !!!)
    The reason is that we use the --copy-links argument for rsync which transforms a symlink into the referent file/dir

  2. The check in canOpenPath() if the link target is a folder within the snapshot is never used in this standard "browse backup" scenario.

  3. If I manually create a symbolic link in the snapshot folder to a folder within the snapshot the link check does work correctly and the linked path is opened.

  4. If I manually create a symbolic link in the snapshot folder to a folder outside the snapshot the link check does work correctly and the linked path is not opened.

  5. In the Now "snapshot" (= source folder) a link is never opened because the variable basePath is wrong (contains /backup).

So to fix this we need to decide if we want to
a) follow each link in Now (no matter if it points inside or outside the source folder)
b) follow only links to target folder inside of Now
b) never follow links (current behavior)

Option b) is the easiest fix I think (by checking for the translated "Now"):

       if  self.isRoot or not os.path.islink(fullPath):  # should be the same as:  self.tag != _'Now'  # Snapshot ID's tag
            return True

BTW:

  • canOpenPath should be renamed (refactored) to IsPathInsideSnapshotFolder or similar to make it more obvious what the function really does.
  • self.isRoot is also a misnomer (means the backup source root folder displayed as "Now").
    It even collides with the meaning of tools.py#isRoot() (which checks if the current process is running with root user rights)
  • The use of --copy-links should be mentioned in our FAQ as a reason for huge backups (if the backups source folder contains one or multiple links to target folder with large disk space consumption).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants