Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
micschk opened this issue May 11, 2022 · 5 comments
Open

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

micschk opened this issue May 11, 2022 · 5 comments

Comments

@micschk
Copy link
Contributor

micschk commented May 11, 2022

  • There's two files (My-FILE.jpg and My-file.jpg) on a case-sensitive filesystem (as any webserver should be).
  • NowFile::find('My-file.jpg) will (wrongly) return the My-FILE.jpg record if that file/record was created earlier.
  • This is because File::find queries the DB on filename/path in the default case-insensitive manner.

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
Copy link
Contributor

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

@dhensby
Copy link
Contributor

dhensby commented May 11, 2022

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

@micschk
Copy link
Contributor Author

micschk commented May 11, 2022

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
Copy link
Contributor Author

micschk commented May 11, 2022

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
Copy link
Contributor

dhensby commented May 11, 2022

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants