[release/6.0] Fix setting timestamp on Windows on readonly files (#62922)
authorAdam Sitnik <adam.sitnik@gmail.com>
Fri, 7 Jan 2022 16:38:04 +0000 (17:38 +0100)
committerGitHub <noreply@github.com>
Fri, 7 Jan 2022 16:38:04 +0000 (09:38 -0700)
* Fix setting timestamp on Windows on readonly files (#62638)

# Conflicts:
# src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs

* make it compile

* fix build

Co-authored-by: Dan Moseley <danmose@microsoft.com>
src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FileOperations.cs
src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs
src/libraries/System.IO.FileSystem/tests/Directory/GetSetTimes.cs
src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs
src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs
src/libraries/System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs
src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs

index cc4896c..c43b4fd 100644 (file)
@@ -23,7 +23,8 @@ internal static partial class Interop
             internal const int FILE_FLAG_OVERLAPPED = 0x40000000;
 
             internal const int FILE_LIST_DIRECTORY = 0x0001;
-        }
 
+            internal const int FILE_WRITE_ATTRIBUTES = 0x100;
+        }
     }
 }
index a959e31..d551718 100644 (file)
@@ -20,7 +20,9 @@ namespace System.IO.Tests
         protected static bool LowTemporalResolution => PlatformDetection.IsBrowser || isHFS;
         protected static bool HighTemporalResolution => !LowTemporalResolution;
 
-        protected abstract T GetExistingItem();
+        protected abstract bool CanBeReadOnly { get; }
+
+        protected abstract T GetExistingItem(bool readOnly = false);
         protected abstract T GetMissingItem();
 
         protected abstract string GetItemPath(T item);
@@ -42,11 +44,8 @@ namespace System.IO.Tests
             public DateTimeKind Kind => Item3;
         }
 
-        [Fact]
-        public void SettingUpdatesProperties()
+        private void SettingUpdatesPropertiesCore(T item)
         {
-            T item = GetExistingItem();
-
             Assert.All(TimeFunctions(requiresRoundtripping: true), (function) =>
             {
                 // Checking that milliseconds are not dropped after setter.
@@ -71,6 +70,25 @@ namespace System.IO.Tests
         }
 
         [Fact]
+        public void SettingUpdatesProperties()
+        {
+            T item = GetExistingItem();
+            SettingUpdatesPropertiesCore(item);
+        }
+
+        [Fact]
+        public void SettingUpdatesPropertiesWhenReadOnly()
+        {
+            if (!CanBeReadOnly)
+            {
+                return; // directories can't be read only, so automatic pass
+            }
+
+            T item = GetExistingItem(readOnly: true);
+            SettingUpdatesPropertiesCore(item);
+        }
+
+        [Fact]
         public void CanGetAllTimesAfterCreation()
         {
             DateTime beforeTime = DateTime.UtcNow.AddSeconds(-3);
index 0a23d63..1fc00ba 100644 (file)
@@ -7,7 +7,9 @@ namespace System.IO.Tests
 {
     public class Directory_GetSetTimes : StaticGetSetTimes
     {
-        protected override string GetExistingItem() => Directory.CreateDirectory(GetTestFilePath()).FullName;
+        protected override bool CanBeReadOnly => false;
+
+        protected override string GetExistingItem(bool _) => Directory.CreateDirectory(GetTestFilePath()).FullName;
 
         public override IEnumerable<TimeFunction> TimeFunctions(bool requiresRoundtripping = false)
         {
index 2dad268..c45c616 100644 (file)
@@ -7,7 +7,9 @@ namespace System.IO.Tests
 {
     public class DirectoryInfo_GetSetTimes : InfoGetSetTimes<DirectoryInfo>
     {
-        protected override DirectoryInfo GetExistingItem() => Directory.CreateDirectory(GetTestFilePath());
+        protected override bool CanBeReadOnly => false;
+
+        protected override DirectoryInfo GetExistingItem(bool _) => Directory.CreateDirectory(GetTestFilePath());
 
         protected override DirectoryInfo GetMissingItem() => new DirectoryInfo(GetTestFilePath());
 
index 67a13bc..f27e79d 100644 (file)
@@ -11,14 +11,22 @@ namespace System.IO.Tests
 {
     public class File_GetSetTimes : StaticGetSetTimes
     {
+        protected override bool CanBeReadOnly => true;
+
         // OSX has the limitation of setting upto 2262-04-11T23:47:16 (long.Max) date.
         // 32bit Unix has time_t up to ~ 2038.
         private static bool SupportsLongMaxDateTime => PlatformDetection.IsWindows || (!PlatformDetection.Is32BitProcess && !PlatformDetection.IsOSXLike);
 
-        protected override string GetExistingItem()
+        protected override string GetExistingItem(bool readOnly = false)
         {
             string path = GetTestFilePath();
             File.Create(path).Dispose();
+
+            if (readOnly)
+            {
+                File.SetAttributes(path, FileAttributes.ReadOnly);
+            }
+
             return path;
         }
 
index d3b9764..1ca3061 100644 (file)
@@ -10,10 +10,18 @@ namespace System.IO.Tests
 {
     public class FileInfo_GetSetTimes : InfoGetSetTimes<FileInfo>
     {
-        protected override FileInfo GetExistingItem()
+        protected override bool CanBeReadOnly => true;
+
+        protected override FileInfo GetExistingItem(bool readOnly = false)
         {
             string path = GetTestFilePath();
             File.Create(path).Dispose();
+
+            if (readOnly)
+            {
+                File.SetAttributes(path, FileAttributes.ReadOnly);
+            }
+
             return new FileInfo(path);
         }
 
index a233347..b1ba363 100644 (file)
@@ -145,10 +145,9 @@ namespace System.IO
             }
         }
 
-        private static SafeFileHandle OpenHandle(string fullPath, bool asDirectory)
+        private static SafeFileHandle OpenHandleToWriteAttributes(string fullPath, bool asDirectory)
         {
-            string root = fullPath.Substring(0, PathInternal.GetRootLength(fullPath.AsSpan()));
-            if (root == fullPath && root[1] == Path.VolumeSeparatorChar)
+            if (fullPath.Length == PathInternal.GetRootLength(fullPath.AsSpan()) && fullPath[1] == Path.VolumeSeparatorChar)
             {
                 // intentionally not fullpath, most upstack public APIs expose this as path.
                 throw new ArgumentException(SR.Arg_PathIsVolume, "path");
@@ -156,7 +155,7 @@ namespace System.IO
 
             SafeFileHandle handle = Interop.Kernel32.CreateFile(
                 fullPath,
-                Interop.Kernel32.GenericOperations.GENERIC_WRITE,
+                Interop.Kernel32.FileOperations.FILE_WRITE_ATTRIBUTES,
                 FileShare.ReadWrite | FileShare.Delete,
                 FileMode.Open,
                 asDirectory ? Interop.Kernel32.FileOperations.FILE_FLAG_BACKUP_SEMANTICS : 0);
@@ -382,7 +381,7 @@ namespace System.IO
             long changeTime = -1,
             uint fileAttributes = 0)
         {
-            using (SafeFileHandle handle = OpenHandle(fullPath, asDirectory))
+            using (SafeFileHandle handle = OpenHandleToWriteAttributes(fullPath, asDirectory))
             {
                 var basicInfo = new Interop.Kernel32.FILE_BASIC_INFO()
                 {