From 19225507cca088ff965ccfc91bcf6f1fcd9960b3 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Thu, 12 Oct 2017 15:05:07 +0200 Subject: [PATCH] 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. --- common/rdr/Exception.h | 8 ++---- common/rfb/CConnection.cxx | 8 +++--- common/rfb/CSecurityTLS.cxx | 2 +- common/rfb/Exception.h | 16 +++++++----- common/rfb/SConnection.cxx | 50 ++++++++++++++++++++++--------------- common/rfb/SConnection.h | 2 +- 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 -- 2.39.5