Detection of within-library symlinks is not ported to GIO in the 0.3.1 series

Bug #502148 reported by Thomas Zander
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Exaile
Fix Released
Undecided
Unassigned
Nominated for 0.3.x by Thomas Zander

Bug Description

The same issue that applied to Bug #406050 in the 0.3.0 branch at the moment affects the 0.3.x continuation after the file handling has switched to GIO.

Revision history for this message
Thomas Zander (thomas-e-zander) wrote :
Revision history for this message
reacocard (reacocard) wrote :

It should already work in trunk, lines 497-505 in xl/collection.py (part of _walk) are designed to handle this.

Also, your patch makes use of os.path, which isn't portable to gio locations other than file://, completely breaking the point of moving to gio in the first place.

Revision history for this message
Thomas Zander (thomas-e-zander) wrote :

Thanks for the quick response Aren. I am afraid I am not an expert on gio yet, so please forgive me the mistake(s).
Anyhow, the part of _walk you mentioned does cover but a fraction of the existing cases. In the following example it does not work. Suppose I have something like:

lib
 + Artists
    + Artist_one
       + Artist_one-Song_that_symlinks_to'../../Sampler/Some_Hits_Vol_213/Track 02'
    + ...
 + Sampler
    + Some_Hits_Vol_213
       + Track 01
       + Track 02
       + ...

In this case, during scanning (I am referring to the code block mentioned by you) exaile will scan the Artist_one folder and the 'dir' variable will correspond to that folder. It will then resolve the link target and come to the following code line:
fil2 = fil.get_child(target)
Then, fil2 will erroneously be resolved to 'lib/Artists/Sampler/Some_Hits_Vol_213/Track 02' (if the line was fil2 = fil.get_parent().get_child(target) it would be correctly resolved), but this does not matter because in any case the resolved link would never satisfy the condition a few lines below:
if fil2.has_prefix(dir):
    continue
because fil2 would point to 'lib/Sampler/Some_Hits_Vol_213/Track 02' while dir still points to 'lib/Artists/Artist_one'

As far as I can see, the implementation in trunk does not work in cases where a symlink contains '../'
This problem could only be resolved in _walk() similar to the 0.3.0-branch if _walk() would not check fil2 against the prefix of dir but to the basedir of the library path that is scanned. Correct?

And one more question to your comment. You say that my patch would not work with other objects than file:// . Wouldn't the same check that the code block mentioned by you performs before using os.path,
if not "://" in target ...
be able to protect my patch against that as well?

Anyhow, if you have any hints to what a committable solution to the problem would be, I'd be happy to spend a little time workin on it.
Best regards

Revision history for this message
reacocard (reacocard) wrote :

> if the line was fil2 = fil.get_parent().get_child(target) it would be correctly resolved
or just use dir.get_child(target), since we know dir is fil's parent already

> ... if _walk() would not check fil2 against the prefix of dir but to the basedir
> of the library path that is scanned. Correct?
basically, yes. probably best to use the passed in 'root' object instead of self.location, since _walk will probably be factored out to a different module in the future, and for the use in Library root will always be equivalent to self.location anyway.

> if you have any hints to what a committable solution to the problem would be,
> I'd be happy to spend a little time workin on it.
The two changes i outlined above based on your feedback seem quite reasonable. If they resolve it for you (i'll attach the patch for you to test), I'll commit it. The changes don't appear to break normal scans on my system, at least.

> Wouldn't the same check that the code block mentioned by you performs
> before using os.path be able to protect my patch against that as well?
Probably true, but i would prefer a pure-gio solution if at all possible. The only reason I have an os.path call in there at all is because as far as I can tell, gio doesn't provide any equivalent of isabs (or really any handling of non-absolute paths at all beyond resolving them to absolute paths)

Revision history for this message
reacocard (reacocard) wrote :
Revision history for this message
Johannes Sasongko (sjohannes) wrote : Re: [Bug 502148] Re: Detection of within-library symlinks is not ported to GIO in the 0.3.1 series

> as far as I can tell, gio doesn't provide any equivalent of isabs

AFAIK you can feed an absolute path to get_child and it will work fine.

>>> gio.File('.').get_child(r'C:\test')
<__main__.GLocalFile at 0200C418: file:///C:/test>

Haven't tried that in Linux, but I imagine it'd be the same.

Revision history for this message
reacocard (reacocard) wrote :

> AFAIK you can feed an absolute path to get_child and it will work fine.
huh, so you can. rather counter-intuitive, but it works.

Revision history for this message
Thomas Zander (thomas-e-zander) wrote :

Great, Aren's patch works for me!
You have already committed it to the trunk if I am not mistaken?
Then we can probably close this bug report. Thanks again!

reacocard (reacocard)
Changed in exaile:
status: New → Fix Committed
milestone: none → 0.3.1
reacocard (reacocard)
Changed in exaile:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.