From 61f4f02a6f27510e96de59dda110f6e5de3236ba Mon Sep 17 00:00:00 2001 From: derek Date: Thu, 8 Dec 2011 02:59:37 -0500 Subject: [PATCH] Fixed: premature EOF issues & updated Windows implementation --- src/api/CMakeLists.txt | 2 +- src/api/internal/io/BamFtp_p.cpp | 24 +++----- src/api/internal/io/BamHttp_p.cpp | 7 +-- src/api/internal/io/HostAddress_p.cpp | 2 +- src/api/internal/io/HostInfo_p.cpp | 3 +- src/api/internal/io/NetWin_p.h | 6 +- src/api/internal/io/RollingBuffer_p.cpp | 6 +- src/api/internal/io/TcpSocketEngine_p.cpp | 7 ++- src/api/internal/io/TcpSocketEngine_p.h | 10 +++- src/api/internal/io/TcpSocketEngine_win_p.cpp | 58 ++++--------------- src/api/internal/io/TcpSocket_p.cpp | 58 ++++++++++--------- 11 files changed, 75 insertions(+), 108 deletions(-) diff --git a/src/api/CMakeLists.txt b/src/api/CMakeLists.txt index cdfc10b..1af7a7f 100644 --- a/src/api/CMakeLists.txt +++ b/src/api/CMakeLists.txt @@ -43,7 +43,7 @@ set_target_properties( BamTools-static PROPERTIES OUTPUT_NAME "bamtools" PREFIX "lib" ) -# link libraries with zlib automatically +# link libraries automatically with zlib (and Winsock2, if applicable) if( _WIN32 ) set( APILibs z ws2_32 ) else( _WIN32 ) diff --git a/src/api/internal/io/BamFtp_p.cpp b/src/api/internal/io/BamFtp_p.cpp index a90a357..b851401 100644 --- a/src/api/internal/io/BamFtp_p.cpp +++ b/src/api/internal/io/BamFtp_p.cpp @@ -2,7 +2,7 @@ // BamFtp_p.cpp (c) 2011 Derek Barnett // Marth Lab, Department of Biology, Boston College // --------------------------------------------------------------------------- -// Last modified: 10 November 2011 (DB) +// Last modified: 8 December 2011 (DB) // --------------------------------------------------------------------------- // Provides reading/writing of BAM files on FTP server // *************************************************************************** @@ -78,7 +78,7 @@ static inline string toLower(const string& s) { string out; const size_t sSize = s.size(); - out.reserve(sSize); + out.resize(sSize); for ( size_t i = 0; i < sSize; ++i ) out[i] = tolower(s[i]); return out; @@ -136,7 +136,7 @@ bool BamFtp::ConnectCommandSocket(void) { // connect to FTP server if ( !m_commandSocket->ConnectToHost(m_hostname, m_port, m_mode) ) { - SetErrorString("BamFtp::ConnectCommandSocket", "could not connect to host"); + SetErrorString("BamFtp::ConnectCommandSocket", "could not connect to host - "); return false; } @@ -180,7 +180,7 @@ bool BamFtp::ConnectDataSocket(void) { } // make sure we're starting with a fresh data channel - if ( m_dataSocket->IsConnected() ) + if ( m_dataSocket->IsConnected() ) m_dataSocket->DisconnectFromHost(); // send passive connection command @@ -227,7 +227,7 @@ bool BamFtp::ConnectDataSocket(void) { m_dataSocket->DisconnectFromHost(); return false; } - + // make sure we have reply code 150 (all good) if ( !startsWith(m_response, "150") ) { // TODO: set error string @@ -368,21 +368,11 @@ int64_t BamFtp::Read(char* data, const unsigned int numBytes) { } int64_t BamFtp::ReadCommandSocket(char* data, const unsigned int maxNumBytes) { - - // try to read 'remainingBytes' from socket - const int64_t numBytesRead = m_commandSocket->Read(data, maxNumBytes); - if ( numBytesRead < 0 ) - return -1; - return numBytesRead; + return m_commandSocket->Read(data, maxNumBytes); } int64_t BamFtp::ReadDataSocket(char* data, const unsigned int maxNumBytes) { - - // try to read 'remainingBytes' from socket - const int64_t numBytesRead = m_dataSocket->Read(data, maxNumBytes); - if ( numBytesRead < 0 ) - return -1; - return numBytesRead; + return m_dataSocket->Read(data, maxNumBytes); } bool BamFtp::ReceiveReply(void) { diff --git a/src/api/internal/io/BamHttp_p.cpp b/src/api/internal/io/BamHttp_p.cpp index ba57337..377be82 100644 --- a/src/api/internal/io/BamHttp_p.cpp +++ b/src/api/internal/io/BamHttp_p.cpp @@ -275,12 +275,7 @@ int64_t BamHttp::Read(char* data, const unsigned int numBytes) { } int64_t BamHttp::ReadFromSocket(char* data, const unsigned int maxNumBytes) { - - // try to read 'remainingBytes' from socket - const int64_t numBytesRead = m_socket->Read(data, maxNumBytes); - if ( numBytesRead < 0 ) - return -1; - return numBytesRead; + return m_socket->Read(data, maxNumBytes); } bool BamHttp::ReceiveResponse(void) { diff --git a/src/api/internal/io/HostAddress_p.cpp b/src/api/internal/io/HostAddress_p.cpp index 873087b..5c42c5b 100644 --- a/src/api/internal/io/HostAddress_p.cpp +++ b/src/api/internal/io/HostAddress_p.cpp @@ -2,7 +2,7 @@ // HostAddress_p.cpp (c) 2011 Derek Barnett // Marth Lab, Department of Biology, Boston College // --------------------------------------------------------------------------- -// Last modified: 10 November 2011 (DB) +// Last modified: 8 December 2011 (DB) // --------------------------------------------------------------------------- // Provides a generic IP address container // *************************************************************************** diff --git a/src/api/internal/io/HostInfo_p.cpp b/src/api/internal/io/HostInfo_p.cpp index 80343f1..40b1047 100644 --- a/src/api/internal/io/HostInfo_p.cpp +++ b/src/api/internal/io/HostInfo_p.cpp @@ -2,7 +2,7 @@ // HostInfo_p.cpp (c) 2011 Derek Barnett // Marth Lab, Department of Biology, Boston College // --------------------------------------------------------------------------- -// Last modified: 10 November 2011 (DB) +// Last modified: 8 December 2011 (DB) // --------------------------------------------------------------------------- // Provides DNS lookup functionality for hostname & its discovered addresses // *************************************************************************** @@ -81,6 +81,7 @@ void HostInfo::SetHostName(const string& name) { HostInfo HostInfo::Lookup(const string& hostname, const string& port) { HostInfo result; + result.SetHostName(hostname); set uniqueAddresses; #ifdef _WIN32 diff --git a/src/api/internal/io/NetWin_p.h b/src/api/internal/io/NetWin_p.h index bcef955..3796e01 100644 --- a/src/api/internal/io/NetWin_p.h +++ b/src/api/internal/io/NetWin_p.h @@ -2,11 +2,11 @@ // NetWin_p.h (c) 2011 Derek Barnett // Marth Lab, Department of Biology, Boston College // --------------------------------------------------------------------------- -// Last modified: 10 November 2011 (DB) +// Last modified: 8 December 2011 (DB) // --------------------------------------------------------------------------- // Provides common networking-related includes, etc. for Windows systems // -// Note: only supports XP and later +// Note: requires Windows XP or later // *************************************************************************** #ifndef NETWIN_P_H @@ -38,7 +38,7 @@ namespace BamTools { namespace Internal { -// use RAII to ensure WSA is en +// use RAII to ensure WSA is initialized class WindowsSockInit { public: WindowsSockInit(void) { diff --git a/src/api/internal/io/RollingBuffer_p.cpp b/src/api/internal/io/RollingBuffer_p.cpp index c3f709d..10e7627 100644 --- a/src/api/internal/io/RollingBuffer_p.cpp +++ b/src/api/internal/io/RollingBuffer_p.cpp @@ -2,7 +2,7 @@ // RollingBuffer_p.cpp (c) 2011 Derek Barnett // Marth Lab, Department of Biology, Boston College // --------------------------------------------------------------------------- -// Last modified: 10 November 2011 (DB) +// Last modified: 8 December 2011 (DB) // --------------------------------------------------------------------------- // Provides a dynamic I/O FIFO byte queue, which removes bytes as they are // read from the front of the buffer and grows to accept bytes being written @@ -167,6 +167,10 @@ void RollingBuffer::Free(size_t n) { size_t RollingBuffer::IndexOf(char c) const { + // skip processing if empty buffer + if ( IsEmpty() ) + return string::npos; + size_t index(0); // iterate over byte arrays diff --git a/src/api/internal/io/TcpSocketEngine_p.cpp b/src/api/internal/io/TcpSocketEngine_p.cpp index 467eaeb..65587b2 100644 --- a/src/api/internal/io/TcpSocketEngine_p.cpp +++ b/src/api/internal/io/TcpSocketEngine_p.cpp @@ -2,7 +2,7 @@ // TcpSocketEngine_p.cpp (c) 2011 Derek Barnett // Marth Lab, Department of Biology, Boston College // --------------------------------------------------------------------------- -// Last modified: 10 November 2011 (DB) +// Last modified: 8 December 2011 (DB) // --------------------------------------------------------------------------- // Provides low-level implementation of TCP I/O // *************************************************************************** @@ -13,6 +13,7 @@ #include "api/internal/io/HostInfo_p.h" #include "api/internal/io/TcpSocketEngine_p.h" + using namespace BamTools; using namespace BamTools::Internal; @@ -150,7 +151,7 @@ bool TcpSocketEngine::WaitForRead(int msec, bool* timedOut) { *timedOut = false; // need to wait for our socket to be ready to read - int ret = nativeSelect(msec, true); + const int ret = nativeSelect(msec, true); // if timed out if ( ret == 0 ) { @@ -169,7 +170,7 @@ bool TcpSocketEngine::WaitForWrite(int msec, bool* timedOut) { *timedOut = false; // need to wait for our socket to be ready to write - int ret = nativeSelect(msec, false); + const int ret = nativeSelect(msec, false); // if timed out if ( ret == 0 ) { diff --git a/src/api/internal/io/TcpSocketEngine_p.h b/src/api/internal/io/TcpSocketEngine_p.h index 1a1a944..9218278 100644 --- a/src/api/internal/io/TcpSocketEngine_p.h +++ b/src/api/internal/io/TcpSocketEngine_p.h @@ -2,7 +2,7 @@ // TcpSocketEngine_p.h (c) 2011 Derek Barnett // Marth Lab, Department of Biology, Boston College // --------------------------------------------------------------------------- -// Last modified: 10 November 2011 (DB) +// Last modified: 8 December 2011 (DB) // --------------------------------------------------------------------------- // Provides low-level implementation of TCP I/O // *************************************************************************** @@ -23,6 +23,10 @@ #include "api/internal/io/HostAddress_p.h" #include "api/internal/io/TcpSocket_p.h" +#ifdef _WIN32 +# include "api/internal/io/NetWin_p.h" +#endif + namespace BamTools { namespace Internal { @@ -87,6 +91,10 @@ struct TcpSocketEngine { TcpSocket::SocketError m_socketError; TcpSocket::SocketState m_socketState; std::string m_errorString; + +#ifdef _WIN32 + WindowsSockInit m_win; +#endif }; } // namespace Internal diff --git a/src/api/internal/io/TcpSocketEngine_win_p.cpp b/src/api/internal/io/TcpSocketEngine_win_p.cpp index d1691ac..c4d9b47 100644 --- a/src/api/internal/io/TcpSocketEngine_win_p.cpp +++ b/src/api/internal/io/TcpSocketEngine_win_p.cpp @@ -2,7 +2,7 @@ // TcpSocketEngine_win_p.cpp (c) 2011 Derek Barnett // Marth Lab, Department of Biology, Boston College // --------------------------------------------------------------------------- -// Last modified: 15 November 2011 (DB) +// Last modified: 8 December 2011 (DB) // --------------------------------------------------------------------------- // Provides low-level implementation of TCP I/O for all Windows systems // *************************************************************************** @@ -14,19 +14,9 @@ using namespace BamTools::Internal; #include #include +#include using namespace std; -// ------------------------ -// static utility methods -// ------------------------ - -namespace BamTools { -namespace Internal { - - -} // namespace Internal -} // namespace BamTools - // -------------------------------- // TcpSocketEngine implementation // -------------------------------- @@ -162,6 +152,10 @@ bool TcpSocketEngine::nativeCreateSocket(HostAddress::NetworkProtocol protocol) m_errorString = "out of resources"; break; default: + m_socketError = TcpSocket::UnknownSocketError; + stringstream errStream(""); + errStream << "WSA ErrorCode: " << errorCode; + m_errorString = errStream.str(); break; } @@ -203,23 +197,10 @@ int64_t TcpSocketEngine::nativeRead(char* dest, size_t max) { DWORD flags = 0; DWORD bytesRead = 0; const int readResult = WSARecv(m_socketDescriptor, &buf, 1, &bytesRead, &flags, 0, 0); + if ( readResult == SOCKET_ERROR ) + return -1; - // if error encountered - if ( readResult == SOCKET_ERROR ) { - const int errorCode = WSAGetLastError(); - switch ( errorCode ) { - case WSAEWOULDBLOCK: // nothing read this time, but more coming later - return -2; - default: - return -1; // on any other errors - } - } - - // check if nothing was read this time, but more is coming - if ( WSAGetLastError() == WSAEWOULDBLOCK ) - return -2; - - // otherwise return number of bytes read + // return number of bytes read return static_cast(bytesRead); } @@ -252,24 +233,9 @@ int64_t TcpSocketEngine::nativeWrite(const char* data, size_t length) { DWORD flags = 0; DWORD bytesWritten = 0; const int writeResult = WSASend(m_socketDescriptor, &buf, 1, &bytesWritten, flags, 0, 0); + if ( writeResult == SOCKET_ERROR ) + return -1; - // error encountered - if ( writeResult == SOCKET_ERROR ) { - - const int errorCode = WSAGetLastError(); - switch ( errorCode ) { - case WSAEWOULDBLOCK: - return 0; - case WSAECONNRESET: - case WSAECONNABORTED: - m_socketError = TcpSocket::NetworkError; - m_errorString = "connection reset or aborted"; - return -1; - default: - return -1; - } - } - - // otherwise return number of bytes written + // return number of bytes written return static_cast(bytesWritten); } diff --git a/src/api/internal/io/TcpSocket_p.cpp b/src/api/internal/io/TcpSocket_p.cpp index b27635e..6195710 100644 --- a/src/api/internal/io/TcpSocket_p.cpp +++ b/src/api/internal/io/TcpSocket_p.cpp @@ -2,7 +2,7 @@ // TcpSocket_p.cpp (c) 2011 Derek Barnett // Marth Lab, Department of Biology, Boston College // --------------------------------------------------------------------------- -// Last modified: 7 December 2011 (DB) +// Last modified: 8 December 2011 (DB) // --------------------------------------------------------------------------- // Provides basic TCP I/O interface // *************************************************************************** @@ -69,14 +69,14 @@ bool TcpSocket::ConnectImpl(const HostInfo& hostInfo, { // skip if we're already connected if ( m_state == TcpSocket::ConnectedState ) { - m_error = TcpSocket::SocketResourceError; + m_error = TcpSocket::SocketResourceError; m_errorString = "socket already connected"; return false; } // reset socket state m_hostName = hostInfo.HostName(); - m_mode = mode; + m_mode = mode; m_state = TcpSocket::UnconnectedState; m_error = TcpSocket::UnknownSocketError; // m_localPort = 0; @@ -154,7 +154,7 @@ bool TcpSocket::ConnectToHost(const string& hostName, // if host name was IP address ("x.x.x.x" or IPv6 format) // otherwise host name was 'plain-text' ("www.foo.bar") // we need to look up IP address(es) - if ( hostAddress.HasIPAddress() ) + if ( hostAddress.HasIPAddress() ) info.SetAddresses( vector(1, hostAddress) ); else info = HostInfo::Lookup(hostName, port); @@ -275,24 +275,25 @@ int64_t TcpSocket::ReadFromSocket(void) { // if we simply timed out if ( timedOut ) { + // TODO: get add'l error info from engine ? m_errorString = "TcpSocket::ReadFromSocket - timed out waiting for ready read"; - // get error from engine ? - return -1; } - // otherwise, there was an error + // otherwise, there was some other error else { + // TODO: get add'l error info from engine ? m_errorString = "TcpSocket::ReadFromSocket - encountered error while waiting for ready read"; - // get error from engine ? - return -1; } + + // return failure + return -1; } // get number of bytes available from socket const int64_t bytesToRead = m_engine->NumBytesAvailable(); if ( bytesToRead < 0 ) { + // TODO: get add'l error info from engine ? m_errorString = "TcpSocket::ReadFromSocket - encountered error while determining numBytesAvailable"; - // get error from engine ? return -1; } @@ -300,8 +301,8 @@ int64_t TcpSocket::ReadFromSocket(void) { char* buffer = m_readBuffer.Reserve(bytesToRead); const int64_t numBytesRead = m_engine->Read(buffer, bytesToRead); if ( numBytesRead == -1 ) { + // TODO: get add'l error info from engine ? m_errorString = "TcpSocket::ReadFromSocket - encountered error while reading bytes"; - // get error from engine ? } // return number of bytes actually read @@ -327,7 +328,7 @@ string TcpSocket::ReadLine(int64_t max) { int64_t readResult; do { - result.Resize( static_cast(std::min(bufferMax, result.Size() + DEFAULT_BUFFER_SIZE)) ); + result.Resize( static_cast(min(bufferMax, result.Size() + DEFAULT_BUFFER_SIZE)) ); readResult = ReadLine(result.Data()+readBytes, result.Size()-readBytes); if ( readResult > 0 || readBytes == 0 ) readBytes += readResult; @@ -347,13 +348,13 @@ string TcpSocket::ReadLine(int64_t max) { } int64_t TcpSocket::ReadLine(char* dest, size_t max) { - + // wait for buffer to contain line contents if ( !WaitForReadLine() ) { m_errorString = "TcpSocket::ReadLine - error waiting for read line"; return -1; } - + // leave room for null term if ( max < 2 ) return -1; @@ -393,11 +394,11 @@ bool TcpSocket::WaitForReadLine(void) { // wait until we can read a line (will return immediately if already capable) while ( !CanReadLine() ) { - if ( !ReadFromSocket() ) + if ( !ReadFromSocket() ) return false; } - // if we get here, success + // if we get here, success return true; } @@ -406,22 +407,23 @@ int64_t TcpSocket::Write(const char* data, const unsigned int numBytes) { // single-shot attempt at write (not buffered, just try to shove the data through socket) // this method purely exists to send 'small' HTTP requests/FTP commands from client to server - int64_t bytesWritten(0); - // wait for our socket to be write-able bool timedOut; const bool isReadyWrite = m_engine->WaitForWrite(3000, &timedOut); + + // if ready, return number of bytes written if ( isReadyWrite ) - bytesWritten = m_engine->Write(data, numBytes); + return m_engine->Write(data, numBytes); + + // otherwise, socket not ready for writing + // set error string depending on reason & return failure + if ( !timedOut ) { + // TODO: get add'l error info from engine ?? + m_errorString = "TcpSocket::Write - timed out waiting for ready-write"; + } else { - // timeout is OK (with current setup), we'll just return 0 & try again - // but we need to report if engine encountered some other error - if ( !timedOut ) { - // TODO: set error string - bytesWritten = -1; - } + // TODO: get add'l error info from engine ?? + m_errorString = "TcpSocket::Write - error encountered while waiting for ready-write"; } - - // return actual number of bytes written to socket - return bytesWritten; + return -1; } -- 2.39.2