Bug 2120: Consistent behavior across protocols when renaming/moving remote files

https://winscp.net/tracker/2120

Source commit: 0f11f96996352d8354d27e06d699e2c3baa4653f

source/components/UnixDirView.cpp

@@ -957,7 +957,7 @@ void __fastcall TUnixDirView::InternalEdit(const tagLVITEMW & HItem)
   if (ITEMFILE->FileName != HItem.pszText)
   {
     FSelectFile = HItem.pszText;
-    Terminal->RenameFile(ITEMFILE, HItem.pszText, true);
+    Terminal->RenameFile(ITEMFILE, HItem.pszText);
   }
 #else
   DebugUsedParam(HItem);

source/core/FtpFileSystem.cpp

@@ -376,6 +376,7 @@ void __fastcall TFTPFileSystem::Open()
   FMVS = false;
   FVMS = false;
   FFileZilla = false;
+  FIIS = false;
   FTransferActiveImmediately = (Data->FtpTransferActiveImmediately == asOn);
   FVMSAllRevisions = Data->VMSAllRevisions;
 
@@ -652,8 +653,7 @@ void __fastcall TFTPFileSystem::CollectUsage()
   // 220 Microsoft FTP Service
   // SYST
   // 215 Windows_NT
-  else if (ContainsText(FWelcomeMessage, L"Microsoft FTP Service") ||
-           ContainsText(FSystem, L"Windows_NT"))
+  else if (FIIS)
   {
     FTerminal->Configuration->Usage->Inc(L"OpenedSessionsFTPIIS");
   }
@@ -1879,6 +1879,9 @@ bool __fastcall TFTPFileSystem::IsCapable(int Capability) const
     case fcCheckingSpaceAvailable:
       return FBytesAvailableSupported || SupportsCommand(AvblCommand) || SupportsCommand(XQuotaCommand);
 
+    case fcMoveOverExistingFile:
+      return !FIIS;
+
     case fcAclChangingFiles:
     case fcModeChangingUpload:
     case fcLoadingAdditionalProperties:
@@ -3433,6 +3436,11 @@ void __fastcall TFTPFileSystem::HandleReplyStatus(UnicodeString Response)
           FTerminal->LogEvent(L"The server requires TLS/SSL handshake on transfer connection before responding 1yz to STOR/APPE");
           FTransferActiveImmediately = true;
         }
+        if (ContainsText(FWelcomeMessage, L"Microsoft FTP Service") && !FIIS)
+        {
+          FTerminal->LogEvent(L"IIS detected.");
+          FIIS = true;
+        }
       }
     }
     else if (FLastCommand == PASS)
@@ -3470,6 +3478,11 @@ void __fastcall TFTPFileSystem::HandleReplyStatus(UnicodeString Response)
         {
           FTerminal->LogEvent(L"The server is probably running Windows, assuming that directory listing timestamps are affected by DST.");
           FWindowsServer = true;
+          if (!FIIS)
+          {
+            FTerminal->LogEvent(L"IIS detected.");
+            FIIS = true;
+          }
         }
         // VMS system type. VMS V5.5-2.
         // VMS VAX/VMS V6.1 on node nsrp14

source/core/FtpFileSystem.h

@@ -296,6 +296,7 @@ private:
   bool FMVS;
   bool FVMS;
   bool FFileZilla;
+  bool FIIS;
   bool FFileTransferAny;
   bool FLoggedIn;
   bool FVMSAllRevisions;

source/core/S3FileSystem.cpp

@@ -767,6 +767,7 @@ bool __fastcall TS3FileSystem::IsCapable(int Capability) const
     case fcParallelTransfers:
     case fcLoadingAdditionalProperties:
     case fcAclChangingFiles:
+    case fcMoveOverExistingFile:
       return true;
 
     case fcPreservingTimestampUpload:

source/core/ScpFileSystem.cpp

@@ -443,6 +443,7 @@ bool __fastcall TSCPFileSystem::IsCapable(int Capability) const
     case fcRemoveCtrlZUpload:
     case fcRemoveBOMUpload:
     case fcCalculatingChecksum:
+    case fcMoveOverExistingFile:
       return true;
 
     case fcTextMode:

source/core/SessionInfo.h

@@ -47,6 +47,7 @@ enum TFSCapability { fcUserGroupListing, fcModeChanging, fcAclChangingFiles, fcG
   fcLocking, fcPreservingTimestampDirs, fcResumeSupport,
   fcChangePassword, fcSkipTransfer, fcParallelTransfers, fcBackgroundTransfers,
   fcTransferOut, fcTransferIn,
+  fcMoveOverExistingFile,
   fcCount };
 //---------------------------------------------------------------------------
 struct TFileSystemInfo

source/core/SftpFileSystem.cpp

@@ -2074,6 +2074,7 @@ bool __fastcall TSFTPFileSystem::IsCapable(int Capability) const
     case fcShellAnyCommand:
     case fcLocking:
     case fcAclChangingFiles: // pending implementation
+    case fcMoveOverExistingFile:
       return false;
 
     case fcNewerOnlyUpload:

source/core/Terminal.cpp

@@ -4098,18 +4098,16 @@ void __fastcall TTerminal::DeleteFile(UnicodeString FileName,
   }
   else
   {
-    LogEvent(FORMAT(L"Deleting file \"%s\".", (FileName)));
-    FileModified(File, FileName, true);
     DoDeleteFile(FileName, File, Params);
-    // Forget if file was or was not encrypted and use user preferences, if we ever recreate it.
-    FEncryptedFileNames.erase(AbsolutePath(FileName, true));
-    ReactOnCommand(fsDeleteFile);
   }
 }
 //---------------------------------------------------------------------------
 void __fastcall TTerminal::DoDeleteFile(const UnicodeString FileName,
   const TRemoteFile * File, int Params)
 {
+  LogEvent(FORMAT(L"Deleting file \"%s\".", (FileName)));
+  FileModified(File, FileName, true);
+
   TRetryOperationLoop RetryLoop(this);
   do
   {
@@ -4130,6 +4128,10 @@ void __fastcall TTerminal::DoDeleteFile(const UnicodeString FileName,
     }
   }
   while (RetryLoop.Retry());
+
+  // Forget if file was or was not encrypted and use user preferences, if we ever recreate it.
+  FEncryptedFileNames.erase(AbsolutePath(FileName, true));
+  ReactOnCommand(fsDeleteFile);
 }
 //---------------------------------------------------------------------------
 bool __fastcall TTerminal::DeleteFiles(TStrings * FilesToDelete, int Params)
@@ -4615,18 +4617,47 @@ void __fastcall TTerminal::CalculateFilesChecksum(
   }
 }
 //---------------------------------------------------------------------------
-void __fastcall TTerminal::RenameFile(const TRemoteFile * File,
-  const UnicodeString NewName, bool CheckExistence)
+void __fastcall TTerminal::RenameFile(const TRemoteFile * File, const UnicodeString & NewName)
 {
-  DebugAssert(File && File->Directory == FFiles);
-  bool Proceed = true;
-  // if filename doesn't contain path, we check for existence of file
-  if ((File->FileName != NewName) && CheckExistence &&
-      Configuration->ConfirmOverwriting &&
-      UnixSamePath(CurrentDirectory, FFiles->Directory))
+  // Already checked in TUnixDirView::InternalEdit
+  if (DebugAlwaysTrue(File->FileName != NewName))
+  {
+    FileModified(File, File->FileName);
+    LogEvent(FORMAT(L"Renaming file \"%s\" to \"%s\".", (File->FileName, NewName)));
+    if (DoRenameFile(File->FileName, File, NewName, false))
+    {
+      ReactOnCommand(fsRenameFile);
+    }
+  }
+}
+//---------------------------------------------------------------------------
+bool __fastcall TTerminal::DoRenameFile(const UnicodeString FileName, const TRemoteFile * File,
+  const UnicodeString NewName, bool Move)
+{
+  bool IsBatchMove = (OperationProgress != NULL) && DebugAlwaysTrue(OperationProgress->Operation == foRemoteMove);
+  TBatchOverwrite BatchOverwrite = (IsBatchMove ? OperationProgress->BatchOverwrite : boNo);
+  UnicodeString AbsoluteNewName = AbsolutePath(NewName, true);
+  bool Result = true;
+  bool ExistenceKnown = false;
+  TRemoteFile * DuplicateFile = NULL;
+  std::unique_ptr<TRemoteFile> DuplicateFileOwner(DuplicateFile);
+  if (BatchOverwrite == boNone)
+  {
+    Result = !FileExists(AbsoluteNewName);
+    ExistenceKnown = true;
+  }
+  else if (BatchOverwrite == boAll)
+  {
+    // noop
+  }
+  else if (DebugAlwaysTrue(BatchOverwrite == boNo) &&
+           Configuration->ConfirmOverwriting)
   {
-    TRemoteFile * DuplicateFile = FFiles->FindFile(NewName);
-    if (DuplicateFile)
+    FileExists(AbsoluteNewName, &DuplicateFile);
+    DuplicateFileOwner.reset(DuplicateFile);
+    ExistenceKnown = true;
+
+    if (DuplicateFile != NULL)
     {
       UnicodeString QuestionFmt;
       if (DuplicateFile->IsDirectory)
@@ -4639,49 +4670,87 @@ void __fastcall TTerminal::RenameFile(const TRemoteFile * File,
       }
       TQueryParams Params(qpNeverAskAgainCheck);
       UnicodeString Question = MainInstructions(FORMAT(QuestionFmt, (NewName)));
-      unsigned int Result = QueryUser(Question, NULL,
-        qaYes | qaNo, &Params);
-      if (Result == qaNeverAskAgain)
-      {
-        Proceed = true;
-        Configuration->ConfirmOverwriting = false;
-      }
-      else
+      unsigned int Answers = qaYes | qaNo | FLAGMASK(OperationProgress != NULL, qaCancel) | FLAGMASK(IsBatchMove, qaYesToAll | qaNoToAll);
+      unsigned int Answer = QueryUser(Question, NULL, Answers, &Params);
+      switch (Answer)
       {
-        Proceed = (Result == qaYes);
+        case qaNeverAskAgain:
+          Configuration->ConfirmOverwriting = false;
+          Result = true;
+          break;
+
+        case qaYes:
+          Result = true;
+          break;
+
+        case qaNo:
+          Result = false;
+          break;
+
+        case qaYesToAll:
+          Result = true;
+          if (DebugAlwaysTrue(IsBatchMove))
+          {
+            OperationProgress->SetBatchOverwrite(boAll);
+          }
+          break;
+
+        case qaNoToAll:
+          Result = false;
+          if (DebugAlwaysTrue(IsBatchMove))
+          {
+            OperationProgress->SetBatchOverwrite(boNone);
+          }
+          break;
+
+        case qaCancel:
+          Result = false;
+          OperationProgress->SetCancel(csCancel);
+          break;
+
+        default:
+          Result = false;
+          DebugFail();
+          break;
       }
     }
   }
 
-  if (Proceed)
-  {
-    FileModified(File, File->FileName);
-    LogEvent(FORMAT(L"Renaming file \"%s\" to \"%s\".", (File->FileName, NewName)));
-    DoRenameFile(File->FileName, File, NewName, false);
-    ReactOnCommand(fsRenameFile);
-  }
-}
-//---------------------------------------------------------------------------
-bool __fastcall TTerminal::DoRenameFile(const UnicodeString FileName, const TRemoteFile * File,
-  const UnicodeString NewName, bool Move)
-{
-  TRetryOperationLoop RetryLoop(this);
-  do
+  if (Result)
   {
-    TMvSessionAction Action(ActionLog, AbsolutePath(FileName, true), AbsolutePath(NewName, true));
-    try
+    if (!IsCapable[fcMoveOverExistingFile])
     {
-      DebugAssert(FFileSystem);
-      FFileSystem->RenameFile(FileName, File, NewName);
+      if (!ExistenceKnown)
+      {
+        FileExists(AbsoluteNewName, &DuplicateFile);
+        DuplicateFileOwner.reset(DuplicateFile);
+      }
+
+      if (DuplicateFile != NULL)
+      {
+        DoDeleteFile(AbsoluteNewName, DuplicateFile, 0);
+      }
     }
-    catch(Exception & E)
+
+    TRetryOperationLoop RetryLoop(this);
+    do
     {
-      UnicodeString Message = FMTLOAD(Move ? MOVE_FILE_ERROR : RENAME_FILE_ERROR, (FileName, NewName));
-      RetryLoop.Error(E, Action, Message);
+      TMvSessionAction Action(ActionLog, AbsolutePath(FileName, true), AbsoluteNewName);
+      try
+      {
+        DebugAssert(FFileSystem);
+        FFileSystem->RenameFile(FileName, File, NewName);
+      }
+      catch(Exception & E)
+      {
+        UnicodeString Message = FMTLOAD(Move ? MOVE_FILE_ERROR : RENAME_FILE_ERROR, (FileName, NewName));
+        RetryLoop.Error(E, Action, Message);
+      }
     }
+    while (RetryLoop.Retry());
+    Result = RetryLoop.Succeeded();
   }
-  while (RetryLoop.Retry());
-  return RetryLoop.Succeeded();
+  return Result;
 }
 //---------------------------------------------------------------------------
 bool __fastcall TTerminal::DoMoveFile(const UnicodeString & FileName, const TRemoteFile * File, /*const TMoveFileParams*/ void * Param)

source/core/Terminal.h

@@ -562,7 +562,7 @@ public:
   void __fastcall TerminalError(Exception * E, UnicodeString Msg, UnicodeString HelpKeyword = L"");
   void __fastcall ReloadDirectory();
   void __fastcall RefreshDirectory();
-  void __fastcall RenameFile(const TRemoteFile * File, const UnicodeString NewName, bool CheckExistence);
+  void __fastcall RenameFile(const TRemoteFile * File, const UnicodeString & NewName);
   void __fastcall MoveFile(const UnicodeString FileName, const TRemoteFile * File,
     /*const TMoveFileParams*/ void * Param);
   bool __fastcall MoveFiles(TStrings * FileList, const UnicodeString Target,

source/core/WebDAVFileSystem.cpp

@@ -602,6 +602,7 @@ bool __fastcall TWebDAVFileSystem::IsCapable(int Capability) const
     case fcSkipTransfer:
     case fcParallelTransfers:
     case fcRemoteCopy:
+    case fcMoveOverExistingFile:
       return true;
 
     case fcUserGroupListing:
@@ -1096,8 +1097,8 @@ void __fastcall TWebDAVFileSystem::DeleteFile(const UnicodeString FileName,
 int __fastcall TWebDAVFileSystem::RenameFileInternal(const UnicodeString & FileName,
   const UnicodeString & NewName)
 {
-  // 0 = no overwrite
-  return ne_move(FSessionContext->NeonSession, 0, PathToNeon(FileName), PathToNeon(NewName));
+  const int Overwrite = 1;
+  return ne_move(FSessionContext->NeonSession, Overwrite, PathToNeon(FileName), PathToNeon(NewName));
 }
 //---------------------------------------------------------------------------
 void __fastcall TWebDAVFileSystem::RenameFile(const UnicodeString FileName, const TRemoteFile * /*File*/,