From 35e09c46b982ae5fcd26cc9d74a752cdfa0e7ff2 Mon Sep 17 00:00:00 2001 From: Adam Potolsky Date: Fri, 22 May 2015 16:09:59 -0700 Subject: [PATCH 1/2] Changed secureSocket routines to return a status, and modify an argument for num of bytes handled #4697 --- src/lib/net/TCPSocket.cpp | 44 ++++++++++++++++-------------- src/lib/net/TCPSocket.h | 4 +-- src/lib/plugin/ns/SecureSocket.cpp | 37 ++++++++++++------------- src/lib/plugin/ns/SecureSocket.h | 4 +-- 4 files changed, 45 insertions(+), 44 deletions(-) diff --git a/src/lib/net/TCPSocket.cpp b/src/lib/net/TCPSocket.cpp index 620e26ec..ea0a8cb4 100644 --- a/src/lib/net/TCPSocket.cpp +++ b/src/lib/net/TCPSocket.cpp @@ -474,22 +474,23 @@ TCPSocket::serviceConnected(ISocketMultiplexerJob* job, if (write) { try { // write data - UInt32 n = m_outputBuffer.getSize(); + int buffSize = m_outputBuffer.getSize(); + int bytesWrote = 0; + int status = 0; if (s_retryOutputBufferSize > 0) { - n = s_retryOutputBufferSize; + buffSize = s_retryOutputBufferSize; } - const void* buffer = m_outputBuffer.peek(n); + const void* buffer = m_outputBuffer.peek(buffSize); if (isSecure()) { if (isSecureReady()) { - s_retryOutputBufferSize = n; - n = secureWrite(buffer, n); - - if (n > 0) { + s_retryOutputBufferSize = buffSize; + status = secureWrite(buffer, buffSize, bytesWrote); + if (status > 0) { s_retryOutputBufferSize = 0; } - else if (n < 0) { + else if (status < 0) { return NULL; } } @@ -498,12 +499,12 @@ TCPSocket::serviceConnected(ISocketMultiplexerJob* job, } } else { - n = (UInt32)ARCH->writeSocket(m_socket, buffer, n); + bytesWrote = (UInt32)ARCH->writeSocket(m_socket, buffer, buffSize); } // discard written data - if (n > 0) { - m_outputBuffer.pop(n); + if (bytesWrote > 0) { + m_outputBuffer.pop(bytesWrote); if (m_outputBuffer.getSize() == 0) { sendEvent(m_events->forIStream().outputFlushed()); m_flushed = true; @@ -543,12 +544,13 @@ TCPSocket::serviceConnected(ISocketMultiplexerJob* job, try { static UInt8 buffer[4096]; memset(buffer, 0, sizeof(buffer)); - size_t n = 0; + int bytesRead = 0; + int status = 0; if (isSecure()) { if (isSecureReady()) { - n = secureRead(buffer, sizeof(buffer)); - if (n < 0) { + status = secureRead(buffer, sizeof(buffer), bytesRead); + if (status < 0) { return NULL; } } @@ -557,27 +559,27 @@ TCPSocket::serviceConnected(ISocketMultiplexerJob* job, } } else { - n = ARCH->readSocket(m_socket, buffer, sizeof(buffer)); + bytesRead = (int) ARCH->readSocket(m_socket, buffer, sizeof(buffer)); } - if (n > 0) { + if (bytesRead > 0) { bool wasEmpty = (m_inputBuffer.getSize() == 0); // slurp up as much as possible do { - m_inputBuffer.write(buffer, (UInt32)n); + m_inputBuffer.write(buffer, bytesRead); if (isSecure() && isSecureReady()) { - n = secureRead(buffer, sizeof(buffer)); - if (n < 0) { + status = secureRead(buffer, sizeof(buffer), bytesRead); + if (status < 0) { return NULL; } } else { - n = ARCH->readSocket(m_socket, buffer, sizeof(buffer)); + bytesRead = (int) ARCH->readSocket(m_socket, buffer, sizeof(buffer)); } - } while (n > 0); + } while (bytesRead > 0 || status > 0); // send input ready if input buffer was empty if (wasEmpty) { diff --git a/src/lib/net/TCPSocket.h b/src/lib/net/TCPSocket.h index 831d4729..c30a2d7e 100644 --- a/src/lib/net/TCPSocket.h +++ b/src/lib/net/TCPSocket.h @@ -67,8 +67,8 @@ protected: IEventQueue* getEvents() { return m_events; } virtual bool isSecureReady() { return false; } virtual bool isSecure() { return false; } - virtual UInt32 secureRead(void* buffer, UInt32) { return 0; } - virtual UInt32 secureWrite(const void*, UInt32) { return 0; } + virtual int secureRead(void* buffer, int, int& ) { return 0; } + virtual int secureWrite(const void*, int, int& ) { return 0; } void setJob(ISocketMultiplexerJob*); ISocketMultiplexerJob* diff --git a/src/lib/plugin/ns/SecureSocket.cpp b/src/lib/plugin/ns/SecureSocket.cpp index d9e47d73..95425445 100644 --- a/src/lib/plugin/ns/SecureSocket.cpp +++ b/src/lib/plugin/ns/SecureSocket.cpp @@ -111,18 +111,17 @@ SecureSocket::secureAccept() getSocket(), isReadable(), isWritable())); } -UInt32 -SecureSocket::secureRead(void* buffer, UInt32 n) +int +SecureSocket::secureRead(void* buffer, int size, int& read) { - int r = 0; if (m_ssl->m_ssl != NULL) { LOG((CLOG_DEBUG2 "reading secure socket")); - r = SSL_read(m_ssl->m_ssl, buffer, n); + read = SSL_read(m_ssl->m_ssl, buffer, size); static int retry; // Check result will cleanup the connection in the case of a fatal - checkResult(r, retry); + checkResult(read, retry); if (retry) { return 0; @@ -132,25 +131,24 @@ SecureSocket::secureRead(void* buffer, UInt32 n) return -1; } } - // According to SSL spec, r must not be negative and not have an error code - // from SSL_get_error(). If this happens, it is itself an error. Let the - // parent handle the case - return (UInt32)r; + // According to SSL spec, the number of bytes read must not be negative and + // not have an error code from SSL_get_error(). If this happens, it is + // itself an error. Let the parent handle the case + return read; } -UInt32 -SecureSocket::secureWrite(const void* buffer, UInt32 n) +int +SecureSocket::secureWrite(const void* buffer, int size, int& wrote) { - int r = 0; if (m_ssl->m_ssl != NULL) { LOG((CLOG_DEBUG2 "writing secure socket:%p", this)); - r = SSL_write(m_ssl->m_ssl, buffer, n); + wrote = SSL_write(m_ssl->m_ssl, buffer, size); static int retry; // Check result will cleanup the connection in the case of a fatal - checkResult(r, retry); + checkResult(wrote, retry); if (retry) { return 0; @@ -163,7 +161,7 @@ SecureSocket::secureWrite(const void* buffer, UInt32 n) // According to SSL spec, r must not be negative and not have an error code // from SSL_get_error(). If this happens, it is itself an error. Let the // parent handle the case - return (UInt32)r; + return wrote; } bool @@ -378,12 +376,13 @@ SecureSocket::showCertificate() } void -SecureSocket::checkResult(int n, int& retry) +SecureSocket::checkResult(int status, int& retry) { // ssl errors are a little quirky. the "want" errors are normal and // should result in a retry. - int errorCode = SSL_get_error(m_ssl->m_ssl, n); + int errorCode = SSL_get_error(m_ssl->m_ssl, status); + switch (errorCode) { case SSL_ERROR_NONE: retry = 0; @@ -416,10 +415,10 @@ SecureSocket::checkResult(int n, int& retry) case SSL_ERROR_SYSCALL: LOG((CLOG_ERR "some secure socket I/O error occurred")); if (ERR_peek_error() == 0) { - if (n == 0) { + if (status == 0) { LOG((CLOG_ERR "an EOF violates the protocol")); } - else if (n == -1) { + else if (status == -1) { // underlying socket I/O reproted an error try { ARCH->throwErrorOnSocket(getSocket()); diff --git a/src/lib/plugin/ns/SecureSocket.h b/src/lib/plugin/ns/SecureSocket.h index 81a6c7f3..e1906991 100644 --- a/src/lib/plugin/ns/SecureSocket.h +++ b/src/lib/plugin/ns/SecureSocket.h @@ -48,8 +48,8 @@ public: void isFatal(bool b) { m_fatal = b; } bool isSecureReady(); bool isSecure() { return true; } - UInt32 secureRead(void* buffer, UInt32 n); - UInt32 secureWrite(const void* buffer, UInt32 n); + int secureRead(void* buffer, int size, int& read); + int secureWrite(const void* buffer, int size, int& wrote); void initSsl(bool server); bool loadCertificates(String& CertFile); void maxRetry(int limit) { m_maxRetry = limit; }; From 905dbfee90bac192a74d1cec3bd5c80b5e56d277 Mon Sep 17 00:00:00 2001 From: Adam Potolsky Date: Fri, 22 May 2015 16:26:40 -0700 Subject: [PATCH 2/2] Fixed order of initializers for mac build #4697 --- src/lib/plugin/ns/SecureSocket.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/lib/plugin/ns/SecureSocket.cpp b/src/lib/plugin/ns/SecureSocket.cpp index 91f90bce..cb65bd85 100644 --- a/src/lib/plugin/ns/SecureSocket.cpp +++ b/src/lib/plugin/ns/SecureSocket.cpp @@ -52,8 +52,8 @@ SecureSocket::SecureSocket( SocketMultiplexer* socketMultiplexer) : TCPSocket(events, socketMultiplexer), m_secureReady(false), - m_maxRetry(MAX_RETRY_COUNT), - m_fatal(false) + m_fatal(false), + m_maxRetry(MAX_RETRY_COUNT) { } @@ -63,8 +63,8 @@ SecureSocket::SecureSocket( ArchSocket socket) : TCPSocket(events, socketMultiplexer, socket), m_secureReady(false), - m_maxRetry(MAX_RETRY_COUNT), - m_fatal(false) + m_fatal(false), + m_maxRetry(MAX_RETRY_COUNT) { }