From 2302d48348994fd1bcfc52bdab4eceb324552fc2 Mon Sep 17 00:00:00 2001 From: Jeremy Kuhne Date: Mon, 19 Feb 2018 21:01:21 -0800 Subject: [PATCH] Handle errors getting state in Unix (dotnet/corefx#27239) * Handle errors getting state in Unix Throwing errors while examining extended state while enumerating isn't consistent with Windows behavior. Windows never throws past getting directory entry data as all state is already available. Ensure entry attribute state is consistent with initial construction. * Win 7 CI machines are also setting NotContentIndexed. Commit migrated from https://github.com/dotnet/corefx/commit/597f44c5056badfb47f45731480454d698849800 --- .../System/IO/Enumeration/FileSystemEntry.Unix.cs | 23 +++-- .../IO/Enumeration/FileSystemEnumerator.Unix.cs | 22 ++-- .../src/System/IO/FileStatus.Unix.cs | 62 +++++------ .../tests/Enumeration/AttributeTests.netcoreapp.cs | 115 +++++++++++++++++++++ .../tests/System.IO.FileSystem.Tests.csproj | 1 + 5 files changed, 178 insertions(+), 45 deletions(-) create mode 100644 src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.netcoreapp.cs diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs b/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs index ccf6f29..f533937 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs @@ -16,6 +16,7 @@ namespace System.IO.Enumeration private ReadOnlySpan _fullPath; private ReadOnlySpan _fileName; private fixed char _fileNameBuffer[FileNameBufferSize]; + private FileAttributes _initialAttributes; internal static FileAttributes Initialize( ref FileSystemEntry entry, @@ -51,7 +52,7 @@ namespace System.IO.Enumeration entry._status = default; FileStatus.Initialize(ref entry._status, isDirectory); - FileAttributes attributes = FileAttributes.Normal; + FileAttributes attributes = default; if (directoryEntry.InodeType == Interop.Sys.NodeType.DT_LNK) attributes |= FileAttributes.ReparsePoint; if (isDirectory) @@ -59,6 +60,10 @@ namespace System.IO.Enumeration if (directoryEntry.Name[0] == '.') attributes |= FileAttributes.Hidden; + if (attributes == default) + attributes = FileAttributes.Normal; + + entry._initialAttributes = attributes; return attributes; } @@ -112,11 +117,17 @@ namespace System.IO.Enumeration /// public ReadOnlySpan OriginalRootDirectory { get; private set; } - public FileAttributes Attributes => _status.GetAttributes(FullPath, FileName); - public long Length => _status.GetLength(FullPath); - public DateTimeOffset CreationTimeUtc => _status.GetCreationTime(FullPath); - public DateTimeOffset LastAccessTimeUtc => _status.GetLastAccessTime(FullPath); - public DateTimeOffset LastWriteTimeUtc => _status.GetLastWriteTime(FullPath); + // Windows never fails getting attributes, length, or time as that information comes back + // with the native enumeration struct. As such we must not throw here. + + public FileAttributes Attributes + // It would be hard to rationalize if the attributes change after our initial find. + => _initialAttributes | (_status.IsReadOnly(FullPath, continueOnError: true) ? FileAttributes.ReadOnly : 0); + + public long Length => _status.GetLength(FullPath, continueOnError: true); + public DateTimeOffset CreationTimeUtc => _status.GetCreationTime(FullPath, continueOnError: true); + public DateTimeOffset LastAccessTimeUtc => _status.GetLastAccessTime(FullPath, continueOnError: true); + public DateTimeOffset LastWriteTimeUtc => _status.GetLastWriteTime(FullPath, continueOnError: true); public bool IsDirectory => _status.InitiallyDirectory; public FileSystemInfo ToFileSystemInfo() diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEnumerator.Unix.cs b/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEnumerator.Unix.cs index 4ec505f..cfc3438 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEnumerator.Unix.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEnumerator.Unix.cs @@ -66,14 +66,20 @@ namespace System.IO.Enumeration } } + private bool InternalContinueOnError(int error) + => (_options.IgnoreInaccessible && IsAccessError(error)) || ContinueOnError(error); + + private static bool IsAccessError(int error) + => error == (int)Interop.Error.EACCES || error == (int)Interop.Error.EBADF + || error == (int)Interop.Error.EPERM; + private IntPtr CreateDirectoryHandle(string path) { IntPtr handle = Interop.Sys.OpenDir(path); if (handle == IntPtr.Zero) { Interop.ErrorInfo info = Interop.Sys.GetLastErrorInfo(); - if ((_options.IgnoreInaccessible && IsAccessError(info.RawErrno)) - || ContinueOnError(info.RawErrno)) + if (InternalContinueOnError(info.RawErrno)) { return IntPtr.Zero; } @@ -107,7 +113,8 @@ namespace System.IO.Enumeration if (_lastEntryFound) return false; - FileAttributes attributes = FileSystemEntry.Initialize(ref entry, _entry, _currentPath, _rootDirectory, _originalRootDirectory, new Span(_pathBuffer)); + FileAttributes attributes = FileSystemEntry.Initialize( + ref entry, _entry, _currentPath, _rootDirectory, _originalRootDirectory, new Span(_pathBuffer)); bool isDirectory = (attributes & FileAttributes.Directory) != 0; bool isSpecialDirectory = false; @@ -131,7 +138,7 @@ namespace System.IO.Enumeration attributes = entry.Attributes; } - if (attributes != (FileAttributes)(-1) && (_options.AttributesToSkip & attributes) != 0) + if ((_options.AttributesToSkip & attributes) != 0) { continue; } @@ -172,8 +179,7 @@ namespace System.IO.Enumeration break; default: // Error - if ((_options.IgnoreInaccessible && IsAccessError(result)) - || ContinueOnError(result)) + if (InternalContinueOnError(result)) { DirectoryFinished(); break; @@ -191,10 +197,6 @@ namespace System.IO.Enumeration _directoryHandle = CreateDirectoryHandle(_currentPath); } - private static bool IsAccessError(int error) - => error == (int)Interop.Error.EACCES || error == (int)Interop.Error.EBADF - || error == (int)Interop.Error.EPERM; - private void InternalDispose(bool disposing) { // It is possible to fail to allocate the lock, but the finalizer will still run diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs index 387cc32..07a48d5 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs @@ -35,17 +35,9 @@ namespace System.IO internal void Invalidate() => _fileStatusInitialized = -1; - public FileAttributes GetAttributes(ReadOnlySpan path, ReadOnlySpan fileName) + internal bool IsReadOnly(ReadOnlySpan path, bool continueOnError = false) { - // IMPORTANT: Attribute logic must match the logic in FileSystemEntry - - EnsureStatInitialized(path); - - if (!_exists) - return (FileAttributes)(-1); - - FileAttributes attrs = default; - + EnsureStatInitialized(path, continueOnError); Interop.Sys.Permissions readBit, writeBit; if (_fileStatus.Uid == Interop.Sys.GetEUid()) { @@ -66,23 +58,35 @@ namespace System.IO writeBit = Interop.Sys.Permissions.S_IWOTH; } - if ((_fileStatus.Mode & (int)readBit) != 0 && // has read permission - (_fileStatus.Mode & (int)writeBit) == 0) // but not write permission - { - attrs |= FileAttributes.ReadOnly; - } + return ((_fileStatus.Mode & (int)readBit) != 0 && // has read permission + (_fileStatus.Mode & (int)writeBit) == 0); // but not write permission + } + + public FileAttributes GetAttributes(ReadOnlySpan path, ReadOnlySpan fileName) + { + // IMPORTANT: Attribute logic must match the logic in FileSystemEntry + + EnsureStatInitialized(path); + + if (!_exists) + return (FileAttributes)(-1); + + FileAttributes attributes = default; + + if (IsReadOnly(path)) + attributes |= FileAttributes.ReadOnly; if ((_fileStatus.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFLNK) - attrs |= FileAttributes.ReparsePoint; + attributes |= FileAttributes.ReparsePoint; if (_isDirectory) - attrs |= FileAttributes.Directory; + attributes |= FileAttributes.Directory; // If the filename starts with a period, it's hidden. if (fileName.Length > 0 && fileName[0] == '.') - attrs |= FileAttributes.Hidden; + attributes |= FileAttributes.Hidden; - return attrs != default ? attrs : FileAttributes.Normal; + return attributes != default ? attributes : FileAttributes.Normal; } public void SetAttributes(string path, FileAttributes attributes) @@ -138,9 +142,9 @@ namespace System.IO return _exists && InitiallyDirectory == _isDirectory; } - internal DateTimeOffset GetCreationTime(ReadOnlySpan path) + internal DateTimeOffset GetCreationTime(ReadOnlySpan path, bool continueOnError = false) { - EnsureStatInitialized(path); + EnsureStatInitialized(path, continueOnError); if (!_exists) return DateTimeOffset.FromFileTime(0); @@ -163,9 +167,9 @@ namespace System.IO SetLastAccessTime(path, time); } - internal DateTimeOffset GetLastAccessTime(ReadOnlySpan path) + internal DateTimeOffset GetLastAccessTime(ReadOnlySpan path, bool continueOnError = false) { - EnsureStatInitialized(path); + EnsureStatInitialized(path, continueOnError); if (!_exists) return DateTimeOffset.FromFileTime(0); return UnixTimeToDateTimeOffset(_fileStatus.ATime, _fileStatus.ATimeNsec); @@ -174,9 +178,9 @@ namespace System.IO internal void SetLastAccessTime(string path, DateTimeOffset time) => SetAccessWriteTimes(path, time.ToUnixTimeSeconds(), null); - internal DateTimeOffset GetLastWriteTime(ReadOnlySpan path) + internal DateTimeOffset GetLastWriteTime(ReadOnlySpan path, bool continueOnError = false) { - EnsureStatInitialized(path); + EnsureStatInitialized(path, continueOnError); if (!_exists) return DateTimeOffset.FromFileTime(0); return UnixTimeToDateTimeOffset(_fileStatus.MTime, _fileStatus.MTimeNsec); @@ -203,9 +207,9 @@ namespace System.IO _fileStatusInitialized = -1; } - internal long GetLength(ReadOnlySpan path) + internal long GetLength(ReadOnlySpan path, bool continueOnError = false) { - EnsureStatInitialized(path); + EnsureStatInitialized(path, continueOnError); return _fileStatus.Size; } @@ -256,14 +260,14 @@ namespace System.IO _fileStatusInitialized = 0; } - internal void EnsureStatInitialized(ReadOnlySpan path) + internal void EnsureStatInitialized(ReadOnlySpan path, bool continueOnError = false) { if (_fileStatusInitialized == -1) { Refresh(path); } - if (_fileStatusInitialized != 0) + if (_fileStatusInitialized != 0 && !continueOnError) { int errno = _fileStatusInitialized; _fileStatusInitialized = -1; diff --git a/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.netcoreapp.cs b/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.netcoreapp.cs new file mode 100644 index 0000000..d357837 --- /dev/null +++ b/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.netcoreapp.cs @@ -0,0 +1,115 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.IO.Enumeration; +using Xunit; + +namespace System.IO.Tests.Enumeration +{ + public class AttributeTests : FileSystemTest + { + private class DefaultFileAttributes : FileSystemEnumerator + { + public DefaultFileAttributes(string directory, EnumerationOptions options) + : base(directory, options) + { + } + + protected override bool ContinueOnError(int error) + { + Assert.False(true, $"Should not have errored {error}"); + return false; + } + + protected override bool ShouldIncludeEntry(ref FileSystemEntry entry) + => !entry.IsDirectory; + + protected override string TransformEntry(ref FileSystemEntry entry) + { + string path = entry.ToFullPath(); + File.Delete(path); + + // Attributes require a stat call on Unix- ensure that we have the right attributes + // even if the returned file is deleted. + Assert.Equal(FileAttributes.Normal, entry.Attributes); + Assert.Equal(path, entry.ToFullPath()); + return new string(entry.FileName); + } + } + + [Fact] + public void FileAttributesAreExpected() + { + DirectoryInfo testDirectory = Directory.CreateDirectory(GetTestFilePath()); + FileInfo fileOne = new FileInfo(Path.Combine(testDirectory.FullName, GetTestFileName())); + + fileOne.Create().Dispose(); + + if (PlatformDetection.IsWindows) + { + // Archive should always be set on a new file. Clear it and other expected flags to + // see that we get "Normal" as the default when enumerating. + + Assert.True((fileOne.Attributes & FileAttributes.Archive) != 0); + fileOne.Attributes &= ~(FileAttributes.Archive | FileAttributes.NotContentIndexed); + } + + using (var enumerator = new DefaultFileAttributes(testDirectory.FullName, new EnumerationOptions())) + { + Assert.True(enumerator.MoveNext()); + Assert.Equal(fileOne.Name, enumerator.Current); + Assert.False(enumerator.MoveNext()); + } + } + + private class DefaultDirectoryAttributes : FileSystemEnumerator + { + public DefaultDirectoryAttributes(string directory, EnumerationOptions options) + : base(directory, options) + { + } + + protected override bool ShouldIncludeEntry(ref FileSystemEntry entry) + => entry.IsDirectory; + + protected override bool ContinueOnError(int error) + { + Assert.False(true, $"Should not have errored {error}"); + return false; + } + + protected override string TransformEntry(ref FileSystemEntry entry) + { + string path = entry.ToFullPath(); + Directory.Delete(path); + + // Attributes require a stat call on Unix- ensure that we have the right attributes + // even if the returned directory is deleted. + Assert.Equal(FileAttributes.Directory, entry.Attributes); + Assert.Equal(path, entry.ToFullPath()); + return new string(entry.FileName); + } + } + + [Fact] + public void DirectoryAttributesAreExpected() + { + DirectoryInfo testDirectory = Directory.CreateDirectory(GetTestFilePath()); + DirectoryInfo subDirectory = Directory.CreateDirectory(Path.Combine(testDirectory.FullName, GetTestFileName())); + + if (PlatformDetection.IsWindows) + { + // Clear possible extra flags to see that we get Directory + subDirectory.Attributes &= ~FileAttributes.NotContentIndexed; + } + + using (var enumerator = new DefaultDirectoryAttributes(testDirectory.FullName, new EnumerationOptions())) + { + Assert.True(enumerator.MoveNext()); + Assert.Equal(subDirectory.Name, enumerator.Current); + Assert.False(enumerator.MoveNext()); + } + } + } +} diff --git a/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj b/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj index 402489b..672d564 100644 --- a/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj +++ b/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj @@ -63,6 +63,7 @@ + -- 2.7.4