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

Incorrect handling of NTFS Reparse points to volumes #75

Closed
arvidn opened this issue Oct 19, 2020 · 3 comments
Closed

Incorrect handling of NTFS Reparse points to volumes #75

arvidn opened this issue Oct 19, 2020 · 3 comments
Assignees
Labels
available on master Fix is done on master branch, issue closed on next release bug Something isn't working Windows Windows platform is affected
Milestone

Comments

@arvidn
Copy link

arvidn commented Oct 19, 2020

Describe the bug

exists() returns false when given a path to an NTFS reparse point that points to a volume.
Additionally, status() returns not_found for such path.

To Reproduce

In Disk Manager, format a new volume. Instead of assigning a drive-letter, assign a mount-point at an empty directory on an NTFS volume.

Call exists() and pass in the path to the mount point.

Observe it returning false, even though the volume exists and is mounted.

Expected behavior

exists() is expected to return true.

Additional context

The problem arises from resolving a "symlink", in status_ex. The symlink being an IO_REPARSE_TAG_MOUNT_POINT is handled here.

The symlink resolves to something of this form: "\??\Volume{<GUID>}". This path is correct. However, it's assigned into a path object (result = ). The path assign() member function will strip the \??\ prefix, which makes the path incorrect.

Recursing down into another call to status_ex() (here) will then fail saying the path doesn't exist (because it doesn't).

Removing the code that's stripping the prefix fixes the problem:

--- a/filesystem.hpp
+++ b/filesystem.hpp
@@ -1663,9 +1663,9 @@ GHC_INLINE void path::postprocess_path_with_format(path::impl_string_type& p, pa
                         p[0] = '\\';
                     }
                 }
-                else if (detail::startsWith(p, std::string("\\??\\"))) {
-                    p.erase(0, 4);
-                }
+//                else if (detail::startsWith(p, std::string("\\??\\"))) {
+//                    p.erase(0, 4);
+//                }
             }
             for (auto& c : p) {
                 if (c == '\\') {

However, that is likely introducing other issues. Perhaps the best solution would be to have a resolveLink return a plain native sting (instead of path) and also make status_ex() accept a plain string, to avoid round-tripping via path in this case.

@gulrak
Copy link
Owner

gulrak commented Oct 20, 2020

Indeed removing that from postprocess_path_with_format has other issues but I might go with the idea of a native string based status.

I'll need to do some testing and will look into it, thanks for reporting.

@gulrak gulrak self-assigned this Oct 20, 2020
@gulrak gulrak added bug Something isn't working Windows Windows platform is affected labels Oct 20, 2020
@gulrak gulrak added this to the v1.3.8 milestone Oct 24, 2020
@gulrak
Copy link
Owner

gulrak commented Nov 8, 2020

Actually I decided to not try to work around the namespaced path by changing the internal handling and stop filtering them, fixed an additional issue with long path handling. I checked there are no issues as far as I can think of and now it will only filter trivial path to keep faithful with the internal generic form where possible, but allow using \\?\Volume{a1b3c5e7f-0000-0000-0000-100000000000}\ types of path (and others like \\?\GLOBALROOT\Device\Harddisk0\Partition1\Windows\notepad.exe ones) without messing around with them. Keeps the changes minimal and is more stable.

gulrak added a commit that referenced this issue Nov 9, 2020
@gulrak gulrak added the available on master Fix is done on master branch, issue closed on next release label Nov 10, 2020
@gulrak
Copy link
Owner

gulrak commented Nov 15, 2020

Released with v1.3.8

@gulrak gulrak closed this as completed Nov 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
available on master Fix is done on master branch, issue closed on next release bug Something isn't working Windows Windows platform is affected
Projects
None yet
Development

No branches or pull requests

2 participants