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.

Sign up for GitHub

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

Jump to bottom

fix(windows): Handle root paths that cannot be canonicalized #10838

Merged
merged 7 commits into from
Sep 19, 2024
Merged

fix(windows): Handle root paths that cannot be canonicalized #10838

merged 7 commits into from
Sep 19, 2024

Conversation

alexmoon
Copy link
Contributor

@alexmoon alexmoon commented Aug 30, 2024 •
edited
Loading

If the root path element of a pattern could not be canonicalized, scope::fs::push_pattern would enter an infinite loop. This is because path.pop() does nothing and returns false when called on a path with no parent, which can happen even when the path element iterator is non-empty. This could occur on Windows, for example, with a pattern of \\?\**.

Fixes #10815

@alexmoon alexmoon requested a review from a team as a code owner August 30, 2024 18:06
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Aug 30, 2024 •
edited
Loading

Package Changes Through 7e5ef79

There are 6 changes which include tauri with prerelease, tauri-runtime-wry with prerelease, tauri-runtime with prerelease, tauri-utils with prerelease, tauri-cli with prerelease, @tauri-apps/cli with prerelease

Planned Package Versions

The following package releases are the planned based on the context of changes in this pull request.

package current next
tauri-utils 2.0.0-rc.12 2.0.0-rc.13
tauri-bundler 2.0.1-rc.13 2.0.1-rc.14
tauri-runtime 2.0.0-rc.12 2.0.0-rc.13
tauri-runtime-wry 2.0.0-rc.13 2.0.0-rc.14
tauri-codegen 2.0.0-rc.12 2.0.0-rc.13
tauri-macros 2.0.0-rc.11 2.0.0-rc.12
tauri-plugin 2.0.0-rc.12 2.0.0-rc.13
tauri-build 2.0.0-rc.12 2.0.0-rc.13
tauri 2.0.0-rc.15 2.0.0-rc.16
@tauri-apps/cli 2.0.0-rc.16 2.0.0-rc.17
tauri-cli 2.0.0-rc.16 2.0.0-rc.17

Add another change file through the GitHub UI by following this link.

Read about change files or the docs at github.com/jbolda/covector

@alexmoon
Copy link
Contributor Author

alexmoon commented Sep 4, 2024 •
edited
Loading

I've added a test case for the various flavors of Windows root paths. I ran into a number of edge cases that are not currently handled well, which I documented in comments in the test case. I can open an issue for those if you would like. A few examples:

  • Canonicalization differs depending on if the path exists. For example "C:\Exists" canonicalizes to "\\?\C:\Exists" while "C:\DoesNotExit" is left as is.
  • A similar issue affects UNC network share paths. If the network share is present/reachable when a pattern is added you will get different patterns added to the scope than if the share is not present/reachable.
  • The path "C:" is different from "C:\" (the former refers to the current working directory of the C drive, while the second refers to the root directory of the C drive). There's effectively no way to pass "C:\" to add_directory because it unconditionally adds a backslash before the wildcards which leads to a pattern like "C:\\**" which doesn't match anything.
  • If you add a verbatim path (one that starts with "\\?\"), it won't match non-verbatim paths that refer to the same target.

amrbashir
amrbashir reviewed Sep 17, 2024
View reviewed changes
Copy link
Member

@amrbashir amrbashir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you and apologies for the late reply

crates/tauri/src/scope/fs.rs Outdated Show resolved Hide resolved
crates/tauri/src/scope/fs.rs Outdated Show resolved Hide resolved
crates/tauri/src/scope/fs.rs Outdated Show resolved Hide resolved
amrbashir
amrbashir approved these changes Sep 19, 2024
View reviewed changes
Copy link
Member

@amrbashir amrbashir left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. Unfortunately, you don't seem to have commit signing enabled and I can't merge the PR until
it is signed.

You need to setup commit signing, then you can sign past commit like this. git rebase --exec 'git commit --amend --no-edit -n -S' -i dev

alexmoon and others added 7 commits September 19, 2024 19:13
@alexmoon
Handle root paths that cannot be canonicalized
fa7cb57 
If the root path element of a pattern could not be canonicalized, `scope::fs::push_pattern` would enter an infinite loop. This is because `path.pop()` does nothing and returns `false` when called on a path with no parent, which can happen even when the path element iterator is non-empty. This could occur on Windows, for example, with a pattern of `\\\\?\\**`.

Fixes tauri-apps#10815
@alexmoon
Fix MSRV violation
2effec7
@alexmoon
Add test case
00cb6b9
@alexmoon
Check for trailing separator in escaped_pattern_with
220678d
@alexmoon
Use dunce::simplified in push_pattern to normalize patterns
0d889cf
@alexmoon
Canonicalize disk-relative paths on Windows when adding to a scope
6cc7570
@alexmoon
Fix clippy warning
7e5ef79
@alexmoon
Copy link
Contributor Author

Sorry about that. My Windows machine doesn't have signing set up.

@amrbashir
Copy link
Member

I have a few changes locally while testing this PR, that simplifies the code a bit and adds more documentation. Since I can't push to your branch, I will open a follow up PR. Thanks for your help and the test cases as well.

@amrbashir amrbashir merged commit 40a45b5 into tauri-apps:dev Sep 19, 2024
19 checks passed
@amrbashir amrbashir mentioned this pull request Sep 19, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FabianLars FabianLars FabianLars left review comments

@amrbashir amrbashir amrbashir approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[bug] Infinite loop in scope::fs::push_pattern
3 participants
@alexmoon @amrbashir @FabianLars