Issue 2457 – When selecting a site to perform a command-line operation with, it was possible to select a workspace or a folder, resulting in unexpected behaviour or failure

https://winscp.net/tracker/2457

Source commit: 4ba24fc7f915728e77c9ec86acceae1932dd58fd

source/forms/CustomScpExplorer.cpp

@@ -535,7 +535,7 @@ bool __fastcall TCustomScpExplorerForm::CommandLineFromAnotherInstance(
       {
         UnicodeString DownloadFile; // unused
         int Flags = GetCommandLineParseUrlFlags(&Params);
-        GetLoginData(SessionName, &Params, DataList.get(), DownloadFile, true, this, Flags);
+        GetLoginData(SessionName, &Params, DataList.get(), DownloadFile, lnSession, this, Flags);
       }
       catch (EAbort &)
       {

source/forms/Login.cpp

@@ -24,14 +24,14 @@ const int LoginImageIndex = 0;
 const int OpenWorkspaceImageIndex = 5;
 const int OpenFolderImageIndex = 6;
 //---------------------------------------------------------------------------
-bool __fastcall DoLoginDialog(TList * DataList, TForm * LinkedForm)
+bool DoLoginDialog(TList * DataList, TForm * LinkedForm, bool NeedTerminal)
 {
   DebugAssert(DataList != NULL);
   TLoginDialog * LoginDialog = SafeFormCreate<TLoginDialog>();
   bool Result;
   try
   {
-    LoginDialog->Init(LinkedForm);
+    LoginDialog->Init(LinkedForm, NeedTerminal);
     Result = LoginDialog->Execute(DataList);
   }
   __finally
@@ -63,6 +63,7 @@ __fastcall TLoginDialog::TLoginDialog(TComponent* AOwner)
   FLinkedForm = NULL;
   FRestoring = false;
   FPrevPos = TPoint(std::numeric_limits<LONG>::min(), std::numeric_limits<LONG>::min());
+  FNeedTerminal = false;
 
   // we need to make sure that window procedure is set asap
   // (so that CM_SHOWINGCHANGED handling is applied)
@@ -93,9 +94,10 @@ void __fastcall TLoginDialog::InvalidateSessionData()
   FSessionData = NULL;
 }
 //---------------------------------------------------------------------
-void __fastcall TLoginDialog::Init(TForm * LinkedForm)
+void TLoginDialog::Init(TForm * LinkedForm, bool NeedTerminal)
 {
   FLinkedForm = LinkedForm;
+  FNeedTerminal = NeedTerminal;
   LoadSessions();
   UnicodeString Dummy;
   RunPageantAction->Visible = FindTool(PageantTool, Dummy);
@@ -1247,7 +1249,7 @@ void __fastcall TLoginDialog::ActionListUpdate(TBasicAction * BasicAction,
   }
   else if (Action == LoginAction)
   {
-    LoginAction->Enabled = CanOpen();
+    LoginAction->Enabled = FNeedTerminal ? IsSiteAndCanOpen() : CanOpen();
     LoginAction->Caption = FolderOrWorkspaceSelected ? LoadStr(LOGIN_OPEN) : LoadStr(LOGIN_LOGIN);
     LoginAction->ImageIndex = FolderOrWorkspaceSelected ? (WorkspaceSelected ? OpenWorkspaceImageIndex : OpenFolderImageIndex) : LoginImageIndex;
     UpdateLoginButton();
@@ -1256,6 +1258,7 @@ void __fastcall TLoginDialog::ActionListUpdate(TBasicAction * BasicAction,
   {
     TSessionData * Data = GetSessionData();
     Action->Enabled =
+      // IsLocalBrowser is an excess test as a stored site cannot be a local browser and such test is not done in other similar situations
       (IsSiteAndCanOpen() && !Data->IsLocalBrowser && !Data->Tunnel) ||
       (IsFolderOrWorkspaceAndCanOpen() && IsFolderNode(SessionTree->Selected));
   }
@@ -3159,7 +3162,7 @@ void __fastcall TLoginDialog::PuttyActionExecute(TObject * /*Sender*/)
   }
   // following may take some time, so cache the shift key state,
   // in case user manages to release it before following finishes
-  bool Close = !OpenInNewWindow();
+  bool Close = !OpenInNewWindow() && !FNeedTerminal;
 
   std::unique_ptr<TList> DataList(new TList());
   SaveDataList(DataList.get());

source/forms/Login.h

@@ -339,6 +339,7 @@ private:
   UnicodeString FPasswordLabel;
   int FFixedSessionImages;
   bool FRestoring;
+  bool FNeedTerminal;
 
   void __fastcall LoadSession(TSessionData * SessionData);
   void __fastcall LoadContents();
@@ -452,7 +453,7 @@ protected:
 public:
   virtual __fastcall TLoginDialog(TComponent* AOwner);
   __fastcall ~TLoginDialog();
-  void __fastcall Init(TForm * LinkedForm);
+  void Init(TForm * LinkedForm, bool NeedTerminal);
   bool __fastcall Execute(TList * DataList);
 };
 //----------------------------------------------------------------------------

source/windows/TerminalManager.cpp

@@ -1807,11 +1807,11 @@ void __fastcall TTerminalManager::NewSession(
     {
       DataList.reset(new TObjectList());
       UnicodeString DownloadFile; // unused
-      GetLoginData(SessionUrl, NULL, DataList.get(), DownloadFile, true, LinkedForm);
+      GetLoginData(SessionUrl, NULL, DataList.get(), DownloadFile, lnSession, LinkedForm);
     }
     else
     {
-      if (!DoLoginDialog(DataList.get(), LinkedForm))
+      if (!DoLoginDialog(DataList.get(), LinkedForm, false))
       {
         Abort(); // As GetLoginData would do
       }

source/windows/WinInterface.h

@@ -176,8 +176,10 @@ bool __fastcall DoChangeMasterPasswordDialog(UnicodeString & NewPassword);
 
 // windows\WinMain.cpp
 int __fastcall Execute();
-void __fastcall GetLoginData(UnicodeString SessionName, TOptions * Options,
-  TObjectList * DataList, UnicodeString & DownloadFile, bool NeedSession, TForm * LinkedForm, int Flags = 0);
+enum TLoginNeed { lnNone, lnSession, lnTerminal };
+void GetLoginData(
+  const UnicodeString & SessionName, TOptions * Options,
+  TObjectList * DataList, UnicodeString & DownloadFile, TLoginNeed LoginNeed, TForm * LinkedForm, int Flags = 0);
 int GetCommandLineParseUrlFlags(TProgramParams * Params);
 
 // forms\InputDlg.cpp
@@ -253,7 +255,7 @@ bool __fastcall DoImportSessionsDialog(TList * Imported);
 enum TLicense { lcNoLicense = -1, lcWinScp, lcExpat };
 void __fastcall DoLicenseDialog(TLicense License);
 
-bool __fastcall DoLoginDialog(TList * DataList, TForm * LinkedForm);
+bool DoLoginDialog(TList * DataList, TForm * LinkedForm, bool NeedTerminal);
 
   // forms\SiteAdvanced.cpp
 bool __fastcall DoSiteAdvancedDialog(TSessionData * SessionData);

source/windows/WinMain.cpp

@@ -22,8 +22,9 @@ UnicodeString GetFolderOrWorkspaceName(const UnicodeString & SessionName)
   return Result;
 }
 //---------------------------------------------------------------------------
-void __fastcall GetLoginData(UnicodeString SessionName, TOptions * Options,
-  TObjectList * DataList, UnicodeString & DownloadFile, bool NeedSession, TForm * LinkedForm, int Flags)
+void GetLoginData(
+  const UnicodeString & SessionName, TOptions * Options,
+  TObjectList * DataList, UnicodeString & DownloadFile, TLoginNeed LoginNeed, TForm * LinkedForm, int Flags)
 {
   bool DefaultsOnly = false;
 
@@ -63,7 +64,7 @@ void __fastcall GetLoginData(UnicodeString SessionName, TOptions * Options,
     }
   }
 
-  if (DefaultsOnly && !NeedSession)
+  if (DefaultsOnly && (LoginNeed == lnNone))
   {
     // No URL specified on command-line and no explicit command-line parameter
     // that requires session was specified => noop
@@ -83,7 +84,7 @@ void __fastcall GetLoginData(UnicodeString SessionName, TOptions * Options,
     // - the specified session does not contain enough information to open [= not even hostname nor local browser]
 
     DebugAssert(DataList->Count <= 1);
-    if (!DoLoginDialog(DataList, LinkedForm))
+    if (!DoLoginDialog(DataList, LinkedForm, (LoginNeed == lnTerminal)))
     {
       Abort();
     }
@@ -1229,7 +1230,19 @@ int __fastcall Execute()
       // from now flash message boxes in background
       SetOnForeground(false);
 
-      bool NeedSession = NewInstance || (ParamCommand != pcNone);
+      TLoginNeed LoginNeed;
+      if (ParamCommand != pcNone)
+      {
+        LoginNeed = lnTerminal;
+      }
+      else if (NewInstance)
+      {
+        LoginNeed = lnSession;
+      }
+      else
+      {
+        LoginNeed = lnNone;
+      }
 
       bool Retry;
       do
@@ -1240,9 +1253,9 @@ int __fastcall Execute()
         {
           int Flags = GetCommandLineParseUrlFlags(Params);
           AddStartupSequence(L"B");
-          GetLoginData(AutoStartSession, Params, DataList.get(), DownloadFile, NeedSession, NULL, Flags);
+          GetLoginData(AutoStartSession, Params, DataList.get(), DownloadFile, LoginNeed, NULL, Flags);
           // GetLoginData now Aborts when session is needed and none is selected
-          if (DebugAlwaysTrue(!NeedSession || (DataList->Count > 0)))
+          if (DebugAlwaysTrue((LoginNeed == lnNone) || (DataList->Count > 0)))
           {
             if (CheckSafe(Params))
             {
@@ -1289,7 +1302,7 @@ int __fastcall Execute()
               }
               else
               {
-                DebugAssert(!NeedSession);
+                DebugAssert(LoginNeed == lnNone);
                 CanStart = true;
               }