Bug 1766: It was not possible to overwrite remote file, when moving of overwritten files to remote recycle bin was configured, but the file could not be moved

https://winscp.net/tracker/1766

Source commit: 69c7cef80a7b0760b8ab4aa366b33421144ba725

source/core/SftpFileSystem.cpp

@@ -5073,7 +5073,11 @@ int __fastcall TSFTPFileSystem::SFTPOpenRemote(void * AOpenParams, void * /*Para
           else
           {
             OperationProgress->Progress();
-            FTerminal->RecycleFile(OpenParams->RemoteFileName, NULL);
+            if (!FTerminal->RecycleFile(OpenParams->RemoteFileName, NULL))
+            {
+              // Allow normal overwrite
+              OpenParams->DontRecycle = true;
+            }
           }
         }
       }

source/core/Terminal.cpp

@@ -485,10 +485,12 @@ public:
   void Error(Exception & E, const UnicodeString & Message);
   void Error(Exception & E, TSessionAction & Action, const UnicodeString & Message);
   bool Retry();
+  bool Succeeded();
 
 private:
   TTerminal * FTerminal;
   bool FRetry;
+  bool FSucceeded;
 
   void DoError(Exception & E, TSessionAction * Action, const UnicodeString & Message);
 };
@@ -497,10 +499,12 @@ TRetryOperationLoop::TRetryOperationLoop(TTerminal * Terminal)
 {
   FTerminal = Terminal;
   FRetry = false;
+  FSucceeded = true;
 }
 //---------------------------------------------------------------------------
 void TRetryOperationLoop::DoError(Exception & E, TSessionAction * Action, const UnicodeString & Message)
 {
+  FSucceeded = false;
   // Note that the action may already be canceled when RollbackAction is called
   unsigned int Result;
   try
@@ -571,9 +575,18 @@ bool TRetryOperationLoop::Retry()
 {
   bool Result = FRetry;
   FRetry = false;
+  if (Result)
+  {
+    FSucceeded = true;
+  }
   return Result;
 }
 //---------------------------------------------------------------------------
+bool TRetryOperationLoop::Succeeded()
+{
+  return FSucceeded;
+}
+//---------------------------------------------------------------------------
 //---------------------------------------------------------------------------
 TCollectedFileList::TCollectedFileList()
 {
@@ -3885,16 +3898,21 @@ bool __fastcall TTerminal::IsRecycledFile(UnicodeString FileName)
   return Result;
 }
 //---------------------------------------------------------------------------
-void __fastcall TTerminal::RecycleFile(UnicodeString FileName,
-  const TRemoteFile * File)
+bool __fastcall TTerminal::RecycleFile(const UnicodeString & AFileName, const TRemoteFile * File)
 {
+  UnicodeString FileName = AFileName;
   if (FileName.IsEmpty())
   {
     DebugAssert(File != NULL);
     FileName = File->FileName;
   }
 
-  if (!IsRecycledFile(FileName))
+  bool Result;
+  if (IsRecycledFile(FileName))
+  {
+    Result = true;
+  }
+  else
   {
     LogEvent(FORMAT(L"Moving file \"%s\" to remote recycle bin '%s'.",
       (FileName, SessionData->RecycleBinPath)));
@@ -3903,13 +3921,15 @@ void __fastcall TTerminal::RecycleFile(UnicodeString FileName,
     Params.Target = SessionData->RecycleBinPath;
     Params.FileMask = FORMAT(L"*-%s.*", (FormatDateTime(L"yyyymmdd-hhnnss", Now())));
 
-    MoveFile(FileName, File, &Params);
+    Result = DoMoveFile(FileName, File, &Params);
 
-    if ((OperationProgress != NULL) && (OperationProgress->Operation == foDelete))
+    if (Result && (OperationProgress != NULL) && (OperationProgress->Operation == foDelete))
     {
       OperationProgress->Succeeded();
     }
   }
+
+  return Result;
 }
 //---------------------------------------------------------------------------
 bool __fastcall TTerminal::TryStartOperationWithFile(
@@ -4456,7 +4476,7 @@ void __fastcall TTerminal::RenameFile(const TRemoteFile * File,
   }
 }
 //---------------------------------------------------------------------------
-void __fastcall TTerminal::DoRenameFile(const UnicodeString FileName, const TRemoteFile * File,
+bool __fastcall TTerminal::DoRenameFile(const UnicodeString FileName, const TRemoteFile * File,
   const UnicodeString NewName, bool Move)
 {
   TRetryOperationLoop RetryLoop(this);
@@ -4475,10 +4495,10 @@ void __fastcall TTerminal::DoRenameFile(const UnicodeString FileName, const TRem
     }
   }
   while (RetryLoop.Retry());
+  return RetryLoop.Succeeded();
 }
 //---------------------------------------------------------------------------
-void __fastcall TTerminal::MoveFile(const UnicodeString FileName,
-  const TRemoteFile * File, /*const TMoveFileParams*/ void * Param)
+bool __fastcall TTerminal::DoMoveFile(const UnicodeString & FileName, const TRemoteFile * File, /*const TMoveFileParams*/ void * Param)
 {
   StartOperationWithFile(FileName, foRemoteMove, foDelete);
   DebugAssert(Param != NULL);
@@ -4487,8 +4507,17 @@ void __fastcall TTerminal::MoveFile(const UnicodeString FileName,
     MaskFileName(UnixExtractFileName(FileName), Params.FileMask);
   LogEvent(FORMAT(L"Moving file \"%s\" to \"%s\".", (FileName, NewName)));
   FileModified(File, FileName);
-  DoRenameFile(FileName, File, NewName, true);
-  ReactOnCommand(fsMoveFile);
+  bool Result = DoRenameFile(FileName, File, NewName, true);
+  if (Result)
+  {
+    ReactOnCommand(fsMoveFile);
+  }
+  return Result;
+}
+//---------------------------------------------------------------------------
+void __fastcall TTerminal::MoveFile(const UnicodeString FileName, const TRemoteFile * File, /*const TMoveFileParams*/ void * Param)
+{
+  DoMoveFile(FileName, File, Param);
 }
 //---------------------------------------------------------------------------
 bool __fastcall TTerminal::MoveFiles(TStrings * FileList, const UnicodeString Target,

source/core/Terminal.h

@@ -273,8 +273,9 @@ protected:
     int Params);
   void __fastcall DoCustomCommandOnFile(UnicodeString FileName,
     const TRemoteFile * File, UnicodeString Command, int Params, TCaptureOutputEvent OutputEvent);
-  void __fastcall DoRenameFile(const UnicodeString FileName, const TRemoteFile * File,
+  bool __fastcall DoRenameFile(const UnicodeString FileName, const TRemoteFile * File,
     const UnicodeString NewName, bool Move);
+  bool __fastcall DoMoveFile(const UnicodeString & FileName, const TRemoteFile * File, /*const TMoveFileParams*/ void * Param);
   void __fastcall DoCopyFile(const UnicodeString FileName, const TRemoteFile * File, const UnicodeString NewName);
   void __fastcall DoChangeFileProperties(const UnicodeString FileName,
     const TRemoteFile * File, const TRemoteProperties * Properties);
@@ -359,7 +360,7 @@ protected:
   void __fastcall DoSynchronizeProgress(const TSynchronizeData & Data, bool Collect);
   void __fastcall DeleteLocalFile(UnicodeString FileName,
     const TRemoteFile * File, void * Param);
-  void __fastcall RecycleFile(UnicodeString FileName, const TRemoteFile * File);
+  bool __fastcall RecycleFile(const UnicodeString & FileName, const TRemoteFile * File);
   TStrings * __fastcall GetFixedPaths();
   void __fastcall DoStartup();
   virtual bool __fastcall DoQueryReopen(Exception * E);