From a0a4a68dc876804f283131b5394610c556b67547 Mon Sep 17 00:00:00 2001 From: Neel Goyal Date: Mon, 2 May 2016 09:30:18 -0700 Subject: [PATCH] Set the appropriate AsyncSocketExceptionType from SSLException Summary: We should use the appropriate AsyncSocketExceptionType when firing SSLExceptions. This derives the type from the constructor args, and introduces a `NETWORK_ERROR` exception type. Open to suggestions on reusing something there. Additionally, rename getType() to getSSLError() to prevent hiding the AsyncSocketException::getType. Reviewed By: siyengar Differential Revision: D3241204 fb-gh-sync-id: de631dfb3554177d5bd561f4b91e767c882767d3 fbshipit-source-id: de631dfb3554177d5bd561f4b91e767c882767d3 --- folly/io/async/AsyncSocketException.h | 31 ++++++------ folly/io/async/ssl/SSLErrors.cpp | 34 ++++++++++++- folly/io/async/ssl/SSLErrors.h | 2 +- .../async/test/AsyncSocketExceptionTest.cpp | 49 ++++++++++++++++++- 4 files changed, 97 insertions(+), 19 deletions(-) diff --git a/folly/io/async/AsyncSocketException.h b/folly/io/async/AsyncSocketException.h index 3113197c..85054d86 100644 --- a/folly/io/async/AsyncSocketException.h +++ b/folly/io/async/AsyncSocketException.h @@ -23,21 +23,22 @@ namespace folly { class AsyncSocketException : public std::runtime_error { public: - enum AsyncSocketExceptionType - { UNKNOWN = 0 - , NOT_OPEN = 1 - , ALREADY_OPEN = 2 - , TIMED_OUT = 3 - , END_OF_FILE = 4 - , INTERRUPTED = 5 - , BAD_ARGS = 6 - , CORRUPTED_DATA = 7 - , INTERNAL_ERROR = 8 - , NOT_SUPPORTED = 9 - , INVALID_STATE = 10 - , SSL_ERROR = 12 - , COULD_NOT_BIND = 13 - , SASL_HANDSHAKE_TIMEOUT = 14 + enum AsyncSocketExceptionType { + UNKNOWN = 0, + NOT_OPEN = 1, + ALREADY_OPEN = 2, + TIMED_OUT = 3, + END_OF_FILE = 4, + INTERRUPTED = 5, + BAD_ARGS = 6, + CORRUPTED_DATA = 7, + INTERNAL_ERROR = 8, + NOT_SUPPORTED = 9, + INVALID_STATE = 10, + SSL_ERROR = 12, + COULD_NOT_BIND = 13, + SASL_HANDSHAKE_TIMEOUT = 14, + NETWORK_ERROR = 15 }; AsyncSocketException(AsyncSocketExceptionType type, diff --git a/folly/io/async/ssl/SSLErrors.cpp b/folly/io/async/ssl/SSLErrors.cpp index 7bbb78c3..c34e8bbd 100644 --- a/folly/io/async/ssl/SSLErrors.cpp +++ b/folly/io/async/ssl/SSLErrors.cpp @@ -68,6 +68,36 @@ const StringPiece getSSLErrorString(SSLError error) { } return ret; } + +AsyncSocketException::AsyncSocketExceptionType exTypefromSSLErrInfo( + int sslErr, + unsigned long errError, + int sslOperationReturnValue) { + if (sslErr == SSL_ERROR_ZERO_RETURN) { + return AsyncSocketException::END_OF_FILE; + } else if (sslErr == SSL_ERROR_SYSCALL) { + if (errError == 0 && sslOperationReturnValue == 0) { + return AsyncSocketException::END_OF_FILE; + } else { + return AsyncSocketException::NETWORK_ERROR; + } + } else { + // Assume an actual SSL error + return AsyncSocketException::SSL_ERROR; + } +} + +AsyncSocketException::AsyncSocketExceptionType exTypefromSSLErr(SSLError err) { + switch (err) { + case SSLError::EOF_ERROR: + return AsyncSocketException::END_OF_FILE; + case SSLError::NETWORK_ERROR: + return AsyncSocketException::NETWORK_ERROR; + default: + // everything else is a SSL_ERROR + return AsyncSocketException::SSL_ERROR; + } +} } namespace folly { @@ -78,7 +108,7 @@ SSLException::SSLException( int sslOperationReturnValue, int errno_copy) : AsyncSocketException( - AsyncSocketException::SSL_ERROR, + exTypefromSSLErrInfo(sslErr, errError, sslOperationReturnValue), decodeOpenSSLError(sslErr, errError, sslOperationReturnValue), sslErr == SSL_ERROR_SYSCALL ? errno_copy : 0) { if (sslErr == SSL_ERROR_ZERO_RETURN) { @@ -93,7 +123,7 @@ SSLException::SSLException( SSLException::SSLException(SSLError error) : AsyncSocketException( - AsyncSocketException::SSL_ERROR, + exTypefromSSLErr(error), getSSLErrorString(error).str(), 0), sslError(error) {} diff --git a/folly/io/async/ssl/SSLErrors.h b/folly/io/async/ssl/SSLErrors.h index 5bd37fde..ad7475b4 100644 --- a/folly/io/async/ssl/SSLErrors.h +++ b/folly/io/async/ssl/SSLErrors.h @@ -39,7 +39,7 @@ class SSLException : public folly::AsyncSocketException { explicit SSLException(SSLError error); - SSLError getType() const { + SSLError getSSLError() const { return sslError; } diff --git a/folly/io/async/test/AsyncSocketExceptionTest.cpp b/folly/io/async/test/AsyncSocketExceptionTest.cpp index 662a3737..50c66599 100644 --- a/folly/io/async/test/AsyncSocketExceptionTest.cpp +++ b/folly/io/async/test/AsyncSocketExceptionTest.cpp @@ -13,8 +13,12 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -#include +#include + #include +#include +#include +#include using namespace testing; @@ -45,4 +49,47 @@ TEST(AsyncSocketException, SimpleTest) { std::string(ex2.what())); } +TEST(AsyncSocketException, SSLExceptionType) { + { + SSLException eof(SSL_ERROR_ZERO_RETURN, 0, 0, 0); + EXPECT_EQ(eof.getType(), AsyncSocketException::END_OF_FILE); + + SSLException netEof(SSL_ERROR_SYSCALL, 0, 0, 0); + EXPECT_EQ(netEof.getType(), AsyncSocketException::END_OF_FILE); + + SSLException netOther(SSL_ERROR_SYSCALL, 0, 1, 0); + EXPECT_EQ(netOther.getType(), AsyncSocketException::NETWORK_ERROR); + + std::array sslErrs{{SSL_ERROR_SSL, + SSL_ERROR_WANT_READ, + SSL_ERROR_WANT_WRITE, + SSL_ERROR_WANT_X509_LOOKUP, + SSL_ERROR_WANT_CONNECT, + SSL_ERROR_WANT_ACCEPT}}; + + for (auto& e : sslErrs) { + SSLException sslEx(e, 0, 0, 0); + EXPECT_EQ(sslEx.getType(), AsyncSocketException::SSL_ERROR); + } + } + + { + SSLException eof(SSLError::EOF_ERROR); + EXPECT_EQ(eof.getType(), AsyncSocketException::END_OF_FILE); + + SSLException net(SSLError::NETWORK_ERROR); + EXPECT_EQ(net.getType(), AsyncSocketException::NETWORK_ERROR); + + std::array errs{{SSLError::CLIENT_RENEGOTIATION, + SSLError::INVALID_RENEGOTIATION, + SSLError::EARLY_WRITE, + SSLError::SSL_ERROR}}; + + for (auto& e : errs) { + SSLException sslEx(e); + EXPECT_EQ(sslEx.getType(), AsyncSocketException::SSL_ERROR); + } + } +} + } // namespace folly -- 2.34.1