Bug 1970: Problems with port forwarding are not detected

https://winscp.net/tracker/1970
(cherry picked from commit d3644dddd0681d05e0de3c8b14a41ffda54d428e)

Source commit: 4178eaab1408be001329bcff98dad7298a0c030d

source/core/SecureShell.cpp

@@ -651,15 +651,23 @@ UnicodeString __fastcall TSecureShell::ConvertFromPutty(const char * Str, int Le
   }
 }
 //---------------------------------------------------------------------------
+const UnicodeString ServerVersionMsg(L"Remote version: ");
+const UnicodeString ForwardingFailureMsg(L"Forwarded connection refused by remote");
+const UnicodeString LocalPortMsg(L"Local port ");
+const UnicodeString ForwadingToMsg(L" forwarding to ");
+const UnicodeString FailedMsg(L" failed:");
+//---------------------------------------------------------------------------
 void __fastcall TSecureShell::PuttyLogEvent(const char * AStr)
 {
   UnicodeString Str = ConvertFromPutty(AStr, strlen(AStr));
-  #define SERVER_VERSION_MSG L"Remote version: "
+  if (Str.Pos(L"failed") > 0)
+  {
+    Str += L"";
+  }
   // Gross hack
-  if (Str.Pos(SERVER_VERSION_MSG) == 1)
+  if (StartsStr(ServerVersionMsg, Str))
   {
-    FSessionInfo.SshVersionString = Str.SubString(wcslen(SERVER_VERSION_MSG) + 1,
-      Str.Length() - wcslen(SERVER_VERSION_MSG));
+    FSessionInfo.SshVersionString = RightStr(Str, Str.Length() - ServerVersionMsg.Length());
 
     const wchar_t * Ptr = wcschr(FSessionInfo.SshVersionString.c_str(), L'-');
     if (Ptr != NULL)
@@ -668,17 +676,30 @@ void __fastcall TSecureShell::PuttyLogEvent(const char * AStr)
     }
     FSessionInfo.SshImplementation = (Ptr != NULL) ? Ptr + 1 : L"";
   }
-  #define FORWARDING_FAILURE_MSG L"Forwarded connection refused by server: "
-  else if (Str.Pos(FORWARDING_FAILURE_MSG) == 1)
+  else if (StartsStr(ForwardingFailureMsg, Str))
   {
-    FLastTunnelError = Str.SubString(wcslen(FORWARDING_FAILURE_MSG) + 1,
-      Str.Length() - wcslen(FORWARDING_FAILURE_MSG));
-
-    static const TPuttyTranslation Translation[] = {
-      { L"Administratively prohibited [%]", PFWD_TRANSL_ADMIN },
-      { L"Connect failed [%]", PFWD_TRANSL_CONNECT },
-    };
-    TranslatePuttyMessage(Translation, LENOF(Translation), FLastTunnelError);
+    if (ForwardingFailureMsg == Str)
+    {
+      FLastTunnelError = Str;
+    }
+    else
+    {
+      FLastTunnelError = RightStr(Str, Str.Length() - ForwardingFailureMsg.Length());
+      UnicodeString Prefix(L": ");
+      if (StartsStr(Prefix, FLastTunnelError))
+      {
+        FLastTunnelError.Delete(1, Prefix.Length());
+      }
+      static const TPuttyTranslation Translation[] = {
+        { L"Administratively prohibited [%]", PFWD_TRANSL_ADMIN },
+        { L"Connect failed [%]", PFWD_TRANSL_CONNECT },
+      };
+      TranslatePuttyMessage(Translation, LENOF(Translation), FLastTunnelError);
+    }
+  }
+  else if (StartsStr(LocalPortMsg, Str) && ContainsStr(Str, ForwadingToMsg) && ContainsStr(Str, FailedMsg))
+  {
+    FLastTunnelError = Str;
   }
   LogEvent(Str);
 }
@@ -1492,7 +1513,7 @@ int __fastcall TSecureShell::TranslateErrorMessage(
   UnicodeString & Message, UnicodeString * HelpKeyword)
 {
   static const TPuttyTranslation Translation[] = {
-    { L"Server unexpectedly closed network connection", UNEXPECTED_CLOSE_ERROR, HELP_UNEXPECTED_CLOSE_ERROR },
+    { L"Remote side unexpectedly closed network connection", UNEXPECTED_CLOSE_ERROR, HELP_UNEXPECTED_CLOSE_ERROR },
     { L"Network error: Connection refused", NET_TRANSL_REFUSED2, HELP_NET_TRANSL_REFUSED },
     { L"Network error: Connection reset by peer", NET_TRANSL_RESET, HELP_NET_TRANSL_RESET },
     { L"Network error: Connection timed out", NET_TRANSL_TIMEOUT2, HELP_NET_TRANSL_TIMEOUT },

source/core/Terminal.cpp

@@ -1330,10 +1330,12 @@ void __fastcall TTerminal::Open()
                     FSecureShell->GetHostKeyFingerprint(FFingerprintScannedSHA256, FFingerprintScannedMD5);
                     FFingerprintScannedSHA1 = UnicodeString();
                   }
+                  // Tunnel errors that happens only once we connect to the local port (server-side forwarding problems).
+                  // Contrary to local side problems handled already in OpenTunnel.
                   if (!FSecureShell->Active && !FTunnelError.IsEmpty())
                   {
-                    // the only case where we expect this to happen
-                    DebugAssert(E.Message == LoadStr(UNEXPECTED_CLOSE_ERROR));
+                    // the only case where we expect this to happen (first in GUI, the latter in scripting)
+                    DebugAssert((E.Message == LoadStr(UNEXPECTED_CLOSE_ERROR)) || (E.Message == LoadStr(NET_TRANSL_CONN_ABORTED)));
                     FatalError(&E, FMTLOAD(TUNNEL_ERROR, (FTunnelError)));
                   }
                   else
@@ -1555,13 +1557,31 @@ void __fastcall TTerminal::OpenTunnel()
       FTunnelOpening = false;
     }
 
+    // When forwarding fails due to a local problem (e.g. port in use), we get the error already here, so abort asap.
+    // If we won't abort, particularly in case of "port in use", we may not even detect the problem,
+    // as the connection to the port may succeed.
+    // Remote-side tunnel problems are handled in TTerminal::Open().
+    if (!FTunnel->LastTunnelError.IsEmpty())
+    {
+      // Throw and handle with any other theoretical forwarding errors that may cause tunnel connection close
+      // (probably cannot happen)
+      EXCEPTION;
+    }
+
     FTunnelThread = new TTunnelThread(FTunnel);
   }
-  catch (...)
+  catch (Exception & E)
   {
     LogEvent(L"Error opening tunnel.");
     CloseTunnel();
-    throw;
+    if (!FTunnelError.IsEmpty())
+    {
+      FatalError(&E, FMTLOAD(TUNNEL_ERROR, (FTunnelError)));
+    }
+    else
+    {
+      throw;
+    }
   }
 }
 //---------------------------------------------------------------------------