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

Ensure FileStatus and FileSystemEntry Hidden and ReadOnly attributes are retrieved the same way #40641

Merged
merged 4 commits into from
Aug 14, 2020
Merged

Ensure FileStatus and FileSystemEntry Hidden and ReadOnly attributes are retrieved the same way #40641

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
aff0182
Ensure FileStatus and FileSystemEntry IsHidden attribute is retrieved…
carlossanlop Aug 11, 2020
cfb8175
Add missing check in public property FileSystemEntry.IsHidden. Addres…
carlossanlop Aug 13, 2020
ba00f54
Add tests for Hidden and ReadOnly attribute check for each platform. …
carlossanlop Aug 13, 2020
a39bf8a
Use _initialAttributes instead of Attributes for IsHidden. Add commen…
carlossanlop Aug 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 20 additions & 18 deletions src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,36 +48,33 @@ namespace System.IO.Enumeration
// directory.
else if ((directoryEntry.InodeType == Interop.Sys.NodeType.DT_LNK
|| directoryEntry.InodeType == Interop.Sys.NodeType.DT_UNKNOWN)
&& Interop.Sys.Stat(entry.FullPath, out Interop.Sys.FileStatus targetStatus) >= 0)
&& Interop.Sys.Stat(entry.FullPath, out Interop.Sys.FileStatus statInfo) >= 0)
{
// Symlink or unknown: Stat to it to see if we can resolve it to a directory.
isDirectory = (targetStatus.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR;
isDirectory = FileStatus.IsDirectory(statInfo);
}
// Same idea as the directory check, just repeated for (and tweaked due to the
// nature of) symlinks.

// Same idea as the directory check, just repeated for (and tweaked due to the nature of) symlinks.
int resultLStat = Interop.Sys.LStat(entry.FullPath, out Interop.Sys.FileStatus lstatInfo);
carlossanlop marked this conversation as resolved.
Show resolved Hide resolved

bool isReadOnly = resultLStat >= 0 && FileStatus.IsReadOnly(lstatInfo);

if (directoryEntry.InodeType == Interop.Sys.NodeType.DT_LNK)
{
isSymlink = true;
}
else if ((directoryEntry.InodeType == Interop.Sys.NodeType.DT_UNKNOWN)
&& (Interop.Sys.LStat(entry.FullPath, out Interop.Sys.FileStatus linkTargetStatus) >= 0))
else if (resultLStat >= 0 && directoryEntry.InodeType == Interop.Sys.NodeType.DT_UNKNOWN)
{
isSymlink = (linkTargetStatus.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFLNK;
isSymlink = FileStatus.IsSymLink(lstatInfo);
}

// If the filename starts with a period or has UF_HIDDEN flag set, it's hidden.
bool isHidden = directoryEntry.Name[0] == '.' || (resultLStat >= 0 && FileStatus.IsHidden(lstatInfo));

entry._status = default;
FileStatus.Initialize(ref entry._status, isDirectory);

FileAttributes attributes = default;
if (isSymlink)
attributes |= FileAttributes.ReparsePoint;
if (isDirectory)
attributes |= FileAttributes.Directory;
if (directoryEntry.Name[0] == '.')
attributes |= FileAttributes.Hidden;

if (attributes == default)
attributes = FileAttributes.Normal;
FileAttributes attributes = FileStatus.GetAttributes(isReadOnly, isSymlink, isDirectory, isHidden);

entry._initialAttributes = attributes;
return attributes;
Expand Down Expand Up @@ -143,7 +140,12 @@ public FileAttributes Attributes
public DateTimeOffset LastAccessTimeUtc => _status.GetLastAccessTime(FullPath, continueOnError: true);
public DateTimeOffset LastWriteTimeUtc => _status.GetLastWriteTime(FullPath, continueOnError: true);
public bool IsDirectory => _status.InitiallyDirectory;
public bool IsHidden => _directoryEntry.Name[0] == '.';
/// <summary>
/// Returns <see langword="true"/> if the file is hidden; <see langword="false" /> otherwise.
/// In Linux and OSX, a file can be marked hidden if the filename is prepended with a dot.
/// In Windows and OSX, a file can be hidden if the special hidden attribute is set. For example, via the <see cref="FileSystemInfo.Attributes" /> enum flag.
/// </summary>
public bool IsHidden => _directoryEntry.Name[0] == '.' || (_initialAttributes & FileAttributes.Hidden) != 0;

public FileSystemInfo ToFileSystemInfo()
{
Expand Down
64 changes: 44 additions & 20 deletions src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,19 +39,23 @@ internal struct FileStatus
internal bool IsReadOnly(ReadOnlySpan<char> path, bool continueOnError = false)
{
EnsureStatInitialized(path, continueOnError);
return IsReadOnly(_fileStatus);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we re-read if we have Refresh() for this.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure I understand the question.

The functionality of this code has not been changed. I merely moved the contents of the old IsReadOnly method into its own internal helper static method that can be accessed by both FileSystemEntry and FileStatus, to avoid repeating code.

I don't think we should do a Refresh call here, that's what EnsureStatInitialized is for - it will only perform a Refresh if we haven't called it yet.

}

internal static bool IsReadOnly(Interop.Sys.FileStatus fileStatus)
Copy link
Contributor

Choose a reason for hiding this comment

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

Private?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, it needs to be accessed by FileSystemEntry too, which is in the same assembly.

{
#if TARGET_BROWSER
const Interop.Sys.Permissions readBit = Interop.Sys.Permissions.S_IRUSR;
const Interop.Sys.Permissions writeBit = Interop.Sys.Permissions.S_IWUSR;
#else
Interop.Sys.Permissions readBit, writeBit;

if (_fileStatus.Uid == Interop.Sys.GetEUid())
if (fileStatus.Uid == Interop.Sys.GetEUid())
{
// User effectively owns the file
readBit = Interop.Sys.Permissions.S_IRUSR;
writeBit = Interop.Sys.Permissions.S_IWUSR;
}
else if (_fileStatus.Gid == Interop.Sys.GetEGid())
else if (fileStatus.Gid == Interop.Sys.GetEGid())
{
// User belongs to a group that effectively owns the file
readBit = Interop.Sys.Permissions.S_IRGRP;
Expand All @@ -65,37 +69,57 @@ internal bool IsReadOnly(ReadOnlySpan<char> path, bool continueOnError = false)
}
#endif

return ((_fileStatus.Mode & (int)readBit) != 0 && // has read permission
(_fileStatus.Mode & (int)writeBit) == 0); // but not write permission
return (fileStatus.Mode & (int)readBit) != 0 && // has read permission
(fileStatus.Mode & (int)writeBit) == 0; // but not write permission
}

public FileAttributes GetAttributes(ReadOnlySpan<char> path, ReadOnlySpan<char> fileName)
internal static bool IsDirectory(Interop.Sys.FileStatus fileStatus)
{
// IMPORTANT: Attribute logic must match the logic in FileSystemEntry
return (fileStatus.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR;
Copy link
Contributor

Choose a reason for hiding this comment

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

It is better move p/invokes from FileSystemEntry.Initialize() in the place and exclude re-reading too.
The same for all attributes.

Copy link
Member Author

Choose a reason for hiding this comment

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

I talked to @Jozkee about this, we can get them moved to a separate helper class, but I would like to do that in a future PR.

}

EnsureStatInitialized(path);
internal static bool IsHidden(Interop.Sys.FileStatus fileStatus)
{
return (fileStatus.UserFlags & (uint)Interop.Sys.UserFlags.UF_HIDDEN) == (uint)Interop.Sys.UserFlags.UF_HIDDEN;
}

if (!_exists)
return (FileAttributes)(-1);
internal static bool IsSymLink(Interop.Sys.FileStatus fileStatus)
{
return (fileStatus.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFLNK;
}

internal static FileAttributes GetAttributes(bool isReadOnly, bool isSymlink, bool isDirectory, bool isHidden)
{
FileAttributes attributes = default;

if (IsReadOnly(path))
if (isReadOnly)
attributes |= FileAttributes.ReadOnly;

if ((_fileStatus.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFLNK)
if (isSymlink)
attributes |= FileAttributes.ReparsePoint;

if (_isDirectory)
if (isDirectory)
attributes |= FileAttributes.Directory;

// If the filename starts with a period or has UF_HIDDEN flag set, it's hidden.
if (fileName.Length > 0 && (fileName[0] == '.' || (_fileStatus.UserFlags & (uint)Interop.Sys.UserFlags.UF_HIDDEN) == (uint)Interop.Sys.UserFlags.UF_HIDDEN))
if (isHidden)
attributes |= FileAttributes.Hidden;

return attributes != default ? attributes : FileAttributes.Normal;
}

public FileAttributes GetAttributes(ReadOnlySpan<char> path, ReadOnlySpan<char> fileName)
{
// IMPORTANT: Attribute logic must match the logic in FileSystemEntry

EnsureStatInitialized(path);

if (!_exists)
return (FileAttributes)(-1);

return GetAttributes(
IsReadOnly(path),
IsSymLink(_fileStatus),
_isDirectory,
(fileName.Length > 0 && fileName[0] == '.') || IsHidden(_fileStatus));
}

public void SetAttributes(string path, FileAttributes attributes)
carlossanlop marked this conversation as resolved.
Show resolved Hide resolved
{
// Validate that only flags from the attribute are being provided. This is an
Expand Down Expand Up @@ -300,13 +324,13 @@ public void Refresh(ReadOnlySpan<char> path)
_exists = true;

// IMPORTANT: Is directory logic must match the logic in FileSystemEntry
_isDirectory = (_fileStatus.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR;
_isDirectory = IsDirectory(_fileStatus);

// If we're a symlink, attempt to check the target to see if it is a directory
if ((_fileStatus.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFLNK &&
Interop.Sys.Stat(path, out Interop.Sys.FileStatus targetStatus) >= 0)
{
_isDirectory = (targetStatus.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR;
_isDirectory = IsDirectory(targetStatus);
}

_fileStatusInitialized = 0;
Expand Down
60 changes: 54 additions & 6 deletions src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,18 +113,42 @@ public void DirectoryAttributesAreExpected()
}

[Fact]
public void IsHiddenAttribute()
[PlatformSpecific(TestPlatforms.Windows | TestPlatforms.OSX)]
public void IsHiddenAttribute_Windows_OSX()
{
// Put a period in front to make it hidden on Unix
IsHiddenAttributeInternal(useDotPrefix: false, useHiddenFlag: true);
Copy link
Member

Choose a reason for hiding this comment

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

Can you also add an OSX test that uses the dot prefix?

Copy link
Member Author

Choose a reason for hiding this comment

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

The test IsHiddenAttribute_Unix in line 127 runs in OSX too (TestPlatforms.AnyUnix is true in OSX).


}


[Fact]
[PlatformSpecific(TestPlatforms.AnyUnix)]
public void IsHiddenAttribute_Unix()
{
// Windows and MacOS hide a file by setting the hidden attribute
IsHiddenAttributeInternal(useDotPrefix: true, useHiddenFlag: false);
}

private void IsHiddenAttributeInternal(bool useDotPrefix, bool useHiddenFlag)
{
string prefix = useDotPrefix ? "." : "";

DirectoryInfo testDirectory = Directory.CreateDirectory(GetTestFilePath());
FileInfo fileOne = new FileInfo(Path.Combine(testDirectory.FullName, GetTestFileName()));

// Put a period in front to make it hidden on Unix
FileInfo fileTwo = new FileInfo(Path.Combine(testDirectory.FullName, "." + GetTestFileName()));
FileInfo fileOne = new FileInfo(Path.Combine(testDirectory.FullName, GetTestFileName()));
FileInfo fileTwo = new FileInfo(Path.Combine(testDirectory.FullName, prefix + GetTestFileName()));

fileOne.Create().Dispose();
fileTwo.Create().Dispose();
if (PlatformDetection.IsWindows)
fileTwo.Attributes = fileTwo.Attributes | FileAttributes.Hidden;

if (useHiddenFlag)
{
fileTwo.Attributes |= FileAttributes.Hidden;
}

FileInfo fileCheck = new FileInfo(fileTwo.FullName);
Assert.Equal(fileTwo.Attributes, fileCheck.Attributes);

IEnumerable<string> enumerable = new FileSystemEnumerable<string>(
testDirectory.FullName,
Expand All @@ -136,5 +160,29 @@ public void IsHiddenAttribute()

Assert.Equal(new string[] { fileTwo.FullName }, enumerable);
}

[Fact]
public void IsReadOnlyAttribute()
{
DirectoryInfo testDirectory = Directory.CreateDirectory(GetTestFilePath());

FileInfo fileOne = new FileInfo(Path.Combine(testDirectory.FullName, GetTestFileName()));
FileInfo fileTwo = new FileInfo(Path.Combine(testDirectory.FullName, GetTestFileName()));

fileOne.Create().Dispose();
fileTwo.Create().Dispose();

fileTwo.Attributes |= FileAttributes.ReadOnly;

IEnumerable<string> enumerable = new FileSystemEnumerable<string>(
testDirectory.FullName,
(ref FileSystemEntry entry) => entry.ToFullPath(),
new EnumerationOptions() { AttributesToSkip = 0 })
{
ShouldIncludePredicate = (ref FileSystemEntry entry) => (entry.Attributes & FileAttributes.ReadOnly) != 0
};

Assert.Equal(new string[] { fileTwo.FullName }, enumerable);
}
}
}
35 changes: 26 additions & 9 deletions src/libraries/System.IO.FileSystem/tests/Enumeration/SkipAttributeTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,25 +21,42 @@ protected virtual string[] GetPaths(string directory, EnumerationOptions options
}

[Fact]
public void SkippingHiddenFiles()
[PlatformSpecific(TestPlatforms.Windows | TestPlatforms.OSX)]
public void SkippingHiddenFiles_Windows_OSX()
{
SkippingHiddenFilesInternal(useDotPrefix: false, useHiddenFlag: true);
}

[Fact]
[PlatformSpecific(TestPlatforms.AnyUnix)]
public void SkippingHiddenFiles_Unix()
{
SkippingHiddenFilesInternal(useDotPrefix: true, useHiddenFlag: false);
}

private void SkippingHiddenFilesInternal(bool useDotPrefix, bool useHiddenFlag)
{
DirectoryInfo testDirectory = Directory.CreateDirectory(GetTestFilePath());
DirectoryInfo testSubdirectory = Directory.CreateDirectory(Path.Combine(testDirectory.FullName, GetTestFileName()));
FileInfo fileOne = new FileInfo(Path.Combine(testDirectory.FullName, GetTestFileName()));

// Put a period in front to make it hidden on Unix
FileInfo fileTwo = new FileInfo(Path.Combine(testDirectory.FullName, "." + GetTestFileName()));
FileInfo fileOne = new FileInfo(Path.Combine(testDirectory.FullName, GetTestFileName()));
Copy link
Member Author

@carlossanlop carlossanlop Aug 11, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

No changes in this test, just organized some lines for easier understanding.
It also helps to show that this test is affected by my PR.

EDIT: I actually ended up making changes in this file, to split the tests into each platform, since the IsHidden property is not supported in Linux.

FileInfo fileThree = new FileInfo(Path.Combine(testSubdirectory.FullName, GetTestFileName()));
FileInfo fileFour = new FileInfo(Path.Combine(testSubdirectory.FullName, "." + GetTestFileName()));

// Put a period in front of files two and four to make them hidden on Unix
string prefix = useDotPrefix ? "." : "";
FileInfo fileTwo = new FileInfo(Path.Combine(testDirectory.FullName, prefix + GetTestFileName()));
FileInfo fileFour = new FileInfo(Path.Combine(testSubdirectory.FullName, prefix + GetTestFileName()));

fileOne.Create().Dispose();
fileTwo.Create().Dispose();
if (PlatformDetection.IsWindows)
fileTwo.Attributes = fileTwo.Attributes | FileAttributes.Hidden;
fileThree.Create().Dispose();
fileFour.Create().Dispose();
if (PlatformDetection.IsWindows)
fileFour.Attributes = fileTwo.Attributes | FileAttributes.Hidden;

if (useHiddenFlag)
{
fileTwo.Attributes |= FileAttributes.Hidden;
fileFour.Attributes |= FileAttributes.Hidden;
}

// Default EnumerationOptions is to skip hidden
string[] paths = GetPaths(testDirectory.FullName, new EnumerationOptions());
Expand Down