From 2c8b8281878ad30c1d1340f3f26d749ca3ac57ad Mon Sep 17 00:00:00 2001 From: Jimmy Campbell Date: Thu, 4 May 2017 10:43:20 -0700 Subject: [PATCH] Fixed race condition between calculating size of source directory and performing a move operation with the files/move endpoint. Tests added. --- .../FileInfo.cs | 7 +- .../Copy/MoveHelper.cs | 14 +- .../Copy/MoveOperation.cs | 40 +++-- .../Startup.cs | 4 + .../Files.cs | 161 ++++++++++++++++++ 5 files changed, 197 insertions(+), 29 deletions(-) diff --git a/src/Microsoft.IIS.Administration.Files.Core/FileInfo.cs b/src/Microsoft.IIS.Administration.Files.Core/FileInfo.cs index 4189548..6c65530 100644 --- a/src/Microsoft.IIS.Administration.Files.Core/FileInfo.cs +++ b/src/Microsoft.IIS.Administration.Files.Core/FileInfo.cs @@ -127,7 +127,12 @@ namespace Microsoft.IIS.Administration.Files public long Size { get { - return _info.Length; + try { + return _info.Length; + } + catch (FileNotFoundException) { + return 0; + } } } diff --git a/src/Microsoft.IIS.Administration.Files/Copy/MoveHelper.cs b/src/Microsoft.IIS.Administration.Files/Copy/MoveHelper.cs index a97b36f..89a4e2d 100644 --- a/src/Microsoft.IIS.Administration.Files/Copy/MoveHelper.cs +++ b/src/Microsoft.IIS.Administration.Files/Copy/MoveHelper.cs @@ -154,21 +154,19 @@ namespace Microsoft.IIS.Administration.Files private MoveOperation MoveDirectory(IFileInfo source, IFileInfo destination, bool copy) { - Task t; - MoveOperation op = null; + var op = new MoveOperation(source, destination, null, _fileService); if (copy) { - t = CopyDirectory(source, destination, (s, d) => SafeMoveFile(s, d, PathUtil.GetTempFilePath(d.Path), true).ContinueWith(t2 => { + op.Task = CopyDirectory(source, destination, (s, d) => SafeMoveFile(s, d, PathUtil.GetTempFilePath(d.Path), true).ContinueWith(t2 => { if (op != null) { op.CurrentSize += _fileService.GetFile(d.Path).Size; } })); } else { - t = Task.Run(() => _fileService.Move(source, destination)); + op.Task = Task.Run(() => _fileService.Move(source, destination)); } - op = new MoveOperation(t, source, destination, null, _fileService); return op; } @@ -176,9 +174,11 @@ namespace Microsoft.IIS.Administration.Files { string temp = PathUtil.GetTempFilePath(destination.Path); - Task t = SafeMoveFile(source, destination, temp, copy); + var op = new MoveOperation(source, destination, temp, _fileService); - return new MoveOperation(t, source, destination, temp, _fileService); + op.Task = SafeMoveFile(source, destination, temp, copy); + + return op; } private async Task SafeMoveFile(IFileInfo source, IFileInfo destination, string temp, bool copy) diff --git a/src/Microsoft.IIS.Administration.Files/Copy/MoveOperation.cs b/src/Microsoft.IIS.Administration.Files/Copy/MoveOperation.cs index 44904a1..3baedce 100644 --- a/src/Microsoft.IIS.Administration.Files/Copy/MoveOperation.cs +++ b/src/Microsoft.IIS.Administration.Files/Copy/MoveOperation.cs @@ -13,11 +13,10 @@ namespace Microsoft.IIS.Administration.Files class MoveOperation { - private long _totalSize = -1; private long _currentSize = -1; private IFileProvider _fileProvider; - public MoveOperation(Task task, IFileInfo source, IFileInfo destination, string tempPath, IFileProvider fileProvider) + public MoveOperation(IFileInfo source, IFileInfo destination, string tempPath, IFileProvider fileProvider) { var bytes = new byte[32]; using (var rng = RandomNumberGenerator.Create()) { @@ -25,38 +24,22 @@ namespace Microsoft.IIS.Administration.Files this.Id = Base64.Encode(bytes); } - this.Task = task; this.Source = source; this.Destination = destination; this.Created = DateTime.UtcNow; this.TempPath = tempPath; _fileProvider = fileProvider; + Initialize(); } public string Id { get; private set; } - public Task Task { get; private set; } + public Task Task { get; set; } public FileType Type { get; private set; } public string TempPath { get; private set; } public DateTime Created { get; private set; } public IFileInfo Destination { get; private set; } public IFileInfo Source { get; private set; } - - public long TotalSize { - get { - if (_totalSize != -1) { - return _totalSize; - } - - if (Source.Type == FileType.File) { - _totalSize = Source.Exists ? Source.Size : 0; - } - else { - _totalSize = Source.Exists ? _fileProvider.GetFiles(Source, "*", SearchOption.AllDirectories).Aggregate(0L, (prev, f) => prev + (f.Exists ? f.Size : 0)) : 0; - } - - return _totalSize; - } - } + public long TotalSize { get; private set; } public long CurrentSize { get { @@ -77,5 +60,20 @@ namespace Microsoft.IIS.Administration.Files _currentSize = value; } } + + private void Initialize() + { + if (Source.Type == FileType.File) { + TotalSize = Source.Exists ? Source.Size : 0; + } + else { + try { + TotalSize = Source.Exists ? _fileProvider.GetFiles(Source, "*", SearchOption.AllDirectories).Aggregate(0L, (prev, f) => prev + f.Size) : 0; + } + catch (DirectoryNotFoundException) { + TotalSize = 0; + } + } + } } } diff --git a/src/Microsoft.IIS.Administration.Files/Startup.cs b/src/Microsoft.IIS.Administration.Files/Startup.cs index 8e0f857..83767e2 100644 --- a/src/Microsoft.IIS.Administration.Files/Startup.cs +++ b/src/Microsoft.IIS.Administration.Files/Startup.cs @@ -91,6 +91,8 @@ namespace Microsoft.IIS.Administration.Files hal.ProvideLink(Defines.CopyResource.Guid, "self", copy => new { href = MoveHelper.GetLocation(copy.id, true) }); hal.ProvideLink(Defines.FilesResource.Guid, Defines.CopyResource.Name, file => new { href = $"/{Defines.COPY_PATH}" }); + + hal.ProvideLink(Defines.DirectoriesResource.Guid, Defines.CopyResource.Name, dir => new { href = $"/{Defines.COPY_PATH}" }); } private void ConfigureMove() @@ -104,6 +106,8 @@ namespace Microsoft.IIS.Administration.Files hal.ProvideLink(Defines.MoveResource.Guid, "self", move => new { href = MoveHelper.GetLocation(move.id, false) }); hal.ProvideLink(Defines.FilesResource.Guid, Defines.MoveResource.Name, file => new { href = $"/{Defines.MOVE_PATH}" }); + + hal.ProvideLink(Defines.DirectoriesResource.Guid, Defines.MoveResource.Name, dir => new { href = $"/{Defines.MOVE_PATH}" }); } } } diff --git a/test/Microsoft.IIS.Administration.Tests/Files.cs b/test/Microsoft.IIS.Administration.Tests/Files.cs index e6f854e..e3eb856 100644 --- a/test/Microsoft.IIS.Administration.Tests/Files.cs +++ b/test/Microsoft.IIS.Administration.Tests/Files.cs @@ -14,6 +14,7 @@ namespace Microsoft.IIS.Administration.Tests using System.Net; using System.Net.Http; using System.Text; + using System.Threading; using System.Threading.Tasks; using Web.Administration; using Xunit; @@ -324,6 +325,108 @@ namespace Microsoft.IIS.Administration.Tests } } + [Fact] + public void MoveDirectory() + { + string startName = "move_dir_test"; + string destName = "move_dir_dest"; + var physicalPath = Path.Combine(Configuration.TEST_ROOT_PATH, startName); + var destPhysicalPath = Path.Combine(Configuration.TEST_ROOT_PATH, destName); + + CreateTestDirectory(physicalPath); + + JObject site = null; + using (HttpClient client = ApiHttpClient.Create()) { + Sites.EnsureNoSite(client, FILE_TEST_SITE_NAME); + site = Sites.CreateSite(client, FILE_TEST_SITE_NAME, Utils.GetAvailablePort(), physicalPath); + + try { + var rootDir = Utils.FollowLink(client, site, "files"); + var rootDirFileInfo = Utils.FollowLink(client, rootDir.Value("file_info"), "self"); + + object move = new { + name = destName, + parent = rootDirFileInfo.Value("parent"), + file = rootDirFileInfo + }; + + var moveInfo = client.Post(Utils.GetLink(rootDirFileInfo, "move"), move); + + // Wait for move to finish + HttpResponseMessage res = null; + while (res == null || res.StatusCode == HttpStatusCode.OK) { + res = client.GetAsync(Utils.Self(moveInfo)).Result; + Thread.Sleep(25); + } + + Assert.True(!Directory.Exists(physicalPath)); + Assert.True(VerifyTestDirectory(destPhysicalPath)); + } + finally { + if (site != null) { + Sites.DeleteSite(client, Utils.Self(site)); + } + + if (Directory.Exists(destPhysicalPath)) { + Directory.Delete(destPhysicalPath, true); + } + } + } + } + + [Fact] + public void CopyDirectory() + { + string startName = "copy_dir_test"; + string destName = "copy_dir_dest"; + var physicalPath = Path.Combine(Configuration.TEST_ROOT_PATH, startName); + var destPhysicalPath = Path.Combine(Configuration.TEST_ROOT_PATH, destName); + + CreateTestDirectory(physicalPath); + + JObject site = null; + using (HttpClient client = ApiHttpClient.Create()) { + Sites.EnsureNoSite(client, FILE_TEST_SITE_NAME); + site = Sites.CreateSite(client, FILE_TEST_SITE_NAME, Utils.GetAvailablePort(), physicalPath); + + try { + var rootDir = Utils.FollowLink(client, site, "files"); + var rootDirFileInfo = Utils.FollowLink(client, rootDir.Value("file_info"), "self"); + + object copy = new { + name = destName, + parent = rootDirFileInfo.Value("parent"), + file = rootDirFileInfo + }; + + var copyInfo = client.Post(Utils.GetLink(rootDirFileInfo, "copy"), copy); + + // + // Wait for copy to finish + HttpResponseMessage res = null; + do { + res = client.GetAsync(Utils.Self(copyInfo)).Result; + } while (res.StatusCode == HttpStatusCode.OK); + + Assert.True(VerifyTestDirectory(physicalPath)); + Assert.True(VerifyTestDirectory(destPhysicalPath)); + } + finally { + if (site != null) { + Sites.DeleteSite(client, Utils.Self(site)); + } + + if (Directory.Exists(physicalPath)) { + Directory.Delete(physicalPath, true); + } + + if (Directory.Exists(destPhysicalPath)) { + Directory.Delete(destPhysicalPath, true); + } + } + } + } + [Fact] public void RangeUploadDownload() { @@ -742,5 +845,63 @@ namespace Microsoft.IIS.Administration.Tests return Utils.FollowLink(client, file, "self"); } + + private void CreateTestDirectory(string path) + { + if (Directory.Exists(path)) { + Directory.Delete(path, true); + Directory.CreateDirectory(path); + } + + // + // Create directory with large amount of files to move + var tasks = new List(); + var kb = GetFileSlice(0, 1000); + for (var i = 0; i < 50; i++) { + string dir = Path.Combine(path, i.ToString()); + + tasks.Add(Task.Run(() => { + Directory.CreateDirectory(dir); + for (var j = 0; j < 50; j++) { + using (var stream = File.Create(Path.Combine(dir, j.ToString()))) { + for (var k = 0; k < 10; k++) { + stream.Write(kb, 0, kb.Length); + } + } + } + })); + } + Task.WaitAll(tasks.ToArray()); + } + + private bool VerifyTestDirectory(string path) + { + if (!Directory.Exists(path)) { + return false; + } + + var kb = GetFileSlice(0, 1000); + for (var i = 0; i < 50; i++) { + string dir = Path.Combine(path, i.ToString()); + + if (!Directory.Exists(dir)) { + return false; + } + + for (var j = 0; j < 50; j++) { + var bytes = File.ReadAllBytes(Path.Combine(dir, j.ToString())); + + for (var k = 0; k < 10; k++) { + for (var l = 0; l < kb.Length; l++) { + if (bytes[(k * kb.Length) + l] != kb[l]) { + return false; + } + } + } + } + } + + return true; + } } } \ No newline at end of file