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

FindFiles(): Letter case is ignored #904

Closed
4 tasks done
infeo opened this issue Jun 5, 2020 · 3 comments
Closed
4 tasks done

FindFiles(): Letter case is ignored #904

infeo opened this issue Jun 5, 2020 · 3 comments

Comments

@infeo
Copy link
Member

infeo commented Jun 5, 2020

Environment

  • Windows version: 10 Pro N 1909
  • Processor architecture: x64
  • Dokany version: 1.3.1.1000
  • Library type (Dokany/FUSE): Dokany

Check List

  • I checked my issue doesn't exist yet
  • [-] My issue is valid with mirror default sample and not specific to my user-mode driver implementation
  • I can always reproduce the issue with the provided description below.
  • I have updated Dokany to the latest version and have reboot my computer after.
  • I tested one of the last snapshot from appveyor CI

Description

By design Dokany also supports case sensitive filesystems (mainly because it is the devs task to implement the file system). Also by design, the developer can decide to implement only the function FindFiles() for file listing and let the dokan handle possible filtering/matching or implement the slightly more complex FindFilesWithPatterns and having the chance to use own filter expression.

But the first case does not always lead to the correct listing result. Looking at the direcotry listing base function DispatchDirectoryInformation() in directory.c, in case of a success, the retrieved file list is processed/filtered by the function MatchFiles():

dokany/dokan/directory.c

Lines 637 to 639 in adc18b3

// extract entries that match search pattern from FindFiles result
index = MatchFiles(EventContext, eventInfo, openInfo->DirListHead,
patternCheck, DokanInstance);

The function contains the parameter BOOL patternCheck, which indirectly decides if the retrieved list is filtered by Dokany or not. If it is TRUE, every entry in the list checked if it matches this pattern:

dokany/dokan/directory.c

Lines 410 to 412 in adc18b3

// pattern is not specified or pattern match is ignore cases
if (!pattern ||
DokanIsNameInExpression(pattern, find->FindData.cFileName, TRUE)) {

By knowing, that for every FindFiles() call patternCheck is set to TRUE, here one can see the bug for case sensitive file systems. The function DokanNameIsExpression has as a third parameter a boolean value deciding if the case of the search pattern is ignored or not. It is fixed to TRUE, therefore the case is always ignored.

Suggested Solution

There are from my point of view two options:

Option A: Change API

Change the definition of FindFiles() by add a pointer to a boolean value, which can be set.

Case sensitivity is inherently tied to the file system. One may think that changing the default behaviour to not ignore the case is the way to go since it is more stricter, but this isn't the Windows way, where a lot of applications don't care about the letter case. Therefore one needs a way to tell Dokany to care about the case. And this could be done by exactly such a pointer.

Option B: Update Docs

Update the documentation to make it clear that FindFiles() is kinda case insensitive.

Quick and easy fix, without breaking the current API. 😉

Logs

I tested this with the windows cmd shell and the case sensitive file system cryptoFS. I created two directories (test and Test). each containing a file (small.txt and big.txt) and tried to navigate to both of them.
cmd_output_buggy.log
cmd_output_correct.log

@infeo infeo changed the title FindFiles() vs FindFilesWithPattern() FindFiles(): Letter case is ignored Jun 5, 2020
@Liryna
Copy link
Member

Liryna commented Jun 5, 2020

Hi @infeo ,

In reality, dokan need more changes to clearly support case or non case sensitive option.
See #839 the code is practically ready and I missed FindFiles! Thanks for reporting it!
I was waiting to finish MemFS to finish be able to make proposer testing.

The option set at mount to enable case or non case sensitive could be used to set the correct DokanNameIsExpression option.

@infeo
Copy link
Member Author

infeo commented Jun 5, 2020

dokan need more changes to clearly support case or non case sensitive option

uhh, didn't knew about that. Guess i'm lucky that I didn't stumbled upon other case problems 😅

I was waiting to finish MemFS to finish be able to make proposer testing.

Oh, yes, that is a good idea. Testing case sensitivity with the mirror examples is kinda useless^^

The option set at mount to enable case or non case sensitive could be used to set the correct DokanNameIsExpression option.

Yes, that is one possiblilty. Also thought about suggesting it, but it would then be decoupled from the actual implementing filesystem which is represented by the DOKAN_OPERATIONS struct. (Your filesystem can be the one time case sensitive and the other not, depending on the mount options.)

@Liryna
Copy link
Member

Liryna commented Jul 25, 2020

Closing it as duplicate with #839

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

No branches or pull requests

2 participants