Bugzilla – Bug 576
Server got stuck in shortcut-folder-loop and dies
Last modified: 2008-12-18 11:54:36 UTC
Somehow I produced a shortcut to a folder inside this folder - and as you can think this isn't the best idea because slimserver follows every shortcut I don't know how often it did the loop until it died with: Can't use an undefined value as an ARRAY reference at C:/SLIMSERVER/server/Slim/Utils/Scan.pm line 194. Maybe it's possible to stop such a loop before it starts to be a neverending story? I still have the scan log from a few loops before the crash - but I think the problem is easy to understand. But if you need it: just ask! Regards, Roland Fischer
Created attachment 141 [details] The log before the server died
perhaps testing any shortcut before we descend. any .lnk that has a base path equal to the music folder should be ignored? This way, users could crosslink for their own use in naviagting with their FileManager, but slimserver will just ignore any internal links knowing that those links are all read in from the raw scan.
Created attachment 142 [details] skips suspected recursive links while scanning the audiodir tree, ignore any link that obviously points back to another directory in the audiodir tree.
I tried the patch from KDF but it didn't work as expected and started reading scan.pm and after that misc.pm and found the solution: in misc.pm theres a sub urlFromWinShortcut with the comment: # the following pattern match throws out the path returned from the # shortcut if the shortcut is contained in a child directory of the path # to avoid simple loops, loops involving more than one shortcut are still # possible and should be dealt with somewhere, just not here. so the error happens at this stage - because the wrong path values are still returned... and the solution is to add a return ""; at line 120 in misc.pm - at least it works for me and I can't see a problem with it. Regards, Roland Fischer
ah ok, I may have set up my test in a different way. the patch I gave relies on detecting ANY shortcut that matches the music folder tree, but this wouldn't work in the playlist tree, for example. You added return would block a simple short loop back onto itself, and that's a good idea. This patch needs to progress to hopefully avoid more cases.
erk...after testing this one line change to Misc.pm, it causes bug580 to reoccur. if you return "" from urlFromWinShortcut (now renamed to pathFromWinShortcut for 5.3.1) then when a user is browsing the music folder, any shortcut will descend into the root folder of that drive. this is a security breach.
Created attachment 146 [details] modified scan.pm and misc.pm for 5.3.0 Don't know how to create a diff file :(
Comment on attachment 146 [details] modified scan.pm and misc.pm for 5.3.0 sorry - more bugs left than solved - just ignore this silly attachmend or delete it...
I'm still confused as to why my patch fails for you. In my test I have d:\mp3 as my music folder, and I've created d:\mp3\test, with a shortcut inside it to d:\mp3\test. This normally caused an infinite loop, but my patch simply noted it as suspicious and carried on.
Roland, is your problem shortcut not in your music folder path? I can see my patch not working if, for example, your music folder is d:\mp3 then you have a shortcut d:\mp3\"shortcut to c:\music". if you have any shortcuts inside c:\music that link back to d:\mp3, c:\music or any subfolder, you have a loop. I believe the cache stores dirs as well so there may be a way to extend the patch I had to look for that. it would be nice to do it in pathFromWinShortcut, but its hard to return from that without risking the "" secutity problem.
Well, I tried to solve the bug on it's base - at first in pathFromWinShortcut and next in readList - but whatever I tried had the one or the other disadvantage - therefore Kevins patch is the best I can think of. Although it only scans for loops inside the music folder - and fails if the current link produces a loop inside a folder that has been reached follwing another link to another volume... In pathFromWinShortcut there's already a simple and effective Loop-Detection - but because it returns a value in either case it's of no use currently. The best way would be to find a way to stop the loop in readline - whenever pathFromWinShortcut returns a string withoug "file:" at the beginning it's a bad one - but I don't know how I could catch this without producing other bugs... Roland
it would be nice if there was a simple way to test against %addToList_jobs to see if the link leads to a dir that is already in the jobs list. This would solve the problem universally.
Created attachment 178 [details] a new idea This patch, instead of comparing to audiodir will just skip any .LNK that has a cached playlist entry. This should skip any links to directories that have already been scanned. Its unlikely that a link to a dir that hasn't been scanned would cause a loop, tho it might get 'scanned' twice if the link is found first.
@new idea: well i think this is really a good idea - but the server need to wipe it's cache whenever he performs a rescan. Does he do this? I don't know - but i think he does :) Roland
We could always create a new bug for the creation of a method for testing if a link has any updated files in order to force a rescan of that path. At least the infinite loops can be avoided right away.
Created attachment 688 [details] Patch that works for me. This works for me - the path that's being extracted from pathFromWinShortcut isn't a URL, so the correct entry is never being put into the DB, which causes the loop to stop. This begs the question - should pathFromWinShortcut() actually return a fileURL? Or rather, should the call be fileURLFromWinShortcut() ? There appears to be only one caller that requires an actual path - at the top of Scan.pm
Fixed in subversion change 3837.
This bug appears to have been fixed in the latest release! If you are still experiencing this problem, feel free to reopen the bug with your new comments and we'll have another look. Make sure to include the version number of the software you are seeing the error with.
Routine bug db maintenance; removing old versions which cause confusion. I apologize for the inconvenience.