-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Add internal AppExecLink support to link APIs #58233
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
Changes from 4 commits
58f0b4e
69c3d78
6f756c8
f150fad
4237aad
ccfd2dd
4c425d1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System.IO.Enumeration; | ||
| using System.Linq; | ||
| using Xunit; | ||
|
|
||
| namespace System.IO.Tests | ||
| { | ||
| [PlatformSpecific(TestPlatforms.Windows)] | ||
| public class AppExecLinks : BaseSymbolicLinks | ||
| { | ||
| [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.HasUsableAppExecLinksDirectory))] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry if I skipped this - will this throw if HasUsableAppExecLinksDirectory return false or the test could be skipped always silently in CIs?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It should return false and cause the test to be skipped due to the absence of a WindowsApps dir. The property this attribute is pointing to, should not throw. If it throws, it's a bug and needs to be fixed.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My concern is that in the case the tests can be never really evaluated on CIs. |
||
| [InlineData(false)] | ||
| [InlineData(true)] | ||
| public void ResolveAppExecLinkTargets(bool returnFinalTarget) | ||
| { | ||
| string windowsAppsDir = Path.Join(Environment.GetEnvironmentVariable("LOCALAPPDATA"), "Microsoft", "WindowsApps"); | ||
| var appExecLinkPaths = new FileSystemEnumerable<string?>( | ||
| windowsAppsDir, | ||
| (ref FileSystemEntry entry) => entry.ToFullPath(), | ||
| new EnumerationOptions { RecurseSubdirectories = true }) | ||
| { | ||
| ShouldIncludePredicate = (ref FileSystemEntry entry) => | ||
| FileSystemName.MatchesWin32Expression("*.exe", entry.FileName) && | ||
| entry.Attributes.HasFlag(FileAttributes.ReparsePoint) | ||
| }; | ||
|
|
||
| foreach (string appExecLinkPath in appExecLinkPaths) | ||
| { | ||
| FileInfo linkInfo = new(appExecLinkPath); | ||
| Assert.Equal(0, linkInfo.Length); | ||
| Assert.True(linkInfo.Attributes.HasFlag(FileAttributes.ReparsePoint)); | ||
|
|
||
| string? linkTarget = linkInfo.LinkTarget; | ||
| Assert.NotNull(linkTarget); | ||
| Assert.NotEqual(appExecLinkPath, linkTarget); | ||
|
|
||
| FileSystemInfo? targetInfoFromFileInfo = linkInfo.ResolveLinkTarget(returnFinalTarget); | ||
| VerifyFileInfo(targetInfoFromFileInfo); | ||
|
|
||
| FileSystemInfo? targetInfoFromFile = File.ResolveLinkTarget(appExecLinkPath, returnFinalTarget); | ||
| VerifyFileInfo(targetInfoFromFile); | ||
| } | ||
|
|
||
| void VerifyFileInfo(FileSystemInfo? info) | ||
| { | ||
| Assert.True(info is FileInfo); | ||
| if (info.Exists) // The target may not exist, that's ok | ||
| { | ||
| Assert.True(((FileInfo)info).Length > 0); | ||
| Assert.False(info.Attributes.HasFlag(FileAttributes.ReparsePoint)); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -539,13 +539,59 @@ internal static void CreateSymbolicLink(string path, string pathToTarget, bool i | |
| Debug.Assert(!PathInternal.IsPartiallyQualified(targetPath)); | ||
| return targetPath.ToString(); | ||
| } | ||
| else if (rbSymlink.ReparseTag == Interop.Kernel32.IOReparseOptions.IO_REPARSE_TAG_APPEXECLINK) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there's a long list of IO_REPARSE_TAG_* .. how is this one special?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is one of the reparse tags proven to point to another file/folder and it is also one of the three tags covered by PowerShell: https://github.com/PowerShell/PowerShell/blob/008f4b057fdc1482d3200de0a61d23570b4d30e4/src/System.Management.Automation/namespaces/FileSystemProvider.cs#L8217-L8230 I am uncertain if all the reparse tags in the link you posted indicate that the reparse point contains a reference to another file/folder, but having these 3 tags (Symlinks, MountPoints and AppExecLinks) seems to me like a good start.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup fair enough, if Powershell has had this support for a while, that's good evidence those are the ones that matter.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It is unnormal reparse point :-) so we had to add it explicitly in PowerShell. In common, reparse point list is "open" and if new reparse points will be added in future perhaps we will have to add its explicitly too. I'd remind about OneDrive. .Net API should be tested for OneDrive paths/links too (at list manually).
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I manually tested enumeration of the OneDrive directory (ensuring there were cloud files) and the behavior was the same in 3.1, 5.0 and 6.0 (including the changes in this PR): I can read the file name and the file attributes, and the files are not getting unexpectedly downloaded - their cloud icon stays next to the files when viewing them in the File Explorer. @iSazonov by any chance, do you know any specific scenarios in which the OneDrive files get unexpectedly downloaded when manipulating them with .NET? I am not sure if you've mentioned this strange behavior in the past, or if it was someone else. It would be a good chance for me to verify it.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @carlossanlop These links are related OneDrive and could be interesting for you: Also for removing PowerShell/PowerShell#15571 Feel free to ask me if you need additional comments. |
||
| { | ||
| success = MemoryMarshal.TryRead(bufferSpan, out Interop.Kernel32.AppExecLinkReparseBuffer rbAppExecLink); | ||
| Debug.Assert(success); | ||
|
|
||
| // The target file is at index 2 | ||
| if (rbAppExecLink.StringCount >= 3) | ||
|
||
| { | ||
| int stringListOffset = sizeof(Interop.Kernel32.AppExecLinkReparseBuffer); | ||
| Span<char> stringList = MemoryMarshal.Cast<byte, char>(bufferSpan.Slice(stringListOffset)); | ||
|
|
||
| return GetAppExecLinkTarget(stringList); | ||
| } | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
| finally | ||
| { | ||
| ArrayPool<byte>.Shared.Return(buffer); | ||
| } | ||
|
|
||
| // Given an appexeclink reparse point string list, finds the | ||
| // start and end (null chars) of the 3rd string in the list. | ||
| static string? GetAppExecLinkTarget(ReadOnlySpan<char> stringList) | ||
| { | ||
| // Find the 2nd and 3rd null char | ||
| int start = -1; | ||
| int end = -1; | ||
| int count = 0; | ||
| for (int i = 0; i < stringList.Length; i++) | ||
| { | ||
| if (stringList[i] == '\0') | ||
| { | ||
| count++; | ||
| if (count == 2) | ||
| { | ||
| start = i + 1; // exclude null char | ||
carlossanlop marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
| else if (count == 3) | ||
| { | ||
| end = i; | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| if (start != -1 && end != -1) | ||
| { | ||
| return stringList.Slice(start, end - start).ToString(); | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
| } | ||
|
|
||
| private static unsafe string? GetFinalLinkTarget(string linkPath, bool isDirectory) | ||
|
|
@@ -555,9 +601,10 @@ internal static void CreateSymbolicLink(string path, string pathToTarget, bool i | |
|
|
||
| // The file or directory is not a reparse point. | ||
| if ((data.dwFileAttributes & (uint)FileAttributes.ReparsePoint) == 0 || | ||
| // Only symbolic links and mount points are supported at the moment. | ||
| // Only symbolic links, mount points (junctions) and app exec links are supported at the moment. | ||
| ((data.dwReserved0 & Interop.Kernel32.IOReparseOptions.IO_REPARSE_TAG_SYMLINK) == 0 && | ||
| (data.dwReserved0 & Interop.Kernel32.IOReparseOptions.IO_REPARSE_TAG_MOUNT_POINT) == 0)) | ||
| (data.dwReserved0 & Interop.Kernel32.IOReparseOptions.IO_REPARSE_TAG_MOUNT_POINT) == 0 && | ||
| (data.dwReserved0 & Interop.Kernel32.IOReparseOptions.IO_REPARSE_TAG_APPEXECLINK) == 0)) | ||
| { | ||
| return null; | ||
| } | ||
|
|
@@ -572,7 +619,7 @@ internal static void CreateSymbolicLink(string path, string pathToTarget, bool i | |
| // If the handle fails because it is unreachable, is because the link was broken. | ||
| // We need to fallback to manually traverse the links and return the target of the last resolved link. | ||
| int error = Marshal.GetLastWin32Error(); | ||
| if (IsPathUnreachableError(error)) | ||
| if (IsPathUnreachableError(error) || error == Interop.Errors.ERROR_CANT_ACCESS_FILE /* Possibly broken AppExecLink */) | ||
carlossanlop marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| { | ||
| return GetFinalLinkTargetSlow(linkPath); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not related the PR but I wonder why do such structs returned from P/Invokes would not be readonly structs?