_imagecache_strip_file_directory() function is flawed - assumes filepath contains File system path

Recently had this problem:

System administrator set up a new Drupal site that had been created using sites/default/files as the files directory. He created a new directory when moving the site over, sites/sitename.co.uk/files, and specified it as the files directory. All the files got saved in to there, which is fine, but it didn't match our set-up in version control, so when we realised we decided to move the files back.

When we did so, all ImageCache image src links broke. They went from being (correctly):

To:

I noted /sites/sitename.co.uk/files was being injected between the preset directory and the filefield directory. I also noted the _imagecache_strip_file_directory() seemed to be the last thing called, so I printed the $path output from this to screen, and low and behold, it wasn't stripped at all. It read:

When it should read:

That's because _imagecache_strip_file_directory() assumes the filepath will contain the site's files directory path, but sometimes it will not. I can clean this up using a MySQL script, but what if I had a multi-site sharing a files table in a central database? The file could be being called from any number of site files directories (each multi-site could have a different folder specified) to be served in *another* site with a different default files directory.

Edge case, I'll grant you, but it would be good to try and resolve this and make it just work.

My initial thought is to use a regular expression to grab the filename and just the filename. IMHO it would be better if _imagecache_strip_file_directory() did *totally* what it says on the tin, and just returns file.jpg. Then imagecache_create_url() should worry about getting the extra data to prepend on that, for example the filefield directory, if required. I don't know if FileField has a helper function to check if a given path is a file that belongs to it? That might work if it does.

Anyways, just thought I'd make you aware that the current implementation has flaws and does not work in all scenarios. I'll think on and if I come up with anything I'll post a patch. =)