From 11803afc3932e5cceef1bd3345f6389c440ab5a7 Mon Sep 17 00:00:00 2001 From: Taloth Saldono Date: Wed, 21 Jan 2015 20:59:03 +0100 Subject: [PATCH] Added FolderWritable to DiskProvider to centralize the check. --- .../DiskTests/DiskProviderFixtureBase.cs | 34 ++++++++++++++++++- src/NzbDrone.Common/Disk/DiskProviderBase.cs | 19 +++++++++++ src/NzbDrone.Common/Disk/IDiskProvider.cs | 1 + .../Download/Clients/Pneumatic/Pneumatic.cs | 26 ++------------ .../Download/DownloadClientBase.cs | 10 ++---- .../HealthCheck/Checks/DroneFactoryCheck.cs | 8 +---- .../HealthCheck/Checks/UpdateCheck.cs | 8 +---- .../DiskProviderTests/DiskProviderFixture.cs | 19 ++++++++++- .../NzbDrone.Mono.Test.csproj | 4 +++ .../DiskProviderTests/DiskProviderFixture.cs | 26 +++++++++++++- 10 files changed, 106 insertions(+), 49 deletions(-) diff --git a/src/NzbDrone.Common.Test/DiskTests/DiskProviderFixtureBase.cs b/src/NzbDrone.Common.Test/DiskTests/DiskProviderFixtureBase.cs index 3c98cdfe2..fc34df228 100644 --- a/src/NzbDrone.Common.Test/DiskTests/DiskProviderFixtureBase.cs +++ b/src/NzbDrone.Common.Test/DiskTests/DiskProviderFixtureBase.cs @@ -8,7 +8,7 @@ using NzbDrone.Test.Common; namespace NzbDrone.Common.Test.DiskTests { - public class DiskProviderFixtureBase : TestBase where TSubject : class, IDiskProvider + public abstract class DiskProviderFixtureBase : TestBase where TSubject : class, IDiskProvider { public DirectoryInfo GetFilledTempFolder() { @@ -46,6 +46,38 @@ namespace NzbDrone.Common.Test.DiskTests Subject.FolderExists(@"C:\ThisBetterNotExist\".AsOsAgnostic()).Should().BeFalse(); } + protected abstract void SetWritePermissions(string path, bool writable); + + [Test] + public void FolderWritable_should_return_true_for_writable_directory() + { + var tempFolder = GetTempFilePath(); + Directory.CreateDirectory(tempFolder); + + var result = Subject.FolderWritable(tempFolder); + + result.Should().BeTrue(); + } + + [Test] + public void FolderWritable_should_return_false_for_unwritable_directory() + { + var tempFolder = GetTempFilePath(); + Directory.CreateDirectory(tempFolder); + + SetWritePermissions(tempFolder, false); + try + { + var result = Subject.FolderWritable(tempFolder); + + result.Should().BeFalse(); + } + finally + { + SetWritePermissions(tempFolder, true); + } + } + [Test] public void MoveFile_should_overwrite_existing_file() { diff --git a/src/NzbDrone.Common/Disk/DiskProviderBase.cs b/src/NzbDrone.Common/Disk/DiskProviderBase.cs index c50801cc9..de3075901 100644 --- a/src/NzbDrone.Common/Disk/DiskProviderBase.cs +++ b/src/NzbDrone.Common/Disk/DiskProviderBase.cs @@ -109,6 +109,25 @@ namespace NzbDrone.Common.Disk } } + public bool FolderWritable(string path) + { + Ensure.That(path, () => path).IsValidPath(); + + try + { + var testPath = Path.Combine(path, "sonarr_write_test.txt"); + var testContent = string.Format("This file was created to verify if '{0}' is writable. It should've been automatically deleted. Feel free to delete it.", path); + File.WriteAllText(testPath, testContent); + File.Delete(testPath); + return true; + } + catch (Exception e) + { + Logger.Trace("Directory '{0}' isn't writable. {1}", path, e.Message); + return false; + } + } + public string[] GetDirectories(string path) { Ensure.That(path, () => path).IsValidPath(); diff --git a/src/NzbDrone.Common/Disk/IDiskProvider.cs b/src/NzbDrone.Common/Disk/IDiskProvider.cs index 445a97fed..7374080e5 100644 --- a/src/NzbDrone.Common/Disk/IDiskProvider.cs +++ b/src/NzbDrone.Common/Disk/IDiskProvider.cs @@ -19,6 +19,7 @@ namespace NzbDrone.Common.Disk bool FolderExists(string path); bool FileExists(string path); bool FileExists(string path, StringComparison stringComparison); + bool FolderWritable(string path); string[] GetDirectories(string path); string[] GetFiles(string path, SearchOption searchOption); long GetFolderSize(string path); diff --git a/src/NzbDrone.Core/Download/Clients/Pneumatic/Pneumatic.cs b/src/NzbDrone.Core/Download/Clients/Pneumatic/Pneumatic.cs index 12cfaf82a..cbbf5f26b 100644 --- a/src/NzbDrone.Core/Download/Clients/Pneumatic/Pneumatic.cs +++ b/src/NzbDrone.Core/Download/Clients/Pneumatic/Pneumatic.cs @@ -122,30 +122,8 @@ namespace NzbDrone.Core.Download.Clients.Pneumatic protected override void Test(List failures) { - failures.AddIfNotNull(TestWrite(Settings.NzbFolder, "NzbFolder")); - failures.AddIfNotNull(TestWrite(Settings.StrmFolder, "StrmFolder")); - } - - private ValidationFailure TestWrite(String folder, String propertyName) - { - if (!_diskProvider.FolderExists(folder)) - { - return new ValidationFailure(propertyName, "Folder does not exist"); - } - - try - { - var testPath = Path.Combine(folder, "drone_test.txt"); - _diskProvider.WriteAllText(testPath, DateTime.Now.ToString()); - _diskProvider.DeleteFile(testPath); - } - catch (Exception ex) - { - _logger.ErrorException(ex.Message, ex); - return new ValidationFailure(propertyName, "Unable to write to folder"); - } - - return null; + failures.AddIfNotNull(TestFolder(Settings.NzbFolder, "NzbFolder")); + failures.AddIfNotNull(TestFolder(Settings.StrmFolder, "StrmFolder")); } private String WriteStrmFile(String title, String nzbFile) diff --git a/src/NzbDrone.Core/Download/DownloadClientBase.cs b/src/NzbDrone.Core/Download/DownloadClientBase.cs index e492eed81..61de7769f 100644 --- a/src/NzbDrone.Core/Download/DownloadClientBase.cs +++ b/src/NzbDrone.Core/Download/DownloadClientBase.cs @@ -104,15 +104,9 @@ namespace NzbDrone.Core.Download if (mustBeWritable) { - try + if (!_diskProvider.FolderWritable(folder)) { - var testPath = Path.Combine(folder, "drone_test.txt"); - _diskProvider.WriteAllText(testPath, DateTime.Now.ToString()); - _diskProvider.DeleteFile(testPath); - } - catch (Exception ex) - { - _logger.ErrorException(ex.Message, ex); + _logger.Error("Folder '{0}' is not writable.", folder); return new NzbDroneValidationFailure(propertyName, "Unable to write to folder") { DetailedDescription = "The folder you specified is not writable. Please verify the folder permissions for the user account that is used to execute NzbDrone." diff --git a/src/NzbDrone.Core/HealthCheck/Checks/DroneFactoryCheck.cs b/src/NzbDrone.Core/HealthCheck/Checks/DroneFactoryCheck.cs index 4ba86b8e0..0bc4f7255 100644 --- a/src/NzbDrone.Core/HealthCheck/Checks/DroneFactoryCheck.cs +++ b/src/NzbDrone.Core/HealthCheck/Checks/DroneFactoryCheck.cs @@ -31,13 +31,7 @@ namespace NzbDrone.Core.HealthCheck.Checks return new HealthCheck(GetType(), HealthCheckResult.Error, "Drone factory folder does not exist"); } - try - { - var testPath = Path.Combine(droneFactoryFolder, "drone_test.txt"); - _diskProvider.WriteAllText(testPath, DateTime.Now.ToString()); - _diskProvider.DeleteFile(testPath); - } - catch (Exception) + if (!_diskProvider.FolderWritable(droneFactoryFolder)) { return new HealthCheck(GetType(), HealthCheckResult.Error, "Unable to write to drone factory folder"); } diff --git a/src/NzbDrone.Core/HealthCheck/Checks/UpdateCheck.cs b/src/NzbDrone.Core/HealthCheck/Checks/UpdateCheck.cs index abeaa0137..c9c968a84 100644 --- a/src/NzbDrone.Core/HealthCheck/Checks/UpdateCheck.cs +++ b/src/NzbDrone.Core/HealthCheck/Checks/UpdateCheck.cs @@ -29,13 +29,7 @@ namespace NzbDrone.Core.HealthCheck.Checks { if (OsInfo.IsWindows || _configFileProvider.UpdateAutomatically) { - try - { - var testPath = Path.Combine(_appFolderInfo.StartUpFolder, "drone_test.txt"); - _diskProvider.WriteAllText(testPath, DateTime.Now.ToString()); - _diskProvider.DeleteFile(testPath); - } - catch (Exception) + if (!_diskProvider.FolderWritable(_appFolderInfo.StartUpFolder)) { return new HealthCheck(GetType(), HealthCheckResult.Error, "Unable to update, running from write-protected folder"); } diff --git a/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs b/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs index 130d72e92..5317ad891 100644 --- a/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs +++ b/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs @@ -1,4 +1,5 @@ -using NUnit.Framework; +using Mono.Unix; +using NUnit.Framework; using NzbDrone.Common.Test.DiskTests; namespace NzbDrone.Mono.Test.DiskProviderTests @@ -11,5 +12,21 @@ namespace NzbDrone.Mono.Test.DiskProviderTests { MonoOnly(); } + + protected override void SetWritePermissions(string path, bool writable) + { + // Remove Write permissions, we're still owner so we can clean it up, but we'll have to do that explicitly. + + var entry = UnixFileSystemInfo.GetFileSystemEntry(path); + + if (writable) + { + entry.FileAccessPermissions |= FileAccessPermissions.UserWrite | FileAccessPermissions.GroupWrite | FileAccessPermissions.OtherWrite; + } + else + { + entry.FileAccessPermissions &= ~(FileAccessPermissions.UserWrite | FileAccessPermissions.GroupWrite | FileAccessPermissions.OtherWrite); + } + } } } diff --git a/src/NzbDrone.Mono.Test/NzbDrone.Mono.Test.csproj b/src/NzbDrone.Mono.Test/NzbDrone.Mono.Test.csproj index 2a4622534..8fa63c167 100644 --- a/src/NzbDrone.Mono.Test/NzbDrone.Mono.Test.csproj +++ b/src/NzbDrone.Mono.Test/NzbDrone.Mono.Test.csproj @@ -58,6 +58,10 @@ False ..\packages\FluentAssertions.3.2.1\lib\net40\FluentAssertions.Core.dll + + False + ..\Libraries\Mono.Posix.dll + False ..\packages\NUnit.2.6.3\lib\nunit.framework.dll diff --git a/src/NzbDrone.Windows.Test/DiskProviderTests/DiskProviderFixture.cs b/src/NzbDrone.Windows.Test/DiskProviderTests/DiskProviderFixture.cs index 88def60e3..c60052c72 100644 --- a/src/NzbDrone.Windows.Test/DiskProviderTests/DiskProviderFixture.cs +++ b/src/NzbDrone.Windows.Test/DiskProviderTests/DiskProviderFixture.cs @@ -1,4 +1,7 @@ -using NUnit.Framework; +using System.IO; +using System.Security.AccessControl; +using System.Security.Principal; +using NUnit.Framework; using NzbDrone.Common.Test.DiskTests; namespace NzbDrone.Windows.Test.DiskProviderTests @@ -11,5 +14,26 @@ namespace NzbDrone.Windows.Test.DiskProviderTests { WindowsOnly(); } + + protected override void SetWritePermissions(string path, bool writable) + { + // Remove Write permissions, we're owner and have Delete permissions, so we can still clean it up. + + var owner = WindowsIdentity.GetCurrent().Owner; + var accessControlType = writable ? AccessControlType.Allow : AccessControlType.Deny; + + if (Directory.Exists(path)) + { + var ds = Directory.GetAccessControl(path); + ds.SetAccessRule(new FileSystemAccessRule(owner, FileSystemRights.Write, accessControlType)); + Directory.SetAccessControl(path, ds); + } + else + { + var fs = File.GetAccessControl(path); + fs.SetAccessRule(new FileSystemAccessRule(owner, FileSystemRights.Write, accessControlType)); + File.SetAccessControl(path, fs); + } + } } }