diff --git a/Duplicati/Library/Main/Operation/Restore/FileProcessor.cs b/Duplicati/Library/Main/Operation/Restore/FileProcessor.cs index 61662b2e2..fcb9f0266 100644 --- a/Duplicati/Library/Main/Operation/Restore/FileProcessor.cs +++ b/Duplicati/Library/Main/Operation/Restore/FileProcessor.cs @@ -87,24 +87,61 @@ namespace Duplicati.Library.Main.Operation.Restore sw_file?.Stop(); // Get information about the blocks for the file + // TODO rather than keeping all of the blocks in memory, we could do a single pass over the blocks using a cursor, only keeping the relevant block requests in memory. Maybe even only a single block request at a time. sw_block?.Start(); var blocks = db.GetBlocksFromFile(file.BlocksetID).ToArray(); sw_block?.Stop(); sw_work?.Start(); // Verify the target file blocks that may already exist. - var (bytes_written, missing_blocks) = await VerifyTargetBlocks(file, blocks, filehasher, blockhasher, options, results, block_request); - - if (missing_blocks.Count > 0 && options.UseLocalBlocks && missing_blocks.Count > 0) + var (bytes_verified, missing_blocks, verified_blocks) = await VerifyTargetBlocks(file, blocks, filehasher, blockhasher, options, results, block_request); + long bytes_written = 0; + if (blocks.Length != missing_blocks.Count + verified_blocks.Count) { - // Verify the local blocks at the original restore path that may be used to restore the file. - var (bw, new_missing_blocks) = await VerifyLocalBlocks(file, missing_blocks, blocks.Length, filehasher, blockhasher, options, results, block_request); - bytes_written = bw; - missing_blocks = new_missing_blocks; + Logging.Log.WriteErrorMessage(LOGTAG, "BlockCountMismatch", null, $"Block count mismatch for {file.TargetPath} - expected: {blocks.Length}, actual: {missing_blocks.Count + verified_blocks.Count}"); + sw_work?.Stop(); + continue; } + // Check if the target file needs to be retargeted + if (missing_blocks.Count > 0 && !options.Overwrite && SystemIO.IO_OS.FileExists(file.TargetPath)) + { + var new_name = GenerateNewName(file, db, filehasher); + if (SystemIO.IO_OS.FileExists(new_name)) + { + // The file already exists, which it only does when it matches the target hash. So we can skip the file. + Logging.Log.WriteInformationMessage(LOGTAG, "FileAlreadyExists", null, $"File {file.TargetPath} already exists and matches the target hash as a copy: {new_name}"); + missing_blocks.Clear(); + } + else + { + Logging.Log.WriteVerboseMessage(LOGTAG, "RetargetingFile", "Retargeting file {0} to {1}", file.TargetPath, new_name); + var new_file = new FileRequest(file.ID, file.OriginalPath, new_name, file.Hash, file.Length, file.BlocksetID); + if (options.UseLocalBlocks) + { + if (options.Dryrun) + { + Logging.Log.WriteDryrunMessage(LOGTAG, "DryrunRestore", @$"Would have copied {verified_blocks.Count} blocks ({verified_blocks.Count * options.Blocksize} bytes) from ""{file.TargetPath}"" to ""{new_file.TargetPath}"""); + } + else + { + CopyOldTargetBlocksToNewTarget(file, new_file, verified_blocks); + } + } + file = new_file; + } + } + + if (missing_blocks.Count > 0 && options.UseLocalBlocks) + { + // Verify the local blocks at the original restore path that may be used to restore the file. + (bytes_written, missing_blocks) = await VerifyLocalBlocks(file, missing_blocks, blocks.Length, filehasher, blockhasher, options, results, block_request); + } + + bool empty_file_or_symlink = false; if (file.BlocksetID != LocalDatabase.SYMLINK_BLOCKSET_ID && (blocks.Length == 0 || (blocks.Length == 1 && blocks[0].BlockSize == 0))) { + empty_file_or_symlink = true; if (options.Dryrun) { Logging.Log.WriteDryrunMessage(LOGTAG, "DryrunRestore", @$"Would have created empty file ""{file.TargetPath}"""); @@ -268,16 +305,22 @@ namespace Duplicati.Library.Main.Operation.Restore } if (!options.SkipMetadata) { - await RestoreMetadata(db, file, block_request, block_response, options, sw_meta, sw_work, sw_req, sw_resp); + empty_file_or_symlink |= await RestoreMetadata(db, file, block_request, block_response, options, sw_meta, sw_work, sw_req, sw_resp); } - // Keep track of the restored files and their sizes - lock (results) + // TODO legacy restore doesn't count metadata restore as a restored file. + if (empty_file_or_symlink || bytes_written > 0) { - results.RestoredFiles++; - results.SizeOfRestoredFiles += bytes_written; + // Keep track of the restored files and their sizes + lock (results) + { + results.RestoredFiles++; + results.SizeOfRestoredFiles += bytes_written; + } } sw_work?.Stop(); + + Logging.Log.WriteVerboseMessage(LOGTAG, "RestoredFile", "Restored file {0}", file.TargetPath); } } catch (RetiredException) @@ -302,6 +345,88 @@ namespace Duplicati.Library.Main.Operation.Restore }); } + /// + /// Copies the blocks that were verified in the old target to the new target file. + /// + /// The old target file. + /// The new target file. + /// The blocks in the old file that were verified. + private static void CopyOldTargetBlocksToNewTarget(FileRequest old_file, FileRequest new_file, List verified_blocks) + { + using var fs_old = SystemIO.IO_OS.FileOpenRead(old_file.TargetPath); + using var fs_new = SystemIO.IO_OS.FileOpenWrite(new_file.TargetPath); + + foreach (var block in verified_blocks) + { + fs_old.Seek(block.BlockOffset * block.BlockSize, SeekOrigin.Begin); + fs_new.Seek(block.BlockOffset * block.BlockSize, SeekOrigin.Begin); + + var buffer = new byte[block.BlockSize]; + fs_old.Read(buffer, 0, buffer.Length); + fs_new.Write(buffer, 0, buffer.Length); + } + } + + /// + /// Generates a new name for the target file. It starts by appending + /// the restore time to the file name. If that name is also already + /// taken, it appends a number to the name. It keeps incrementing the + /// number until it finds a name that is not taken. If it encounters a + /// file with the same name and the correct hash, it will retarget the + /// filename to that file, assuming that the file is the correct one. + /// + /// The original target file. + /// The restore database. + /// The file hasher used to verify whether any of the + /// The new filename. If the file already exists, its file hash matches the target hash. + private static string GenerateNewName(FileRequest request, LocalRestoreDatabase database, System.Security.Cryptography.HashAlgorithm filehasher) + { + var ext = SystemIO.IO_OS.PathGetExtension(request.TargetPath) ?? ""; + if (!string.IsNullOrEmpty(ext) && !ext.StartsWith(".", StringComparison.Ordinal)) + ext = "." + ext; + + // First we try with a simple date append, assuming that there are not many conflicts there + var newname = SystemIO.IO_OS.PathChangeExtension(request.TargetPath, null) + "." + database.RestoreTime.ToLocalTime().ToString("yyyy-MM-dd", System.Globalization.CultureInfo.InvariantCulture); + var tr = newname + ext; + var c = 0; + var max_tries = 100; + while (SystemIO.IO_OS.FileExists(tr) && c < max_tries) + { + try + { + // If we have a file with the correct name, + // it is most likely the file we want + filehasher.Initialize(); + + string key; + using (var file = SystemIO.IO_OS.FileOpenRead(tr)) + key = Convert.ToBase64String(filehasher.ComputeHash(file)); + + if (key == request.Hash) + { + //TODO: Also needs metadata check to make correct decision. + // We stick to the policy to restore metadata in place, if data ok. So, metadata block may be restored. + break; + } + } + catch (Exception ex) + { + Logging.Log.WriteWarningMessage(LOGTAG, "FailedToReadRestoreTarget", ex, "Failed to read candidate restore target {0}", tr); + } + tr = newname + " (" + (c++).ToString() + ")" + ext; + } + + if (c >= max_tries) + { + Logging.Log.WriteErrorMessage(LOGTAG, "TooManyConflicts", null, "Too many conflicts when trying to find a new name for the target file"); + throw new Exception("Too many conflicts when trying to find a new name for the target file"); + } + + newname = tr; + + return newname; + } + /// /// Restores the metadata for a file. /// @@ -314,7 +439,7 @@ namespace Duplicati.Library.Main.Operation.Restore /// The stopwatch for internal profiling of the general processing. /// The stopwatch for internal profiling of the block requests. /// The stopwatch for internal profiling of the block responses. - private static async Task RestoreMetadata(LocalRestoreDatabase db, FileRequest file, IChannel block_request, IChannel block_response, Options options, Stopwatch sw_meta, Stopwatch sw_work, Stopwatch sw_req, Stopwatch sw_resp) + private static async Task RestoreMetadata(LocalRestoreDatabase db, FileRequest file, IChannel block_request, IChannel block_response, Options options, Stopwatch sw_meta, Stopwatch sw_work, Stopwatch sw_req, Stopwatch sw_resp) { sw_work?.Stop(); sw_meta?.Start(); @@ -338,7 +463,7 @@ namespace Duplicati.Library.Main.Operation.Restore } ms.Seek(0, SeekOrigin.Begin); - RestoreHandler.ApplyMetadata(file.TargetPath, ms, options.RestorePermissions, options.RestoreSymlinkMetadata, options.Dryrun); + return RestoreHandler.ApplyMetadata(file.TargetPath, ms, options.RestorePermissions, options.RestoreSymlinkMetadata, options.Dryrun); } /// @@ -352,10 +477,11 @@ namespace Duplicati.Library.Main.Operation.Restore /// The restoration results. /// The channel to request blocks from the block manager. Used to inform the block manager which blocks are already present. /// An awaitable `Task`, which returns a collection of data blocks that are missing. - private static async Task<(long,List)> VerifyTargetBlocks(FileRequest file, BlockRequest[] blocks, System.Security.Cryptography.HashAlgorithm filehasher, System.Security.Cryptography.HashAlgorithm blockhasher, Options options, RestoreResults results, IChannel block_request) + private static async Task<(long,List,List)> VerifyTargetBlocks(FileRequest file, BlockRequest[] blocks, System.Security.Cryptography.HashAlgorithm filehasher, System.Security.Cryptography.HashAlgorithm blockhasher, Options options, RestoreResults results, IChannel block_request) { long bytes_read = 0; List missing_blocks = []; + List verified_blocks = []; // Check if the file exists if (File.Exists(file.TargetPath)) @@ -379,6 +505,7 @@ namespace Duplicati.Library.Main.Operation.Restore bytes_read += read; blocks[i].CacheDecrEvict = true; await block_request.WriteAsync(blocks[i]); + verified_blocks.Add(blocks[i]); } else { @@ -439,7 +566,7 @@ namespace Duplicati.Library.Main.Operation.Restore missing_blocks.AddRange(blocks); } - return (bytes_read, missing_blocks); + return (bytes_read, missing_blocks, verified_blocks); } /// @@ -457,6 +584,7 @@ namespace Duplicati.Library.Main.Operation.Restore private static async Task<(long, List)> VerifyLocalBlocks(FileRequest file, List blocks, long total_blocks, System.Security.Cryptography.HashAlgorithm filehasher, System.Security.Cryptography.HashAlgorithm blockhasher, Options options, RestoreResults results, IChannel block_request) { List missing_blocks = []; + List verified_blocks = []; // Check if the file exists if (File.Exists(file.OriginalPath)) @@ -465,7 +593,11 @@ namespace Duplicati.Library.Main.Operation.Restore // Open both files, as the target file is still being read to produce the overall file hash, if all the blocks are present across both the target and original files. using var f_original = SystemIO.IO_OS.FileOpenRead(file.OriginalPath); - using var f_target = SystemIO.IO_OS.FileOpenWrite(file.TargetPath); + using var f_target = options.Dryrun ? + (SystemIO.IO_OS.FileExists(file.TargetPath) ? + SystemIO.IO_OS.FileOpenRead(file.TargetPath) : + null) : + SystemIO.IO_OS.FileOpenWrite(file.TargetPath); var buffer = new byte[options.Blocksize]; long bytes_read = 0; long bytes_written = 0; @@ -499,6 +631,8 @@ namespace Duplicati.Library.Main.Operation.Restore missing_blocks.Add(blocks[j]); } else + { + if (!options.Dryrun) { try { @@ -512,11 +646,13 @@ namespace Duplicati.Library.Main.Operation.Restore results.BrokenLocalFiles.Add(file.TargetPath); } throw; + } } bytes_read += read; bytes_written += read; blocks[j].CacheDecrEvict = true; await block_request.WriteAsync(blocks[j]); + verified_blocks.Add(blocks[j]); } } else @@ -536,8 +672,8 @@ namespace Duplicati.Library.Main.Operation.Restore // The current block is not a missing block - read from the target file. try { - f_target.Seek(i * options.Blocksize, SeekOrigin.Begin); - read = await f_target.ReadAsync(buffer, 0, options.Blocksize); + f_target?.Seek(i * options.Blocksize, SeekOrigin.Begin); + read = await f_target?.ReadAsync(buffer, 0, options.Blocksize); } catch (Exception) { @@ -558,7 +694,7 @@ namespace Duplicati.Library.Main.Operation.Restore if (Convert.ToBase64String(filehasher.Hash) == file.Hash) { // Truncate the file if it is larger than the expected size. - if (file.Length < f_target.Length) + if (file.Length < f_target?.Length) { if (options.Dryrun) { @@ -591,6 +727,11 @@ namespace Duplicati.Library.Main.Operation.Restore } } + if (options.Dryrun) + { + Logging.Log.WriteDryrunMessage(LOGTAG, "DryrunRestore", @$"Would have restored {verified_blocks.Count} blocks ({bytes_written} bytes) from ""{file.OriginalPath}"" to ""{file.TargetPath}"""); + } + return (bytes_written, missing_blocks); } else diff --git a/Duplicati/Library/Main/Operation/RestoreHandler.cs b/Duplicati/Library/Main/Operation/RestoreHandler.cs index a3cd7d3db..766b4e1a1 100644 --- a/Duplicati/Library/Main/Operation/RestoreHandler.cs +++ b/Duplicati/Library/Main/Operation/RestoreHandler.cs @@ -607,8 +607,9 @@ namespace Duplicati.Library.Main.Operation m_result.EndTime = DateTime.UtcNow; } - public static void ApplyMetadata(string path, System.IO.Stream stream, bool restorePermissions, bool restoreSymlinkMetadata, bool dryrun) + public static bool ApplyMetadata(string path, System.IO.Stream stream, bool restorePermissions, bool restoreSymlinkMetadata, bool dryrun) { + // TODO This has been modified to return a bool indicating if anything was written to properly report the number of files restored. The legacy restore doesn't check this, which produces an error in the CI where it reports that no files have been restored, even though one have. It's in Duplicati/UnitTests/SymLinkTests.cs the test SymLinkTests.SymLinkExists() that fails on the very last assert that there are 0 warnings. using (var tr = new System.IO.StreamReader(stream)) using (var jr = new Newtonsoft.Json.JsonTextReader(tr)) { @@ -616,10 +617,11 @@ namespace Duplicati.Library.Main.Operation string k; long t; System.IO.FileAttributes fa; + var wrote_something = false; // If this is dry-run, we stop after having deserialized the metadata if (dryrun) - return; + return wrote_something; var isDirTarget = path.EndsWith(DIRSEP, StringComparison.Ordinal); var targetpath = isDirTarget ? path.Substring(0, path.Length - 1) : path; @@ -637,6 +639,7 @@ namespace Duplicati.Library.Main.Operation SystemIO.IO_OS.DirectoryDelete(targetpath, false); } SystemIO.IO_OS.CreateSymlink(targetpath, k, isDirTarget); + wrote_something = true; } // If the target is a folder, make sure we create it first else if (isDirTarget && !SystemIO.IO_OS.DirectoryExists(targetpath)) @@ -646,7 +649,7 @@ namespace Duplicati.Library.Main.Operation if (!restoreSymlinkMetadata && Snapshots.SnapshotUtility.IsSymlink(SystemIO.IO_OS, targetpath)) { Logging.Log.WriteVerboseMessage(LOGTAG, "no-symlink-metadata-restored", "Not applying metadata to symlink: {0}", targetpath); - return; + return wrote_something; } if (metadata.TryGetValue("CoreLastWritetime", out k) && long.TryParse(k, out t)) @@ -669,6 +672,7 @@ namespace Duplicati.Library.Main.Operation SystemIO.IO_OS.SetFileAttributes(targetpath, fa); SystemIO.IO_OS.SetMetadata(path, metadata, restorePermissions); + return wrote_something; } } diff --git a/Duplicati/UnitTest/IssueTests.cs b/Duplicati/UnitTest/IssueTests.cs index aada62bea..91d995fe2 100644 --- a/Duplicati/UnitTest/IssueTests.cs +++ b/Duplicati/UnitTest/IssueTests.cs @@ -118,6 +118,141 @@ namespace Duplicati.UnitTest } + [Test] + [Category("Restore"), Category("Bug")] + public void Issue5825RestoreNoOverwrite([Values] bool legacy, [Values] bool local_blocks) + { + // Reproduction of Issue #5825 + // The logic in the previous version was to create a timestamped version of the file being restored to, if it already exists. + // It appears the new restore flow is not correctly doing the same. + // See forum thread: https://forum.duplicati.com/t/save-different-versions-with-timestamp-in-file-name-broken/19805 + + var testopts = new Dictionary(TestOptions) + { + ["restore-legacy"] = legacy.ToString().ToLower(), + ["restore-with-local-blocks"] = local_blocks.ToString().ToLower() + }; + int test_filesize = 1024; + + // TODO tjek om de gamle filer forbliver urørte. + + var original_dir = Path.Combine(DATAFOLDER, "some_original_dir"); + Directory.CreateDirectory(original_dir); + string f0 = Path.Combine(original_dir, "some_file"); + TestUtils.WriteTestFile(f0, test_filesize); + + // Backup the files + using (var c = new Library.Main.Controller("file://" + TARGETFOLDER, testopts, null)) + { + IBackupResults backupResults = c.Backup([DATAFOLDER]); + TestUtils.AssertResults(backupResults); + } + + // Attempt to restore the file + testopts["restore-path"] = RESTOREFOLDER; + using (var c = new Library.Main.Controller("file://" + TARGETFOLDER, testopts, null)) + { + var restoreResults = c.Restore([f0]); + Assert.That(restoreResults.RestoredFiles, Is.EqualTo(1), "File should have been restored"); + } + + // Verify that the files are equal + string f1 = Path.Combine(RESTOREFOLDER, "some_file"); + Assert.That(File.ReadAllBytes(f0), Is.EqualTo(File.ReadAllBytes(f1)), "Restored file should be equal to original file"); + + // Modify the restored file + TestUtils.WriteTestFile(f1, test_filesize); + + // Restore the file again, with overwrite. + testopts["overwrite"] = "true"; + using (var c = new Library.Main.Controller("file://" + TARGETFOLDER, testopts, null)) + { + var restoreResults = c.Restore([f0]); + Assert.That(restoreResults.RestoredFiles, Is.EqualTo(1), "File should have been restored"); + } + + // Verify that the files are equal + Assert.That(File.ReadAllBytes(f0), Is.EqualTo(File.ReadAllBytes(f1)), "Restored file should be equal to original file"); + + // Save the timestamp of the file + var timestamp = File.GetLastWriteTime(f1); + + // Touch the file + File.SetLastWriteTime(f1, DateTime.Now); + + // Check that the timestamp has changed, but the file is still equal + Assert.That(File.GetLastWriteTime(f1), Is.Not.EqualTo(timestamp), "Timestamp should have changed"); + Assert.That(File.ReadAllBytes(f0), Is.EqualTo(File.ReadAllBytes(f1)), "Restored file should be equal to original file"); + + // Restore the file again, with overwrite, should restore the timestamp of the file + using (var c = new Library.Main.Controller("file://" + TARGETFOLDER, testopts, null)) + { + var restoreResults = c.Restore([f0]); + Assert.That(restoreResults.RestoredFiles, Is.EqualTo(0), "File should not have been restored, only the metadata."); + } + + // Verify that the files are equal and the timestamp is restored + Assert.That(File.ReadAllBytes(f0), Is.EqualTo(File.ReadAllBytes(f1)), "Restored file should be equal to original file"); + Assert.That(File.GetLastWriteTime(f1), Is.EqualTo(timestamp), "Timestamp should be restored"); + + // Modify the restored file + TestUtils.WriteTestFile(f1, test_filesize); + var f1_original = File.ReadAllBytes(f1); + + // Restore the file again, without overwrite. + testopts["overwrite"] = "false"; + using (var c = new Library.Main.Controller("file://" + TARGETFOLDER, testopts, null)) + { + var restoreResults = c.Restore([f0]); + Assert.That(restoreResults.RestoredFiles, Is.EqualTo(1), "File should have been restored"); + } + + // Verify that f1 is still the same + Assert.That(File.ReadAllBytes(f1), Is.EqualTo(f1_original), "The first restored file should remain untouched"); + + // Verify that there exists a new file with a timestamp + var files = Directory.GetFiles(RESTOREFOLDER, "*", SearchOption.TopDirectoryOnly); + Assert.That(files.Length, Is.EqualTo(2), "There should be two files in the folder"); + var f2 = files.FirstOrDefault(v => v != f1); + + // Modify the new restored file as well + TestUtils.WriteTestFile(f2, test_filesize); + var f2_original = File.ReadAllBytes(f2); + + // Restore the file again, without overwrite. + using (var c = new Library.Main.Controller("file://" + TARGETFOLDER, testopts, null)) + { + var restoreResults = c.Restore([f0]); + Assert.That(restoreResults.RestoredFiles, Is.EqualTo(1), "File should have been restored"); + } + + // Verify that f1 and f2 are still the same + Assert.That(File.ReadAllBytes(f1), Is.EqualTo(f1_original), "The first restored file should remain untouched"); + Assert.That(File.ReadAllBytes(f2), Is.EqualTo(f2_original), "The second restored file should remain untouched"); + + // Verify that there exists a new file with a timestamp + files = Directory.GetFiles(RESTOREFOLDER, "*", SearchOption.TopDirectoryOnly); + Assert.That(files.Length, Is.EqualTo(3), "There should be three files in the folder"); + + // Verify that the files are equal + var f3 = files.FirstOrDefault(v => v != f1 && v != f2); + Assert.That(File.ReadAllBytes(f0), Is.EqualTo(File.ReadAllBytes(f3)), "Restored file should be equal to original file"); + + // Modify the second file to match the original file - should not restore any files + File.WriteAllBytes(f2, File.ReadAllBytes(f0)); + + // Restore the file again, without overwrite. + using (var c = new Library.Main.Controller("file://" + TARGETFOLDER, testopts, null)) + { + var restoreResults = c.Restore([f0]); + Assert.That(restoreResults.RestoredFiles, Is.EqualTo(0), "File should not have been restored"); + } + + // Verify that f1 is still untouched + Assert.That(File.ReadAllBytes(f1), Is.EqualTo(f1_original), "The first restored file should remain untouched"); + } + + [Test] [Category("Restore"), Category("Bug")] public void Issue5826RestoreMissingFolder([Values] bool compressRestorePaths, [Values] bool restoreLegacy)