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

System.IO.FileSystem.AccessControl can't set ACLs on files with long paths #91980

Closed
hach-que opened this issue Sep 13, 2023 · 3 comments · Fixed by #92460
Closed

System.IO.FileSystem.AccessControl can't set ACLs on files with long paths #91980

hach-que opened this issue Sep 13, 2023 · 3 comments · Fixed by #92460
Assignees
Labels
area-System.IO help wanted [up-for-grabs] Good issue for external contributors
Milestone

Comments

@hach-que
Copy link
Contributor

Description

When attempting to set ACLs on a file with a long path on Windows, SetAccessControl throws ArgumentException: Invalid name:

Failed to grant Everyone full control of 'C:\ProgramData\UET\SDKs\Android-android-33-33.0.1-3.10.2.4988404-25.1.8937393-jdk-11.0.19+7\Sdk\cmdline-tools\lib\external\com\google\guava\listenablefuture\9999.0-empty-to-avoid-conflict-with-guava\listenablefuture-9999.0-empty-to-avoid-conflict-with-guava.jar'. Exception was: System.ArgumentException: Invalid name. (Parameter 'name')    at System.Security.AccessControl.NativeObjectSecurity.Persist(String name, SafeHandle handle, AccessControlSections includeSections, Object exceptionContext)    at System.Security.AccessControl.FileSystemSecurity.Persist(String fullPath)

Reproduction Steps

var dacl = file.GetAccessControl(AccessControlSections.Access);
dacl.AddAccessRule(new FileSystemAccessRule(_everyoneWindows.Value, FileSystemRights.FullControl, AccessControlType.Allow));
file.SetAccessControl(dacl);

where file is a FileInfo pointing to a file with a long path.

Expected behavior

SetAccessControl should work with long paths on Windows.

Actual behavior

SetAccessControl throws ArgumentException: Invalid name.

Regression?

No response

Known Workarounds

I tried to workaround this with this code:

var dacl = new FileSecurity(@"\\?\" + file.FullName, AccessControlSections.Access);
dacl.AddAccessRule(new FileSystemAccessRule(_everyoneWindows.Value, FileSystemRights.FullControl, AccessControlType.Allow));
dacl.Persist(@"\\?\" + file.FullName);

however, Persist is internal/protected and not available to user code. You also can't inherit from FileSystemSecurity to access the protected method, as all constructors on that class are internal.

Configuration

.NET 7

Other information

No response

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Sep 13, 2023
@ghost
Copy link

ghost commented Sep 13, 2023

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

When attempting to set ACLs on a file with a long path on Windows, SetAccessControl throws ArgumentException: Invalid name:

Failed to grant Everyone full control of 'C:\ProgramData\UET\SDKs\Android-android-33-33.0.1-3.10.2.4988404-25.1.8937393-jdk-11.0.19+7\Sdk\cmdline-tools\lib\external\com\google\guava\listenablefuture\9999.0-empty-to-avoid-conflict-with-guava\listenablefuture-9999.0-empty-to-avoid-conflict-with-guava.jar'. Exception was: System.ArgumentException: Invalid name. (Parameter 'name')    at System.Security.AccessControl.NativeObjectSecurity.Persist(String name, SafeHandle handle, AccessControlSections includeSections, Object exceptionContext)    at System.Security.AccessControl.FileSystemSecurity.Persist(String fullPath)

Reproduction Steps

var dacl = file.GetAccessControl(AccessControlSections.Access);
dacl.AddAccessRule(new FileSystemAccessRule(_everyoneWindows.Value, FileSystemRights.FullControl, AccessControlType.Allow));
file.SetAccessControl(dacl);

where file is a FileInfo pointing to a file with a long path.

Expected behavior

SetAccessControl should work with long paths on Windows.

Actual behavior

SetAccessControl throws ArgumentException: Invalid name.

Regression?

No response

Known Workarounds

I tried to workaround this with this code:

var dacl = new FileSecurity(@"\\?\" + file.FullName, AccessControlSections.Access);
dacl.AddAccessRule(new FileSystemAccessRule(_everyoneWindows.Value, FileSystemRights.FullControl, AccessControlType.Allow));
dacl.Persist(@"\\?\" + file.FullName);

however, Persist is internal/protected and not available to user code. You also can't inherit from FileSystemSecurity to access the protected method, as all constructors on that class are internal.

Configuration

.NET 7

Other information

No response

Author: hach-que
Assignees: -
Labels:

area-System.IO

Milestone: -

@adamsitnik adamsitnik added help wanted [up-for-grabs] Good issue for external contributors and removed untriaged New issue has not been triaged by the area owner labels Sep 13, 2023
@adamsitnik adamsitnik added this to the Future milestone Sep 13, 2023
@adamsitnik
Copy link
Member

adamsitnik commented Sep 13, 2023

Triage: I've marked this issue as help wanted. Whoever wants to work on it should attach a debugger and see which sys-call is being used in that scenario:

I suspect it's the one that takes the string name. In such case we need to ensure that we call the sys-call in the right way, if we do and it's impossible to get it working we should most likely open the file on our own via File.OpenHandle and use the sys-call that accepts a file handle.

@karakasa
Copy link
Contributor

karakasa commented Sep 21, 2023

the call hierarchy is:

  • FileSystemAclExtensions.SetAccessControl(this FileInfo)
  • FileSystemSecurity.Persist(string name)
  • NativeObjectSecurity.Persist (handle is null)
  • Win32.SetSecurityInfo (handle is null)
  • Interop.Advapi32.SetSecurityInfoByName when handle is null

It seems the file handle is never created, if the security object is constructed via name (file location).

I think we may add System.IO.PathInternal.EnsureExtendedPrefixIfNeeded to FileSystemSecurity.Persist(string) just as it does in the constructor.

Another workaround would be FileSystemAclExtensions.SetAccessControl(this FileStream fileStream, FileSecurity fileSecurity) because it calls FileSystemSecurity.Persist(SafeFileHandle)

I also noticed that the Security tab is missing for long-named files in explorer... 🤔

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Sep 22, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Sep 22, 2023
@adamsitnik adamsitnik modified the milestones: Future, 9.0.0 Sep 22, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Oct 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.IO help wanted [up-for-grabs] Good issue for external contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants