Make exception classes have clearer messages

Include the type of exception in the string generated by each
subclass. Also simplify the constructs to what is needed.

6 files changed, 47 insertions, 39 deletions

diff --git a/common/rdr/Exception.h b/common/rdr/Exception.h
index 62f452b2..69abbedb 100644
--- a/common/rdr/Exception.h
+++ b/common/rdr/Exception.h
@@ -43,15 +43,11 @@ namespace rdr {
   }; 
 
   struct TimedOut : public Exception {
-    TimedOut(const char* s="Timed out") : Exception("%s", s) {}
+    TimedOut() : Exception("Timed out") {}
   };
  
   struct EndOfStream : public Exception {
-    EndOfStream(const char* s="End of stream") : Exception("%s", s) {}
-  };
-
-  struct FrameException : public Exception {
-    FrameException(const char* s="Frame exception") : Exception("%s", s) {}
+    EndOfStream() : Exception("End of stream") {}
   };
 
 }
diff --git a/common/rfb/CConnection.cxx b/common/rfb/CConnection.cxx
index 88befd5e..ce489b1b 100644
--- a/common/rfb/CConnection.cxx
+++ b/common/rfb/CConnection.cxx
@@ -270,12 +270,10 @@ void CConnection::processSecurityResultMsg()
   default:
     throw Exception("Unknown security result from server");
   }
-  CharArray reason;
-  if (cp.beforeVersion(3,8))
-    reason.buf = strDup("Authentication failure");
-  else
-    reason.buf = is->readString();
   state_ = RFBSTATE_INVALID;
+  if (cp.beforeVersion(3,8))
+    throw AuthFailureException();
+  CharArray reason(is->readString());
   throw AuthFailureException(reason.buf);
 }
 
diff --git a/common/rfb/CSecurityTLS.cxx b/common/rfb/CSecurityTLS.cxx
index 8116e9c1..9a698f03 100644
--- a/common/rfb/CSecurityTLS.cxx
+++ b/common/rfb/CSecurityTLS.cxx
@@ -153,7 +153,7 @@ bool CSecurityTLS::processMsg(CConnection* cc)
       if (result == secResultFailed || result == secResultTooMany)
         reason.buf = is->readString();
       else
-        reason.buf = strDup("Authentication failure (protocol error)");
+        reason.buf = strDup("protocol error");
       throw AuthFailureException(reason.buf);
     }
 
diff --git a/common/rfb/Exception.h b/common/rfb/Exception.h
index 5f47fcf2..827ced52 100644
--- a/common/rfb/Exception.h
+++ b/common/rfb/Exception.h
@@ -23,16 +23,20 @@
 namespace rfb {
   typedef rdr::Exception Exception;
   struct AuthFailureException : public Exception {
-    AuthFailureException(const char* s="Authentication failure")
-      : Exception("%s", s) {}
+    AuthFailureException()
+      : Exception("Authentication failure") {}
+    AuthFailureException(const char* reason)
+      : Exception("Authentication failure: %s", reason) {}
   };
   struct AuthCancelledException : public rfb::Exception {
-    AuthCancelledException(const char* s="Authentication cancelled")
-      : Exception("%s", s) {}
+    AuthCancelledException()
+      : Exception("Authentication cancelled") {}
   };
   struct ConnFailedException : public Exception {
-    ConnFailedException(const char* s="Connection failed")
-      : Exception("%s", s) {}
+    ConnFailedException()
+      : Exception("Connection failed") {}
+    ConnFailedException(const char* reason)
+      : Exception("Connection failed: %s", reason) {}
   };
 }
 #endif
diff --git a/common/rfb/SConnection.cxx b/common/rfb/SConnection.cxx
index c5c9038c..6b810559 100644
--- a/common/rfb/SConnection.cxx
+++ b/common/rfb/SConnection.cxx
@@ -116,11 +116,9 @@ void SConnection::processVersionMsg()
 
   if (cp.majorVersion != 3) {
     // unknown protocol version
-    char msg[256];
-    sprintf(msg,"Error: client needs protocol version %d.%d, server has %d.%d",
-            cp.majorVersion, cp.minorVersion,
-            defaultMajorVersion, defaultMinorVersion);
-    throwConnFailedException(msg);
+    throwConnFailedException("Client needs protocol version %d.%d, server has %d.%d",
+                             cp.majorVersion, cp.minorVersion,
+                             defaultMajorVersion, defaultMinorVersion);
   }
 
   if (cp.minorVersion != 3 && cp.minorVersion != 7 && cp.minorVersion != 8) {
@@ -150,10 +148,8 @@ void SConnection::processVersionMsg()
       if (*i == secTypeNone || *i == secTypeVncAuth) break;
     }
     if (i == secTypes.end()) {
-      char msg[256];
-      sprintf(msg,"No supported security type for %d.%d client",
-              cp.majorVersion, cp.minorVersion);
-      throwConnFailedException(msg);
+      throwConnFailedException("No supported security type for %d.%d client",
+                               cp.majorVersion, cp.minorVersion);
     }
 
     os->writeU32(*i);
@@ -204,7 +200,7 @@ void SConnection::processSecurityType(int secType)
     state_ = RFBSTATE_SECURITY;
     ssecurity = security.GetSSecurity(secType);
   } catch (rdr::Exception& e) {
-    throwConnFailedException(e.str());
+    throwConnFailedException("%s", e.str());
   }
 
   processSecurityMsg();
@@ -236,22 +232,31 @@ void SConnection::processInitMsg()
   reader_->readClientInit();
 }
 
-void SConnection::throwConnFailedException(const char* msg)
+void SConnection::throwConnFailedException(const char* format, ...)
 {
-  vlog.info("%s", msg);
+	va_list ap;
+	char str[256];
+
+	va_start(ap, format);
+	(void) vsnprintf(str, sizeof(str), format, ap);
+	va_end(ap);
+
+  vlog.info("Connection failed: %s", str);
+
   if (state_ == RFBSTATE_PROTOCOL_VERSION) {
     if (cp.majorVersion == 3 && cp.minorVersion == 3) {
       os->writeU32(0);
-      os->writeString(msg);
+      os->writeString(str);
       os->flush();
     } else {
       os->writeU8(0);
-      os->writeString(msg);
+      os->writeString(str);
       os->flush();
     }
   }
+
   state_ = RFBSTATE_INVALID;
-  throw ConnFailedException(msg);
+  throw ConnFailedException(str);
 }
 
 void SConnection::writeConnFailedFromScratch(const char* msg,
@@ -301,15 +306,17 @@ void SConnection::approveConnection(bool accept, const char* reason)
   if (state_ != RFBSTATE_QUERYING)
     throw Exception("SConnection::approveConnection: invalid state");
 
-  if (!reason) reason = "Authentication failure";
-
   if (!cp.beforeVersion(3,8) || ssecurity->getType() != secTypeNone) {
     if (accept) {
       os->writeU32(secResultOK);
     } else {
       os->writeU32(secResultFailed);
-      if (!cp.beforeVersion(3,8)) // 3.8 onwards have failure message
-        os->writeString(reason);
+      if (!cp.beforeVersion(3,8)) { // 3.8 onwards have failure message
+        if (reason)
+          os->writeString(reason);
+        else
+          os->writeString("Authentication failure");
+      }
     }
     os->flush();
   }
@@ -321,7 +328,10 @@ void SConnection::approveConnection(bool accept, const char* reason)
     authSuccess();
   } else {
     state_ = RFBSTATE_INVALID;
-    throw AuthFailureException(reason);
+    if (reason)
+      throw AuthFailureException(reason);
+    else
+      throw AuthFailureException();
   }
 }
 
diff --git a/common/rfb/SConnection.h b/common/rfb/SConnection.h
index bc435834..47092e35 100644
--- a/common/rfb/SConnection.h
+++ b/common/rfb/SConnection.h
@@ -144,7 +144,7 @@ namespace rfb {
     // throwConnFailedException() prints a message to the log, sends a conn
     // failed message to the client (if possible) and throws a
     // ConnFailedException.
-    void throwConnFailedException(const char* msg);
+    void throwConnFailedException(const char* format, ...) __printf_attr(2, 3);
 
     // writeConnFailedFromScratch() sends a conn failed message to an OutStream
     // without the need to negotiate the protocol version first.  It actually