Bug 1592: When connection is lost while deleting source remote file during "Download and Delete" operation, downloaded file may be deleted once connection is resumed

https://winscp.net/tracker/1592

Source commit: 99e7f9d9cd841848d00afdb002d2ef898a402395

source/core/Terminal.cpp

@@ -7100,19 +7100,35 @@ void __fastcall TTerminal::SinkRobust(
   TDownloadSessionAction Action(ActionLog);
   bool * AFileTransferAny = FLAGSET(Flags, tfUseFileTransferAny) ? &FFileTransferAny : NULL;
   TRobustOperationLoop RobustLoop(this, OperationProgress, AFileTransferAny);
+  bool Sunk = false;
 
   do
   {
-    bool ChildError = false;
     try
     {
-      Sink(FileName, File, TargetDir, CopyParam, Params, OperationProgress, Flags, Action, ChildError);
+      // If connection is lost while deleting the file, so not retry download on the next round.
+      // The file may not exist anymore and the download attempt might overwrite the (only) local copy.
+      if (!Sunk)
+      {
+        Sink(FileName, File, TargetDir, CopyParam, Params, OperationProgress, Flags, Action);
+        Sunk = true;
+      }
+
+      if (FLAGSET(Params, cpDelete))
+      {
+        DebugAssert(FLAGCLEAR(Params, cpNoRecurse));
+        // If file is directory, do not delete it recursively, because it should be
+        // empty already. If not, it should not be deleted (some files were
+        // skipped or some new files were copied to it, while we were downloading)
+        int Params = dfNoRecursive;
+        DeleteFile(FileName, File, &Params);
+      }
     }
     catch (Exception & E)
     {
       if (!RobustLoop.TryReopen(E))
       {
-        if (!ChildError)
+        if (!Sunk)
         {
           RollbackAction(Action, OperationProgress, &E);
         }
@@ -7149,7 +7165,7 @@ struct TSinkFileParams
 void __fastcall TTerminal::Sink(
   const UnicodeString & FileName, const TRemoteFile * File, const UnicodeString & TargetDir,
   const TCopyParamType * CopyParam, int Params, TFileOperationProgressType * OperationProgress, unsigned int Flags,
-  TDownloadSessionAction & Action, bool & ChildError)
+  TDownloadSessionAction & Action)
 {
   Action.FileName(FileName);
 
@@ -7256,18 +7272,6 @@ void __fastcall TTerminal::Sink(
 
     LogFileDone(OperationProgress, ExpandUNCFileName(DestFullName));
   }
-
-  if (FLAGSET(Params, cpDelete))
-  {
-    DebugAssert(FLAGCLEAR(Params, cpNoRecurse));
-    ChildError = true;
-    // If file is directory, do not delete it recursively, because it should be
-    // empty already. If not, it should not be deleted (some files were
-    // skipped or some new files were copied to it, while we were downloading)
-    int Params = dfNoRecursive;
-    DeleteFile(FileName, File, &Params);
-    ChildError = false;
-  }
 }
 //---------------------------------------------------------------------------
 void __fastcall TTerminal::UpdateTargetAttrs(

source/core/Terminal.h

@@ -443,7 +443,7 @@ protected:
   void __fastcall Sink(
     const UnicodeString & FileName, const TRemoteFile * File, const UnicodeString & TargetDir,
     const TCopyParamType * CopyParam, int Params, TFileOperationProgressType * OperationProgress, unsigned int Flags,
-    TDownloadSessionAction & Action, bool & ChildError);
+    TDownloadSessionAction & Action);
   void __fastcall SinkFile(UnicodeString FileName, const TRemoteFile * File, void * Param);
   void __fastcall UpdateTargetAttrs(
     const UnicodeString & DestFullName, const TRemoteFile * File, const TCopyParamType * CopyParam, int Attrs);