silverstripe / silverstripe-assets

Silverstripe Assets component
BSD 3-Clause "New" or "Revised" License
9 stars 65 forks source link

DB query in File::find($filepath) should be case-sensitive #493

Open micschk opened 2 years ago

micschk commented 2 years ago

Proposed fix: make File::find() case-sensitive by applying the:case searchfilter on the Name query;

    public static function find($filename)
    {
        // ...
        $item = File::get()->filter([
                'Name:case' => $part,
                'ParentID' => $parentID
            ])->first();
        // ...
    }
michalkleiner commented 2 years ago

Makes sense, though I wonder how many files would suddenly become missing...

dhensby commented 2 years ago

I think a safer implementation would be to not allow duplicate file names (by case) - I'm surprised this is even possible 🤔

micschk commented 2 years ago

Makes sense, though I wonder how many files would suddenly become missing...

None should go missing (unless there's somehow a discrepancy between filenames in the DB and on the filesys, but that could be considered a data-integrity issue which should be fixed anyway/elsewhere?). If however issues like files gone missing are indeed to be expected maybe we could fallback to case-insensitive query if no records are found via case-sensitive?

micschk commented 2 years ago

I think a safer implementation would be to not allow duplicate file names (by case) - I'm surprised this is even possible 🤔

Safer implementation possibly, but not a correction of this issue imo :-)

This situation is possible when loading files into File dataobjects manually (for example when checking for existing files via file_exists() instead of File::find()). Even if the decision would be to disallow duplicate filenames eg when uploaded via the CMS, still the current result of File::find() would be incorrect.

Also I did notice what I think is at least one occurence of this issue in CMS-uploaded files as well.

dhensby commented 2 years ago

Historically I know case-sensitivity (including database sensitivity) has been considered quite a bit around files. We should try to find the historical decisions around this.

We need to remember that we are a framework that should be striving to provide interoperability regardless of the case-sensitivity of the filesystem being used. That must be the priority, if that means not allowing files to share names regardless of case, then that's what we should do (if that's decided to be the best approach).