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

fix: Handle utf-8 file paths on Windows #125

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

taratatach
Copy link
Contributor

Although the WindowsBackend uses unicode aware Windows file API
functions when processing events, it does not do so when searching
directories and their content when creating a new Subscription.
This results in invalid file paths stored in the subscription's tree
when the file name contains non-ascii UTF-8 characters and thus,
dropped events when they rely on the existance of a dir entry with the
valid path.

e.g. consider, we have a directory dir/, containing a file ``

1. Create the following hierarchy: `dir/` `dir/spécial` (i.e. the 3rd letter of the file name is a French e-acute)
2. Subscribe to `dir/` 3. Delete the file `spécial`

-> no events will be notified for the deletion because the file path
can't be found in the subscription tree

Using unicode aware search functions in BruteForceBackend::readTree
solves this.

  Although the WindowsBackend uses unicode aware Windows file API
  functions when processing events, it does not do so when searching
  directories and their content when creating a new Subscription.
  This results in invalid file paths stored in the subscription's tree
  when the file name contains non-ascii UTF-8 characters and thus,
  dropped events when they rely on the existance of a dir entry with the
  valid path.

  e.g. consider, we have a directory `dir/`, containing a file ``

    1. Create the following hierarchy:
      `dir/`
      `dir/spécial` (i.e. the 3rd letter of the file name is a French e-acute)
    2. Subscribe to `dir/`
    3. Delete the file `spécial`

    -> no events will be notified for the deletion because the file path
    can't be found in the subscription tree

  Using unicode aware search functions in BruteForceBackend::readTree
  solves this.
@taratatach
Copy link
Contributor Author

The one test that's failing because it takes more than 2s to complete runs in 52ms on my own machine which isn't a beast. Is it possible that the setup is longer on the CI?

@mischnic
Copy link
Member

mischnic commented Jan 6, 2023

You could lengthen that timeout if you want to test that. But I don't think that is likely to be the problem

this.timeout(5000);

@benmccann
Copy link

@taratatach it looks like there's a merge conflict on this PR

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

Successfully merging this pull request may close these issues.

3 participants