From 473405ceebd4763a0eff84615120bc25a6aa86e6 Mon Sep 17 00:00:00 2001 From: Qstick Date: Sat, 2 Jul 2022 18:40:00 -0500 Subject: [PATCH] Update file and folder handling methods from Radarr (#1051) * Update file/folder handling methods from Radarr * fixup! --- .../DiskTests/DiskProviderFixtureBase.cs | 10 + .../DiskTests/DiskTransferServiceFixture.cs | 91 +++++++++ .../DiskTests/FreeSpaceFixtureBase.cs | 37 ---- src/NzbDrone.Common/Disk/DiskProviderBase.cs | 29 ++- .../Disk/DiskTransferService.cs | 63 +++++- src/NzbDrone.Common/Disk/IDiskProvider.cs | 5 +- src/NzbDrone.Common/Disk/LongPathSupport.cs | 64 +++++++ .../UpdateTests/UpdateServiceFixture.cs | 21 +- .../Update/InstallUpdateService.cs | 31 +-- ...odValidator.cs => FolderChmodValidator.cs} | 6 +- .../DiskProviderTests/DiskProviderFixture.cs | 179 ++++++++++++++---- .../DiskProviderTests/FreeSpaceFixture.cs | 17 +- src/NzbDrone.Mono/Disk/DiskProvider.cs | 91 ++++----- src/NzbDrone.Mono/Disk/FindDriveType.cs | 1 + .../UpdateEngine/InstallUpdateService.cs | 10 +- .../DiskProviderTests/FreeSpaceFixture.cs | 29 ++- src/NzbDrone.Windows/Disk/DiskProvider.cs | 6 +- 17 files changed, 531 insertions(+), 159 deletions(-) rename src/NzbDrone.Core/Validation/{FileChmodValidator.cs => FolderChmodValidator.cs} (69%) diff --git a/src/NzbDrone.Common.Test/DiskTests/DiskProviderFixtureBase.cs b/src/NzbDrone.Common.Test/DiskTests/DiskProviderFixtureBase.cs index 8b9e72b4f..63c991a29 100644 --- a/src/NzbDrone.Common.Test/DiskTests/DiskProviderFixtureBase.cs +++ b/src/NzbDrone.Common.Test/DiskTests/DiskProviderFixtureBase.cs @@ -10,6 +10,16 @@ namespace NzbDrone.Common.Test.DiskTests public abstract class DiskProviderFixtureBase : TestBase where TSubject : class, IDiskProvider { + [Test] + public void writealltext_should_truncate_existing() + { + var file = GetTempFilePath(); + + Subject.WriteAllText(file, "A pretty long string"); + Subject.WriteAllText(file, "A short string"); + Subject.ReadAllText(file).Should().Be("A short string"); + } + [Test] [Retry(5)] public void directory_exist_should_be_able_to_find_existing_folder() diff --git a/src/NzbDrone.Common.Test/DiskTests/DiskTransferServiceFixture.cs b/src/NzbDrone.Common.Test/DiskTests/DiskTransferServiceFixture.cs index 1672fe0d4..9e6799d43 100644 --- a/src/NzbDrone.Common.Test/DiskTests/DiskTransferServiceFixture.cs +++ b/src/NzbDrone.Common.Test/DiskTests/DiskTransferServiceFixture.cs @@ -402,6 +402,40 @@ namespace NzbDrone.Common.Test.DiskTests VerifyCopyFolder(source.FullName, destination.FullName); } + [Test] + public void CopyFolder_should_detect_caseinsensitive_parents() + { + WindowsOnly(); + + WithRealDiskProvider(); + + var original = GetFilledTempFolder(); + var root = new DirectoryInfo(GetTempFilePath()); + var source = new DirectoryInfo(root.FullName + "A/series"); + var destination = new DirectoryInfo(root.FullName + "a/series"); + + Subject.TransferFolder(original.FullName, source.FullName, TransferMode.Copy); + + Assert.Throws(() => Subject.TransferFolder(source.FullName, destination.FullName, TransferMode.Copy)); + } + + [Test] + public void CopyFolder_should_detect_caseinsensitive_folder() + { + WindowsOnly(); + + WithRealDiskProvider(); + + var original = GetFilledTempFolder(); + var root = new DirectoryInfo(GetTempFilePath()); + var source = new DirectoryInfo(root.FullName + "A/series"); + var destination = new DirectoryInfo(root.FullName + "A/Series"); + + Subject.TransferFolder(original.FullName, source.FullName, TransferMode.Copy); + + Assert.Throws(() => Subject.TransferFolder(source.FullName, destination.FullName, TransferMode.Copy)); + } + [Test] public void CopyFolder_should_ignore_nfs_temp_file() { @@ -451,6 +485,42 @@ namespace NzbDrone.Common.Test.DiskTests VerifyMoveFolder(original.FullName, source.FullName, destination.FullName); } + [Test] + public void MoveFolder_should_detect_caseinsensitive_parents() + { + WindowsOnly(); + + WithRealDiskProvider(); + + var original = GetFilledTempFolder(); + var root = new DirectoryInfo(GetTempFilePath()); + var source = new DirectoryInfo(root.FullName + "A/series"); + var destination = new DirectoryInfo(root.FullName + "a/series"); + + Subject.TransferFolder(original.FullName, source.FullName, TransferMode.Copy); + + Assert.Throws(() => Subject.TransferFolder(source.FullName, destination.FullName, TransferMode.Move)); + } + + [Test] + public void MoveFolder_should_rename_caseinsensitive_folder() + { + WindowsOnly(); + + WithRealDiskProvider(); + + var original = GetFilledTempFolder(); + var root = new DirectoryInfo(GetTempFilePath()); + var source = new DirectoryInfo(root.FullName + "A/series"); + var destination = new DirectoryInfo(root.FullName + "A/Series"); + + Subject.TransferFolder(original.FullName, source.FullName, TransferMode.Copy); + + Subject.TransferFolder(source.FullName, destination.FullName, TransferMode.Move); + + source.FullName.GetActualCasing().Should().Be(destination.FullName); + } + [Test] public void should_throw_if_destination_is_readonly() { @@ -553,6 +623,23 @@ namespace NzbDrone.Common.Test.DiskTests VerifyCopyFolder(original.FullName, destination.FullName); } + [Test] + public void MirrorFolder_should_handle_trailing_slash() + { + WithRealDiskProvider(); + + var original = GetFilledTempFolder(); + var source = new DirectoryInfo(GetTempFilePath()); + var destination = new DirectoryInfo(GetTempFilePath()); + + Subject.TransferFolder(original.FullName, source.FullName, TransferMode.Copy); + + var count = Subject.MirrorFolder(source.FullName + Path.DirectorySeparatorChar, destination.FullName); + + count.Should().Equals(3); + VerifyCopyFolder(original.FullName, destination.FullName); + } + [Test] public void TransferFolder_should_use_movefolder_if_on_same_mount() { @@ -752,6 +839,10 @@ namespace NzbDrone.Common.Test.DiskTests .Setup(v => v.CreateFolder(It.IsAny())) .Callback(v => Directory.CreateDirectory(v)); + Mocker.GetMock() + .Setup(v => v.MoveFolder(It.IsAny(), It.IsAny(), It.IsAny())) + .Callback((v, r, b) => Directory.Move(v, r)); + Mocker.GetMock() .Setup(v => v.DeleteFolder(It.IsAny(), It.IsAny())) .Callback((v, r) => Directory.Delete(v, r)); diff --git a/src/NzbDrone.Common.Test/DiskTests/FreeSpaceFixtureBase.cs b/src/NzbDrone.Common.Test/DiskTests/FreeSpaceFixtureBase.cs index 63d973946..220f4734d 100644 --- a/src/NzbDrone.Common.Test/DiskTests/FreeSpaceFixtureBase.cs +++ b/src/NzbDrone.Common.Test/DiskTests/FreeSpaceFixtureBase.cs @@ -28,14 +28,6 @@ namespace NzbDrone.Common.Test.DiskTests Subject.GetAvailableSpace(Path.Combine(path, "invalidFolder")).Should().NotBe(0); } - [Ignore("Docker")] - [Test] - public void should_be_able_to_check_space_on_ramdrive() - { - PosixOnly(); - Subject.GetAvailableSpace("/run/").Should().NotBe(0); - } - [Ignore("Docker")] [Test] public void should_return_free_disk_space() @@ -44,35 +36,6 @@ namespace NzbDrone.Common.Test.DiskTests result.Should().BeGreaterThan(0); } - [Test] - public void should_be_able_to_get_space_on_unc() - { - WindowsOnly(); - - var result = Subject.GetAvailableSpace(@"\\localhost\c$\Windows"); - result.Should().BeGreaterThan(0); - } - - [Test] - public void should_throw_if_drive_doesnt_exist() - { - WindowsOnly(); - - // Find a drive that doesn't exist. - for (char driveletter = 'Z'; driveletter > 'D'; driveletter--) - { - if (new DriveInfo(driveletter.ToString()).IsReady) - { - continue; - } - - Assert.Throws(() => Subject.GetAvailableSpace(driveletter + @":\NOT_A_REAL_PATH\DOES_NOT_EXIST".AsOsAgnostic())); - return; - } - - Assert.Inconclusive("No drive available for testing."); - } - [Ignore("Docker")] [Test] public void should_be_able_to_get_space_on_folder_that_doesnt_exist() diff --git a/src/NzbDrone.Common/Disk/DiskProviderBase.cs b/src/NzbDrone.Common/Disk/DiskProviderBase.cs index 084e515c2..9aa5ebdec 100644 --- a/src/NzbDrone.Common/Disk/DiskProviderBase.cs +++ b/src/NzbDrone.Common/Disk/DiskProviderBase.cs @@ -30,7 +30,8 @@ namespace NzbDrone.Common.Disk public abstract long? GetAvailableSpace(string path); public abstract void InheritFolderPermissions(string filename); public abstract void SetEveryonePermissions(string filename); - public abstract void SetPermissions(string path, string mask); + public abstract void SetFilePermissions(string path, string mask, string group); + public abstract void SetPermissions(string path, string mask, string group); public abstract void CopyPermissions(string sourcePath, string targetPath); public abstract long? GetTotalSize(string path); @@ -130,7 +131,7 @@ namespace NzbDrone.Common.Disk { var testPath = Path.Combine(path, "prowlarr_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); + WriteAllText(testPath, testContent); File.Delete(testPath); return true; } @@ -258,17 +259,6 @@ namespace NzbDrone.Common.Disk Ensure.That(source, () => source).IsValidPath(); Ensure.That(destination, () => destination).IsValidPath(); - if (source.PathEquals(destination)) - { - throw new IOException(string.Format("Source and destination can't be the same {0}", source)); - } - - if (FolderExists(destination) && overwrite) - { - DeleteFolder(destination, true); - } - - RemoveReadOnlyFolder(source); Directory.Move(source, destination); } @@ -310,7 +300,16 @@ namespace NzbDrone.Common.Disk { Ensure.That(filename, () => filename).IsValidPath(); RemoveReadOnly(filename); - File.WriteAllText(filename, contents); + + // File.WriteAllText is broken on net core when writing to some CIFS mounts + // This workaround from https://github.com/dotnet/runtime/issues/42790#issuecomment-700362617 + using (var fs = new FileStream(filename, FileMode.Create, FileAccess.Write, FileShare.None)) + { + using (var writer = new StreamWriter(fs)) + { + writer.Write(contents); + } + } } public void FolderSetLastWriteTime(string path, DateTime dateTime) @@ -550,7 +549,7 @@ namespace NzbDrone.Common.Disk } } - public virtual bool IsValidFilePermissionMask(string mask) + public virtual bool IsValidFolderPermissionMask(string mask) { throw new NotSupportedException(); } diff --git a/src/NzbDrone.Common/Disk/DiskTransferService.cs b/src/NzbDrone.Common/Disk/DiskTransferService.cs index 5d7e7d23a..44d28a9df 100644 --- a/src/NzbDrone.Common/Disk/DiskTransferService.cs +++ b/src/NzbDrone.Common/Disk/DiskTransferService.cs @@ -4,7 +4,6 @@ using System.Linq; using System.Threading; using NLog; using NzbDrone.Common.EnsureThat; -using NzbDrone.Common.EnvironmentInfo; using NzbDrone.Common.Extensions; namespace NzbDrone.Common.Disk @@ -27,11 +26,56 @@ namespace NzbDrone.Common.Disk _logger = logger; } + private string ResolveRealParentPath(string path) + { + var parentPath = path.GetParentPath(); + if (!_diskProvider.FolderExists(parentPath)) + { + return path; + } + + var realParentPath = parentPath.GetActualCasing(); + + var partialChildPath = path.Substring(parentPath.Length); + + return realParentPath + partialChildPath; + } + public TransferMode TransferFolder(string sourcePath, string targetPath, TransferMode mode) { Ensure.That(sourcePath, () => sourcePath).IsValidPath(); Ensure.That(targetPath, () => targetPath).IsValidPath(); + sourcePath = ResolveRealParentPath(sourcePath); + targetPath = ResolveRealParentPath(targetPath); + + _logger.Debug("{0} Directory [{1}] > [{2}]", mode, sourcePath, targetPath); + + if (sourcePath == targetPath) + { + throw new IOException(string.Format("Source and destination can't be the same {0}", sourcePath)); + } + + if (mode == TransferMode.Move && sourcePath.PathEquals(targetPath, StringComparison.InvariantCultureIgnoreCase) && _diskProvider.FolderExists(targetPath)) + { + // Move folder out of the way to allow case-insensitive renames + var tempPath = sourcePath + ".backup~"; + _logger.Trace("Rename Intermediate Directory [{0}] > [{1}]", sourcePath, tempPath); + _diskProvider.MoveFolder(sourcePath, tempPath); + + if (!_diskProvider.FolderExists(targetPath)) + { + _logger.Trace("Rename Intermediate Directory [{0}] > [{1}]", tempPath, targetPath); + _logger.Debug("Rename Directory [{0}] > [{1}]", sourcePath, targetPath); + _diskProvider.MoveFolder(tempPath, targetPath); + return mode; + } + + // There were two separate folders, revert the intermediate rename and let the recursion deal with it + _logger.Trace("Rename Intermediate Directory [{0}] > [{1}]", tempPath, sourcePath); + _diskProvider.MoveFolder(tempPath, sourcePath); + } + if (mode == TransferMode.Move && !_diskProvider.FolderExists(targetPath)) { var sourceMount = _diskProvider.GetMount(sourcePath); @@ -40,7 +84,7 @@ namespace NzbDrone.Common.Disk // If we're on the same mount, do a simple folder move. if (sourceMount != null && targetMount != null && sourceMount.RootDirectory == targetMount.RootDirectory) { - _logger.Debug("Move Directory [{0}] > [{1}]", sourcePath, targetPath); + _logger.Debug("Rename Directory [{0}] > [{1}]", sourcePath, targetPath); _diskProvider.MoveFolder(sourcePath, targetPath); return mode; } @@ -79,6 +123,13 @@ namespace NzbDrone.Common.Disk if (mode.HasFlag(TransferMode.Move)) { + var totalSize = _diskProvider.GetFileInfos(sourcePath).Sum(v => v.Length); + + if (totalSize > (100 * 1024L * 1024L)) + { + throw new IOException($"Large files still exist in {sourcePath} after folder move, not deleting source folder"); + } + _diskProvider.DeleteFolder(sourcePath, true); } @@ -92,7 +143,10 @@ namespace NzbDrone.Common.Disk Ensure.That(sourcePath, () => sourcePath).IsValidPath(); Ensure.That(targetPath, () => targetPath).IsValidPath(); - _logger.Debug("Mirror [{0}] > [{1}]", sourcePath, targetPath); + sourcePath = ResolveRealParentPath(sourcePath); + targetPath = ResolveRealParentPath(targetPath); + + _logger.Debug("Mirror Folder [{0}] > [{1}]", sourcePath, targetPath); if (!_diskProvider.FolderExists(targetPath)) { @@ -204,6 +258,9 @@ namespace NzbDrone.Common.Disk Ensure.That(sourcePath, () => sourcePath).IsValidPath(); Ensure.That(targetPath, () => targetPath).IsValidPath(); + sourcePath = ResolveRealParentPath(sourcePath); + targetPath = ResolveRealParentPath(targetPath); + _logger.Debug("{0} [{1}] > [{2}]", mode, sourcePath, targetPath); var originalSize = _diskProvider.GetFileSize(sourcePath); diff --git a/src/NzbDrone.Common/Disk/IDiskProvider.cs b/src/NzbDrone.Common/Disk/IDiskProvider.cs index cb262504a..4f1cad811 100644 --- a/src/NzbDrone.Common/Disk/IDiskProvider.cs +++ b/src/NzbDrone.Common/Disk/IDiskProvider.cs @@ -11,7 +11,8 @@ namespace NzbDrone.Common.Disk long? GetAvailableSpace(string path); void InheritFolderPermissions(string filename); void SetEveryonePermissions(string filename); - void SetPermissions(string path, string mask); + void SetFilePermissions(string path, string mask, string group); + void SetPermissions(string path, string mask, string group); void CopyPermissions(string sourcePath, string targetPath); long? GetTotalSize(string path); DateTime FolderGetCreationTime(string path); @@ -56,6 +57,6 @@ namespace NzbDrone.Common.Disk List GetFileInfos(string path, SearchOption searchOption = SearchOption.TopDirectoryOnly); void RemoveEmptySubfolders(string path); void SaveStream(Stream stream, string path); - bool IsValidFilePermissionMask(string mask); + bool IsValidFolderPermissionMask(string mask); } } diff --git a/src/NzbDrone.Common/Disk/LongPathSupport.cs b/src/NzbDrone.Common/Disk/LongPathSupport.cs index ef4bd3f7c..8d10f78d6 100644 --- a/src/NzbDrone.Common/Disk/LongPathSupport.cs +++ b/src/NzbDrone.Common/Disk/LongPathSupport.cs @@ -1,15 +1,79 @@ using System; +using System.IO; +using NzbDrone.Common.EnvironmentInfo; namespace NzbDrone.Common.Disk { public static class LongPathSupport { + private static int MAX_PATH; + private static int MAX_NAME; + public static void Enable() { // Mono has an issue with enabling long path support via app.config. // This works for both mono and .net on Windows. AppContext.SetSwitch("Switch.System.IO.UseLegacyPathHandling", false); AppContext.SetSwitch("Switch.System.IO.BlockLongPaths", false); + + DetectLongPathLimits(); + } + + private static void DetectLongPathLimits() + { + if (!int.TryParse(Environment.GetEnvironmentVariable("MAX_PATH"), out MAX_PATH)) + { + if (OsInfo.IsLinux) + { + MAX_PATH = 4096; + } + else + { + try + { + // Windows paths can be up to 32,767 characters long, but each component of the path must be less than 255. + // If the OS does not have Long Path enabled, then the following will throw an exception + // ref: https://docs.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation + Path.GetDirectoryName($@"C:\{new string('a', 254)}\{new string('a', 254)}"); + MAX_PATH = 4096; + } + catch + { + MAX_PATH = 260 - 1; + } + } + } + + if (!int.TryParse(Environment.GetEnvironmentVariable("MAX_NAME"), out MAX_NAME)) + { + MAX_NAME = 255; + } + } + + public static int MaxFilePathLength + { + get + { + if (MAX_PATH == 0) + { + DetectLongPathLimits(); + } + + return MAX_PATH; + } + } + + public static int MaxFileNameLength + { + get + { + if (MAX_NAME == 0) + { + DetectLongPathLimits(); + } + + return MAX_NAME; + } } } } diff --git a/src/NzbDrone.Core.Test/UpdateTests/UpdateServiceFixture.cs b/src/NzbDrone.Core.Test/UpdateTests/UpdateServiceFixture.cs index 56b78ade5..70fd2fbca 100644 --- a/src/NzbDrone.Core.Test/UpdateTests/UpdateServiceFixture.cs +++ b/src/NzbDrone.Core.Test/UpdateTests/UpdateServiceFixture.cs @@ -68,6 +68,10 @@ namespace NzbDrone.Core.Test.UpdateTests .Setup(c => c.FolderWritable(It.IsAny())) .Returns(true); + Mocker.GetMock() + .Setup(v => v.FileExists(It.Is(s => s.EndsWith("Prowlarr.Update.exe")))) + .Returns(true); + _sandboxFolder = Mocker.GetMock().Object.GetUpdateSandboxFolder(); } @@ -149,7 +153,7 @@ namespace NzbDrone.Core.Test.UpdateTests } [Test] - public void should_start_update_client() + public void should_start_update_client_if_updater_exists() { Subject.Execute(new ApplicationUpdateCommand()); @@ -157,6 +161,21 @@ namespace NzbDrone.Core.Test.UpdateTests .Verify(c => c.Start(It.IsAny(), It.Is(s => s.StartsWith("12")), null, null, null), Times.Once()); } + [Test] + public void should_return_with_warning_if_updater_doesnt_exists() + { + Mocker.GetMock() + .Setup(v => v.FileExists(It.Is(s => s.EndsWith("Prowlarr.Update.exe")))) + .Returns(false); + + Subject.Execute(new ApplicationUpdateCommand()); + + Mocker.GetMock() + .Verify(c => c.Start(It.IsAny(), It.IsAny(), null, null, null), Times.Never()); + + ExceptionVerification.ExpectedWarns(1); + } + [Test] public void should_return_without_error_or_warnings_when_no_updates_are_available() { diff --git a/src/NzbDrone.Core/Update/InstallUpdateService.cs b/src/NzbDrone.Core/Update/InstallUpdateService.cs index 1c0489798..c7b358861 100644 --- a/src/NzbDrone.Core/Update/InstallUpdateService.cs +++ b/src/NzbDrone.Core/Update/InstallUpdateService.cs @@ -146,16 +146,24 @@ namespace NzbDrone.Core.Update _logger.Info("Preparing client"); _diskTransferService.TransferFolder(_appFolderInfo.GetUpdateClientFolder(), updateSandboxFolder, TransferMode.Move); + var updateClientExePath = _appFolderInfo.GetUpdateClientExePath(updatePackage.Runtime); + + if (!_diskProvider.FileExists(updateClientExePath)) + { + _logger.Warn("Update client {0} does not exist, aborting update.", updateClientExePath); + return false; + } + // Set executable flag on update app if (OsInfo.IsOsx || (OsInfo.IsLinux && PlatformInfo.IsNetCore)) { - _diskProvider.SetPermissions(_appFolderInfo.GetUpdateClientExePath(updatePackage.Runtime), "0755"); + _diskProvider.SetFilePermissions(updateClientExePath, "755", null); } - _logger.Info("Starting update client {0}", _appFolderInfo.GetUpdateClientExePath(updatePackage.Runtime)); + _logger.Info("Starting update client {0}", updateClientExePath); _logger.ProgressInfo("Prowlarr will restart shortly."); - _processProvider.Start(_appFolderInfo.GetUpdateClientExePath(updatePackage.Runtime), GetUpdaterArgs(updateSandboxFolder)); + _processProvider.Start(updateClientExePath, GetUpdaterArgs(updateSandboxFolder)); return true; } @@ -298,14 +306,6 @@ namespace NzbDrone.Core.Update // Check if we have to do an application update on startup try { - // Don't do a prestartup update check unless BuiltIn update is enabled - if (_configFileProvider.UpdateAutomatically || - _configFileProvider.UpdateMechanism != UpdateMechanism.BuiltIn || - _deploymentInfoProvider.IsExternalUpdateMechanism) - { - return; - } - var updateMarker = Path.Combine(_appFolderInfo.AppDataFolder, "update_required"); if (!_diskProvider.FileExists(updateMarker)) { @@ -314,6 +314,15 @@ namespace NzbDrone.Core.Update _logger.Debug("Post-install update check requested"); + // Don't do a prestartup update check unless BuiltIn update is enabled + if (!_configFileProvider.UpdateAutomatically || + _configFileProvider.UpdateMechanism != UpdateMechanism.BuiltIn || + _deploymentInfoProvider.IsExternalUpdateMechanism) + { + _logger.Debug("Built-in updater disabled, skipping post-install update check"); + return; + } + var latestAvailable = _checkUpdateService.AvailableUpdate(); if (latestAvailable == null) { diff --git a/src/NzbDrone.Core/Validation/FileChmodValidator.cs b/src/NzbDrone.Core/Validation/FolderChmodValidator.cs similarity index 69% rename from src/NzbDrone.Core/Validation/FileChmodValidator.cs rename to src/NzbDrone.Core/Validation/FolderChmodValidator.cs index c9f0881a7..3e90bf9fa 100644 --- a/src/NzbDrone.Core/Validation/FileChmodValidator.cs +++ b/src/NzbDrone.Core/Validation/FolderChmodValidator.cs @@ -3,11 +3,11 @@ using NzbDrone.Common.Disk; namespace NzbDrone.Core.Validation { - public class FileChmodValidator : PropertyValidator + public class FolderChmodValidator : PropertyValidator { private readonly IDiskProvider _diskProvider; - public FileChmodValidator(IDiskProvider diskProvider) + public FolderChmodValidator(IDiskProvider diskProvider) : base("Must contain a valid Unix permissions octal") { _diskProvider = diskProvider; @@ -20,7 +20,7 @@ namespace NzbDrone.Core.Validation return false; } - return _diskProvider.IsValidFilePermissionMask(context.PropertyValue.ToString()); + return _diskProvider.IsValidFolderPermissionMask(context.PropertyValue.ToString()); } } } diff --git a/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs b/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs index 5ca5e831e..cb804ecf4 100644 --- a/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs +++ b/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs @@ -18,11 +18,32 @@ namespace NzbDrone.Mono.Test.DiskProviderTests [Platform(Exclude = "Win")] public class DiskProviderFixture : DiskProviderFixtureBase { + private string _tempPath; + public DiskProviderFixture() { PosixOnly(); } + [TearDown] + public void MonoDiskProviderFixtureTearDown() + { + // Give ourselves back write permissions so we can delete it + if (_tempPath != null) + { + if (Directory.Exists(_tempPath)) + { + Syscall.chmod(_tempPath, FilePermissions.S_IRWXU); + } + else if (File.Exists(_tempPath)) + { + Syscall.chmod(_tempPath, FilePermissions.S_IRUSR | FilePermissions.S_IWUSR); + } + + _tempPath = null; + } + } + protected override void SetWritePermissions(string path, bool writable) { if (Environment.UserName == "root") @@ -30,16 +51,41 @@ namespace NzbDrone.Mono.Test.DiskProviderTests Assert.Inconclusive("Need non-root user to test write permissions."); } + SetWritePermissionsInternal(path, writable, false); + } + + protected void SetWritePermissionsInternal(string path, bool writable, bool setgid) + { // 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); + Stat stat; + Syscall.stat(path, out stat); + FilePermissions mode = stat.st_mode; if (writable) { - entry.FileAccessPermissions |= FileAccessPermissions.UserWrite | FileAccessPermissions.GroupWrite | FileAccessPermissions.OtherWrite; + mode |= FilePermissions.S_IWUSR | FilePermissions.S_IWGRP | FilePermissions.S_IWOTH; + } + else + { + mode &= ~(FilePermissions.S_IWUSR | FilePermissions.S_IWGRP | FilePermissions.S_IWOTH); + } + + if (setgid) + { + mode |= FilePermissions.S_ISGID; } else { - entry.FileAccessPermissions &= ~(FileAccessPermissions.UserWrite | FileAccessPermissions.GroupWrite | FileAccessPermissions.OtherWrite); + mode &= ~FilePermissions.S_ISGID; + } + + if (stat.st_mode != mode) + { + if (Syscall.chmod(path, mode) < 0) + { + var error = Stdlib.GetLastError(); + throw new LinuxPermissionsException("Error setting group: " + error); + } } } @@ -165,24 +211,25 @@ namespace NzbDrone.Mono.Test.DiskProviderTests var tempFile = GetTempFilePath(); File.WriteAllText(tempFile, "File1"); - SetWritePermissions(tempFile, false); + SetWritePermissionsInternal(tempFile, false, false); + _tempPath = tempFile; // Verify test setup Syscall.stat(tempFile, out var fileStat); NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0444"); - Subject.SetPermissions(tempFile, "644"); + Subject.SetPermissions(tempFile, "755", null); Syscall.stat(tempFile, out fileStat); NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0644"); - Subject.SetPermissions(tempFile, "0644"); + Subject.SetPermissions(tempFile, "0755", null); Syscall.stat(tempFile, out fileStat); NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0644"); if (OsInfo.Os != Os.Bsd) { // This is not allowed on BSD - Subject.SetPermissions(tempFile, "1664"); + Subject.SetPermissions(tempFile, "1775", null); Syscall.stat(tempFile, out fileStat); NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("1664"); } @@ -194,62 +241,118 @@ namespace NzbDrone.Mono.Test.DiskProviderTests var tempPath = GetTempFilePath(); Directory.CreateDirectory(tempPath); - SetWritePermissions(tempPath, false); + SetWritePermissionsInternal(tempPath, false, false); + _tempPath = tempPath; // Verify test setup Syscall.stat(tempPath, out var fileStat); NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0555"); - Subject.SetPermissions(tempPath, "644"); + Subject.SetPermissions(tempPath, "755", null); Syscall.stat(tempPath, out fileStat); NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0755"); - Subject.SetPermissions(tempPath, "0644"); + Subject.SetPermissions(tempPath, "775", null); Syscall.stat(tempPath, out fileStat); - NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0755"); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0775"); - Subject.SetPermissions(tempPath, "1664"); + Subject.SetPermissions(tempPath, "750", null); Syscall.stat(tempPath, out fileStat); - NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("1775"); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0750"); - Subject.SetPermissions(tempPath, "775"); + Subject.SetPermissions(tempPath, "051", null); Syscall.stat(tempPath, out fileStat); - NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0775"); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0051"); + } + + [Test] + public void should_preserve_setgid_on_set_folder_permissions() + { + var tempPath = GetTempFilePath(); - Subject.SetPermissions(tempPath, "640"); + Directory.CreateDirectory(tempPath); + SetWritePermissionsInternal(tempPath, false, true); + _tempPath = tempPath; + + // Verify test setup + Syscall.stat(tempPath, out var fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("2555"); + + Subject.SetPermissions(tempPath, "755", null); Syscall.stat(tempPath, out fileStat); - NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0750"); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("2755"); - Subject.SetPermissions(tempPath, "0041"); + Subject.SetPermissions(tempPath, "775", null); Syscall.stat(tempPath, out fileStat); - NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0051"); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("2775"); - // reinstate sane permissions so fokder can be cleaned up - Subject.SetPermissions(tempPath, "775"); + Subject.SetPermissions(tempPath, "750", null); Syscall.stat(tempPath, out fileStat); - NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0775"); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("2750"); + + Subject.SetPermissions(tempPath, "051", null); + Syscall.stat(tempPath, out fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("2051"); } [Test] - public void IsValidFilePermissionMask_should_return_correct() + public void should_clear_setgid_on_set_folder_permissions() { - // Files may not be executable - Subject.IsValidFilePermissionMask("0777").Should().BeFalse(); - Subject.IsValidFilePermissionMask("0544").Should().BeFalse(); - Subject.IsValidFilePermissionMask("0454").Should().BeFalse(); - Subject.IsValidFilePermissionMask("0445").Should().BeFalse(); + var tempPath = GetTempFilePath(); + + Directory.CreateDirectory(tempPath); + SetWritePermissionsInternal(tempPath, false, true); + _tempPath = tempPath; + + // Verify test setup + Syscall.stat(tempPath, out var fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("2555"); + Subject.SetPermissions(tempPath, "0755", null); + Syscall.stat(tempPath, out fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0755"); + + Subject.SetPermissions(tempPath, "0775", null); + Syscall.stat(tempPath, out fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0775"); + + Subject.SetPermissions(tempPath, "0750", null); + Syscall.stat(tempPath, out fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0750"); + + Subject.SetPermissions(tempPath, "0051", null); + Syscall.stat(tempPath, out fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0051"); + } + + [Test] + public void IsValidFolderPermissionMask_should_return_correct() + { // No special bits should be set - Subject.IsValidFilePermissionMask("1644").Should().BeFalse(); - Subject.IsValidFilePermissionMask("2644").Should().BeFalse(); - Subject.IsValidFilePermissionMask("4644").Should().BeFalse(); - Subject.IsValidFilePermissionMask("7644").Should().BeFalse(); - - // Files should be readable and writeable by owner - Subject.IsValidFilePermissionMask("0400").Should().BeFalse(); - Subject.IsValidFilePermissionMask("0000").Should().BeFalse(); - Subject.IsValidFilePermissionMask("0200").Should().BeFalse(); - Subject.IsValidFilePermissionMask("0600").Should().BeTrue(); + Subject.IsValidFolderPermissionMask("1755").Should().BeFalse(); + Subject.IsValidFolderPermissionMask("2755").Should().BeFalse(); + Subject.IsValidFolderPermissionMask("4755").Should().BeFalse(); + Subject.IsValidFolderPermissionMask("7755").Should().BeFalse(); + + // Folder should be readable and writeable by owner + Subject.IsValidFolderPermissionMask("000").Should().BeFalse(); + Subject.IsValidFolderPermissionMask("100").Should().BeFalse(); + Subject.IsValidFolderPermissionMask("200").Should().BeFalse(); + Subject.IsValidFolderPermissionMask("300").Should().BeFalse(); + Subject.IsValidFolderPermissionMask("400").Should().BeFalse(); + Subject.IsValidFolderPermissionMask("500").Should().BeFalse(); + Subject.IsValidFolderPermissionMask("600").Should().BeFalse(); + Subject.IsValidFolderPermissionMask("700").Should().BeTrue(); + + // Folder should be readable and writeable by owner + Subject.IsValidFolderPermissionMask("0000").Should().BeFalse(); + Subject.IsValidFolderPermissionMask("0100").Should().BeFalse(); + Subject.IsValidFolderPermissionMask("0200").Should().BeFalse(); + Subject.IsValidFolderPermissionMask("0300").Should().BeFalse(); + Subject.IsValidFolderPermissionMask("0400").Should().BeFalse(); + Subject.IsValidFolderPermissionMask("0500").Should().BeFalse(); + Subject.IsValidFolderPermissionMask("0600").Should().BeFalse(); + Subject.IsValidFolderPermissionMask("0700").Should().BeTrue(); } } } diff --git a/src/NzbDrone.Mono.Test/DiskProviderTests/FreeSpaceFixture.cs b/src/NzbDrone.Mono.Test/DiskProviderTests/FreeSpaceFixture.cs index 619194cdd..b1c518c90 100644 --- a/src/NzbDrone.Mono.Test/DiskProviderTests/FreeSpaceFixture.cs +++ b/src/NzbDrone.Mono.Test/DiskProviderTests/FreeSpaceFixture.cs @@ -1,3 +1,5 @@ +using FluentAssertions; +using Moq; using NUnit.Framework; using NzbDrone.Common.Test.DiskTests; using NzbDrone.Mono.Disk; @@ -8,9 +10,20 @@ namespace NzbDrone.Mono.Test.DiskProviderTests [Platform(Exclude = "Win")] public class FreeSpaceFixture : FreeSpaceFixtureBase { - public FreeSpaceFixture() + [SetUp] + public void Setup() { - PosixOnly(); + Mocker.SetConstant(new ProcMountProvider(TestLogger)); + Mocker.GetMock() + .Setup(v => v.GetCompleteRealPath(It.IsAny())) + .Returns(s => s); + } + + [Ignore("Docker")] + [Test] + public void should_be_able_to_check_space_on_ramdrive() + { + Subject.GetAvailableSpace("/run/").Should().NotBe(0); } } } diff --git a/src/NzbDrone.Mono/Disk/DiskProvider.cs b/src/NzbDrone.Mono/Disk/DiskProvider.cs index 6ea2549dd..e18332124 100644 --- a/src/NzbDrone.Mono/Disk/DiskProvider.cs +++ b/src/NzbDrone.Mono/Disk/DiskProvider.cs @@ -61,15 +61,41 @@ namespace NzbDrone.Mono.Disk { } - public override void SetPermissions(string path, string mask) + public override void SetFilePermissions(string path, string mask, string group) { - _logger.Debug("Setting permissions: {0} on {1}", mask, path); + var permissions = NativeConvert.FromOctalPermissionString(mask); + + SetPermissions(path, mask, group, permissions); + } + public override void SetPermissions(string path, string mask, string group) + { var permissions = NativeConvert.FromOctalPermissionString(mask); - if (Directory.Exists(path)) + if (File.Exists(path)) { - permissions = GetFolderPermissions(permissions); + permissions = GetFilePermissions(permissions); + } + + SetPermissions(path, mask, group, permissions); + } + + protected void SetPermissions(string path, string mask, string group, FilePermissions permissions) + { + _logger.Debug("Setting permissions: {0} on {1}", mask, path); + + // Preserve non-access permissions + if (Syscall.stat(path, out var curStat) < 0) + { + var error = Stdlib.GetLastError(); + + throw new LinuxPermissionsException("Error getting current permissions: " + error); + } + + // Preserve existing non-access permissions unless mask is 4 digits + if (mask.Length < 4) + { + permissions |= curStat.st_mode & ~FilePermissions.ACCESSPERMS; } if (Syscall.chmod(path, permissions) < 0) @@ -78,33 +104,39 @@ namespace NzbDrone.Mono.Disk throw new LinuxPermissionsException("Error setting permissions: " + error); } + + var groupId = GetGroupId(group); + + if (Syscall.chown(path, unchecked((uint)-1), groupId) < 0) + { + var error = Stdlib.GetLastError(); + + throw new LinuxPermissionsException("Error setting group: " + error); + } } - private static FilePermissions GetFolderPermissions(FilePermissions permissions) + private static FilePermissions GetFilePermissions(FilePermissions permissions) { - permissions |= (FilePermissions)((int)(permissions & (FilePermissions.S_IRUSR | FilePermissions.S_IRGRP | FilePermissions.S_IROTH)) >> 2); + permissions &= ~(FilePermissions.S_IXUSR | FilePermissions.S_IXGRP | FilePermissions.S_IXOTH); return permissions; } - public override bool IsValidFilePermissionMask(string mask) + public override bool IsValidFolderPermissionMask(string mask) { try { var permissions = NativeConvert.FromOctalPermissionString(mask); - if ((permissions & (FilePermissions.S_ISUID | FilePermissions.S_ISGID | FilePermissions.S_ISVTX)) != 0) - { - return false; - } - - if ((permissions & (FilePermissions.S_IXUSR | FilePermissions.S_IXGRP | FilePermissions.S_IXOTH)) != 0) + if ((permissions & ~FilePermissions.ACCESSPERMS) != 0) { + // Only allow access permissions return false; } - if ((permissions & (FilePermissions.S_IRUSR | FilePermissions.S_IWUSR)) != (FilePermissions.S_IRUSR | FilePermissions.S_IWUSR)) + if ((permissions & FilePermissions.S_IRWXU) != FilePermissions.S_IRWXU) { + // We expect at least full owner permissions (700) return false; } @@ -281,9 +313,7 @@ namespace NzbDrone.Mono.Disk // Catch the exception and attempt to handle these edgecases // Mono 6.x till 6.10 doesn't properly try use rename first. - if (move && - ((PlatformInfo.Platform == PlatformType.Mono && PlatformInfo.GetVersion() < new Version(6, 10)) || - (PlatformInfo.Platform == PlatformType.NetCore))) + if (move && (PlatformInfo.Platform == PlatformType.NetCore)) { if (Syscall.lstat(source, out var sourcestat) == 0 && Syscall.lstat(destination, out var deststat) != 0 && @@ -311,32 +341,7 @@ namespace NzbDrone.Mono.Disk var dstInfo = new FileInfo(destination); var exists = dstInfo.Exists && srcInfo.Exists; - if (PlatformInfo.Platform == PlatformType.Mono && PlatformInfo.GetVersion() >= new Version(6, 6) && - exists && dstInfo.Length == 0 && srcInfo.Length != 0) - { - // mono >=6.6 bug: zero length file since chmod happens at the start - _logger.Debug("{3} failed to {2} file likely due to known {3} bug, attempting to {2} directly. '{0}' -> '{1}'", source, destination, move ? "move" : "copy", PlatformInfo.PlatformName); - - try - { - _logger.Trace("Copying content from {0} to {1} ({2} bytes)", source, destination, srcInfo.Length); - using (var srcStream = new FileStream(source, FileMode.Open, FileAccess.Read)) - using (var dstStream = new FileStream(destination, FileMode.Create, FileAccess.Write)) - { - srcStream.CopyTo(dstStream); - } - } - catch - { - // If it fails again then bail - throw; - } - } - else if (((PlatformInfo.Platform == PlatformType.Mono && - PlatformInfo.GetVersion() >= new Version(6, 0) && - PlatformInfo.GetVersion() < new Version(6, 6)) || - PlatformInfo.Platform == PlatformType.NetCore) && - exists && dstInfo.Length == srcInfo.Length) + if (PlatformInfo.Platform == PlatformType.NetCore && exists && dstInfo.Length == srcInfo.Length) { // mono 6.0, mono 6.4 and netcore 3.1 bug: full length file since utime and chmod happens at the end _logger.Debug("{3} failed to {2} file likely due to known {3} bug, attempting to {2} directly. '{0}' -> '{1}'", source, destination, move ? "move" : "copy", PlatformInfo.PlatformName); diff --git a/src/NzbDrone.Mono/Disk/FindDriveType.cs b/src/NzbDrone.Mono/Disk/FindDriveType.cs index a01c6c41f..d0481c3d4 100644 --- a/src/NzbDrone.Mono/Disk/FindDriveType.cs +++ b/src/NzbDrone.Mono/Disk/FindDriveType.cs @@ -12,6 +12,7 @@ namespace NzbDrone.Mono.Disk { "apfs", DriveType.Fixed }, { "fuse.mergerfs", DriveType.Fixed }, { "fuse.glusterfs", DriveType.Network }, + { "nullfs", DriveType.Fixed }, { "zfs", DriveType.Fixed } }; diff --git a/src/NzbDrone.Update/UpdateEngine/InstallUpdateService.cs b/src/NzbDrone.Update/UpdateEngine/InstallUpdateService.cs index c44820c50..4ae927c4a 100644 --- a/src/NzbDrone.Update/UpdateEngine/InstallUpdateService.cs +++ b/src/NzbDrone.Update/UpdateEngine/InstallUpdateService.cs @@ -90,6 +90,12 @@ namespace NzbDrone.Update.UpdateEngine Verify(installationFolder, processId); + if (installationFolder.EndsWith(@"\bin\Prowlarr") || installationFolder.EndsWith(@"/bin/Prowlarr")) + { + installationFolder = installationFolder.GetParentPath(); + _logger.Info("Fixed Installation Folder: {0}", installationFolder); + } + var appType = _detectApplicationType.GetAppType(); _processProvider.FindProcessByName(ProcessProvider.PROWLARR_CONSOLE_PROCESS_NAME); @@ -125,7 +131,7 @@ namespace NzbDrone.Update.UpdateEngine // Set executable flag on app if (OsInfo.IsOsx || (OsInfo.IsLinux && PlatformInfo.IsNetCore)) { - _diskProvider.SetPermissions(Path.Combine(installationFolder, "Prowlarr"), "0755"); + _diskProvider.SetPermissions(Path.Combine(installationFolder, "Prowlarr"), "0755", null); } } catch (Exception e) @@ -146,7 +152,7 @@ namespace NzbDrone.Update.UpdateEngine _terminateNzbDrone.Terminate(processId); _logger.Info("Waiting for external auto-restart."); - for (int i = 0; i < 5; i++) + for (int i = 0; i < 10; i++) { System.Threading.Thread.Sleep(1000); diff --git a/src/NzbDrone.Windows.Test/DiskProviderTests/FreeSpaceFixture.cs b/src/NzbDrone.Windows.Test/DiskProviderTests/FreeSpaceFixture.cs index c68de2451..fdb0b0988 100644 --- a/src/NzbDrone.Windows.Test/DiskProviderTests/FreeSpaceFixture.cs +++ b/src/NzbDrone.Windows.Test/DiskProviderTests/FreeSpaceFixture.cs @@ -1,4 +1,6 @@ -using NUnit.Framework; +using System.IO; +using FluentAssertions; +using NUnit.Framework; using NzbDrone.Common.Test.DiskTests; using NzbDrone.Windows.Disk; @@ -12,5 +14,30 @@ namespace NzbDrone.Windows.Test.DiskProviderTests { WindowsOnly(); } + + [Test] + public void should_throw_if_drive_doesnt_exist() + { + // Find a drive that doesn't exist. + for (char driveletter = 'Z'; driveletter > 'D'; driveletter--) + { + if (new DriveInfo(driveletter.ToString()).IsReady) + { + continue; + } + + Assert.Throws(() => Subject.GetAvailableSpace(driveletter + @":\NOT_A_REAL_PATH\DOES_NOT_EXIST")); + return; + } + + Assert.Inconclusive("No drive available for testing."); + } + + [Test] + public void should_be_able_to_get_space_on_unc() + { + var result = Subject.GetAvailableSpace(@"\\localhost\c$\Windows"); + result.Should().BeGreaterThan(0); + } } } diff --git a/src/NzbDrone.Windows/Disk/DiskProvider.cs b/src/NzbDrone.Windows/Disk/DiskProvider.cs index 943058a6e..d8b9dc448 100644 --- a/src/NzbDrone.Windows/Disk/DiskProvider.cs +++ b/src/NzbDrone.Windows/Disk/DiskProvider.cs @@ -91,7 +91,11 @@ namespace NzbDrone.Windows.Disk } } - public override void SetPermissions(string path, string mask) + public override void SetFilePermissions(string path, string mask, string group) + { + } + + public override void SetPermissions(string path, string mask, string group) { }