! Please note that this is a snapshot of our old Bugzilla server, which is read only since May 29, 2020. Please go to gitlab.xfce.org for our new server !
Wine symbolic link looping when searching with Catfish
Status:
RESOLVED: FIXED
Product:
Catfish
Component:
General

Comments

Description bennyhillthebest 2019-12-12 16:50:27 CET
Created attachment 9305 
Looping in action

Hello, i use Manjaro XFCE and with Catfish 1.4.10 and i started experiencing symbolic link looping because of Wine.

I am probably not the only one: https://ubuntuforums.org/showthread.php?t=2431588

I guess symbolic link traversing is cool, but maybe it would be useful to have a knob in the settings for turning it off.

Thank you very much for the hard work for this very sane and lightweight desktop environment.
Comment 1 Filip Brygidyn 2019-12-14 19:43:50 CET
Created attachment 9309 
proposed_patches

The symlink traversal was introduced in 1.4.10 and I can confirm that the looping is a bug.
The list of to-be-traversed paths was not filtered.

I am attaching 2 patches that fix this behavior. They can be applied on top of 2 other patches I submitted in bug #15985

The first patch simply fixes the bug -  to-be-traversed dirs will be filtered.
It is still possible to get duplicate result with it:
- for dirs it's max of 2 results - one real and one link depending on traversal order
- for files there is no limit - all file links will be found (there can be many links pointing to the same file), just the traversal is limited, so again the same link may be listed 2 times (one accessed by real path, one by link path)

Second patch makes sure that there are no duplicates (real-path wise).

@Sean Davis
I am not sure if the second patch is what we actually want - skipping multiple links to the same file for example. What do you think?

For reference:

When we have following structure:
-root_dir+
         +-home+
               +-user+
                     +test+
                          +test.txt
               +-.wine+
                      +dosdevices+
                                 +z:->root_dir/home
         +-file.txt->home/user/test/test.txt

and search for 'test' in 'root_dir':
- Without any patch we get looped results - in my case it stops at 84 results, can't really say why it stops, links seem to stop working at some point.

- With only the first patch we get :
  - root_dir/test.txt < file links are not filtered
  - root_dir/home/user/test < real path
  - root_dir/home/user/test/test.txt < file links are not filtered
  - root_dir/home/.wine/dosdevices/z:/home/user/test < path after traversing the link once (any subsequent ones are filtered)
  - root_dir/home/.wine/dosdevices/z:/home/user/test/test.txt < file links are not filtered

- With both patches we get:
  - root_dir/test.txt
  - root_dir/home/user/test
In this case we get only one path to per 'real file' and the first path to it we encounter will be displayed. So depending on the dir structure and prioritization we can get path to either a link or to a real file.
Comment 2 Filip Brygidyn 2019-12-14 19:55:05 CET
Created attachment 9310 
first_patch

Bugzilla can't seem to detect correct MIME for tar.gz, attaching plaintext instead
Comment 3 Filip Brygidyn 2019-12-14 19:56:01 CET
Created attachment 9311 
second_patch
Comment 4 Sean Davis editbugs 2019-12-19 12:37:53 CET
@Filip,

I applied your first patch, which seems like something we do want. As for the second, I think it's preferable to display both versions since the user might know it as one and not the other.

I also applied this patch, let me know your thoughts:

https://git.xfce.org/apps/catfish/commit/?id=56a3d50442d8509fcdce644251e1a14dc476871e

This basically sandboxes the searching area to not search above the current directory. For example, Wine creates a symbolic link z: that points to /

~/.wine/dosdevices/z: -> /

If you're searching within home, you will now also search the entire file system and all mounted partitions. This patch prevents that.
Comment 5 Sean Davis editbugs 2019-12-21 13:07:33 CET
Marking resolved with the release of Catfish 1.4.11.
Comment 6 Filip Brygidyn 2019-12-21 22:18:48 CET
@Sean Davis

I tested the 1,4,11 you pushed and I found another weird behavior:

with following structure:
         -wine+
              +-drive_c+
                    +test+
                         +test.txt
              +-dosdevices+
                     +c:->../drive_c

With  only 'walk' method enabled I launch catfish in 'dosdevices' directory and search for 'test'.
I find 'test.txt' only.

This seems kind of alarming:
- If I understand your intention about sand-boxing correctly then catfish is traversed outside the sand-box
- If I understand it wrong and we should follow the c:->../drive_c link then the 'test' directory is missing from the results

This 'missing result' issue seems related to the link-following and is present since 1.4.10.
I thought my patches should have fixed missing results like this so I must have  overlooked something while writing them.
I will take a look at this after Christmas, don't have time now. I will open another bug report when I do.




I also have a question about the sand-boxing patch and your intention for it:
Doesn't this patch effectively remove functionality of link following?
If we are sand-boxed inside the initial directory selected by the user then we will only follow links that point to children of this directory.
But even without following links catfish would still find all those dirs/files precisely because they are children of the user selected dir.
So in effect following links would now only affect the order at which the files are found.
Comment 7 Filip Brygidyn 2019-12-22 08:29:01 CET
Forget my question about the sand-box, I understand what you meant: your patch simply prevents going to the ancestor of the current dir. You explained it well but I got confused by a word 'sand-box' and let my thoughts run wild.
Comment 8 Filip Brygidyn 2019-12-30 18:44:34 CET
I opened bug #16318 about the missing search results described above

Bug #16272

Reported by:
bennyhillthebest
Reported on: 2019-12-12
Last modified on: 2019-12-30

People

Assignee:
Sean Davis
CC List:
2 users

Version

Attachments

Looping in action (304.68 KB, image/png)
2019-12-12 16:50 CET , bennyhillthebest
no flags
proposed_patches (1.27 KB, patch)
2019-12-14 19:43 CET , Filip Brygidyn
no flags
first_patch (1.73 KB, patch)
2019-12-14 19:55 CET , Filip Brygidyn
no flags
second_patch (1.50 KB, patch)
2019-12-14 19:56 CET , Filip Brygidyn
no flags

Additional information