10 files changed, 104 insertions, 45 deletions
diff --git a/Swiften/Avatars/UnitTest/AvatarManagerImplTest.cpp b/Swiften/Avatars/UnitTest/AvatarManagerImplTest.cpp
index 241f375..5a35410 100644
--- a/Swiften/Avatars/UnitTest/AvatarManagerImplTest.cpp
+++ b/Swiften/Avatars/UnitTest/AvatarManagerImplTest.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014-2016 Isode Limited.
+ * Copyright (c) 2014-2018 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
@@ -91,6 +91,7 @@ class AvatarManagerImplTest : public CppUnit::TestFixture {
             /* send new presence to notify of blank avatar */
 
             vcardUpdate = std::make_shared<VCardUpdate>();
+            vcardUpdate->setPhotoHash("da39a3ee5e6b4b0d3255bfef95601890afd80709");
             presence = std::make_shared<Presence>();
             presence->setTo(ownerJID);
             presence->setFrom(personJID);
diff --git a/Swiften/Avatars/VCardUpdateAvatarManager.cpp b/Swiften/Avatars/VCardUpdateAvatarManager.cpp
index 3e8d87b..349af2f 100644
--- a/Swiften/Avatars/VCardUpdateAvatarManager.cpp
+++ b/Swiften/Avatars/VCardUpdateAvatarManager.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010-2016 Isode Limited.
+ * Copyright (c) 2010-2018 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
@@ -32,6 +32,10 @@ void VCardUpdateAvatarManager::handlePresenceReceived(std::shared_ptr<Presence>
         return;
     }
     JID from = getAvatarJID(presence->getFrom());
+    if (update->getPhotoHash().size() != 40) {
+        SWIFT_LOG(debug) << "Invalid vCard avatar photo hash length. Must be hex-encoded SHA-1, i.e. 40 characters." << std::endl;
+        return;
+    }
     if (getAvatarHash(from) == update->getPhotoHash()) {
         return;
     }
diff --git a/Swiften/Base/Log.cpp b/Swiften/Base/Log.cpp
index 0efac7e..9b16531 100644
--- a/Swiften/Base/Log.cpp
+++ b/Swiften/Base/Log.cpp
@@ -16,6 +16,7 @@
 namespace Swift {
 
 static Log::Severity logLevel = Log::warning;
+std::unique_ptr<FILE, Log::LogFileClose> Log::logfile;
 
 Log::Log() {
 }
@@ -25,8 +26,14 @@ Log::~Log() {
     __android_log_print(ANDROID_LOG_VERBOSE, "Swift", stream.str().c_str(), 1);
 #else
     // Using stdio for thread safety (POSIX file i/o calls are guaranteed to be atomic)
-    fprintf(stderr, "%s", stream.str().c_str());
-    fflush(stderr);
+    if (logfile) {
+        fwrite(stream.str().c_str(), sizeof(char), stream.str().size(), logfile.get());
+        fflush(logfile.get());
+    }
+    else {
+        fwrite(stream.str().c_str(), sizeof(char), stream.str().size(), stderr);
+        fflush(stderr);
+    }
 #endif
 }
 
@@ -48,4 +55,10 @@ void Log::setLogLevel(Severity level) {
     logLevel = level;
 }
 
+void Log::setLogFile(const std::string& fileName) {
+    if (!fileName.empty()) {
+        logfile = std::unique_ptr<FILE, Log::LogFileClose>(fopen(fileName.c_str(), "a"));
+    }
+}
+
 }
diff --git a/Swiften/Base/Log.h b/Swiften/Base/Log.h
index 33c969d..e3e04a5 100644
--- a/Swiften/Base/Log.h
+++ b/Swiften/Base/Log.h
@@ -6,6 +6,8 @@
 
 #pragma once
 
+#include <cstdio>
+#include <memory>
 #include <sstream>
 
 #include <Swiften/Base/API.h>
@@ -29,9 +31,18 @@ namespace Swift {
 
             static Severity getLogLevel();
             static void setLogLevel(Severity level);
+            static void setLogFile(const std::string& fileName);
 
         private:
+            struct LogFileClose {
+                void operator()(FILE* p) {
+                    if (p) {
+                        fclose(p);
+                    }
+                }
+            };
             std::ostringstream stream;
+            static std::unique_ptr<FILE, LogFileClose> logfile;
     };
 }
 
diff --git a/Swiften/ChangeLog.md b/Swiften/ChangeLog.md
index d823954..a664341 100644
--- a/Swiften/ChangeLog.md
+++ b/Swiften/ChangeLog.md
@@ -1,3 +1,20 @@
+4.0 (2017-03-20)
+----------------
+- Moved code-base to C++11
+    - Use C++11 threading instead of Boost.Thread library
+    - Use C++11 smart pointers instead of Boost's
+- Migrated from Boost.Signals to Boost.Signals2
+- Build without warnings on our CI platforms
+- General cleanup like remove of superflous files and #include statements. This means header files that previously were included implictly need to be explicitly included now
+- Support IPv6 addresses in URLs
+- Handle sessions being closed by the server
+- Verify certificates when using HTTPS in BOSH connections
+- In memory caching of latest entity capabilites lookups
+- Changed source code style to use soft tabs (4 spaces wide) instead of hard tabs. Custom patches for Swiften will need to be reformatted accordingly
+- Require a TLS backend for building
+- Update 3rdParty/lcov to version 1.12
+- Fix several possible race conditions, smaller leaks, and other small bugs
+
 4.0-rc1 ( 2017-05-17 )
 ----------------------
 - Handle sessions being closed by the server
diff --git a/Swiften/Client/ClientSession.cpp b/Swiften/Client/ClientSession.cpp
index 661a832..bb9be58 100644
--- a/Swiften/Client/ClientSession.cpp
+++ b/Swiften/Client/ClientSession.cpp
@@ -231,13 +231,13 @@ void ClientSession::handleElement(std::shared_ptr<ToplevelElement> element) {
 #ifdef SWIFTEN_PLATFORM_WIN32
             if (singleSignOn) {
                 const boost::optional<std::string> authenticationHostname = streamFeatures->getAuthenticationHostname();
-                bool gssapiSupported = streamFeatures->hasAuthenticationMechanism("GSSAPI") && authenticationHostname && !authenticationHostname->empty();
+                bool gssapiSupported = streamFeatures->hasAuthenticationMechanism("GSSAPI");
 
                 if (!gssapiSupported) {
                     finishSession(Error::NoSupportedAuthMechanismsError);
                 }
                 else {
-                    WindowsGSSAPIClientAuthenticator* gssapiAuthenticator = new WindowsGSSAPIClientAuthenticator(*authenticationHostname, localJID.getDomain(), authenticationPort);
+                    WindowsGSSAPIClientAuthenticator* gssapiAuthenticator = new WindowsGSSAPIClientAuthenticator(authenticationHostname.value_or(""), localJID.getDomain(), authenticationPort);
                     std::shared_ptr<Error> error = std::make_shared<Error>(Error::AuthenticationFailedError);
 
                     authenticator = gssapiAuthenticator;
diff --git a/Swiften/Network/Connector.cpp b/Swiften/Network/Connector.cpp
index 457d8a9..5eddaba 100644
--- a/Swiften/Network/Connector.cpp
+++ b/Swiften/Network/Connector.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010-2016 Isode Limited.
+ * Copyright (c) 2010-2018 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
@@ -148,6 +148,13 @@ void Connector::handleConnectionConnectFinished(bool error) {
             timer->stop();
             timer.reset();
     }
+    if (!currentConnection) {
+        // We've hit a race condition where multiple finisheds were on the eventloop queue at once.
+        // This is particularly likely on macOS where the hourly momentary wakeup while asleep
+        // can cause both a timeout and an onConnectFinished to be queued sequentially (SWIFT-232).
+        // Let the first one process as normal, but ignore the second.
+        return;
+    }
     currentConnection->onConnectFinished.disconnect(boost::bind(&Connector::handleConnectionConnectFinished, shared_from_this(), _1));
     if (error) {
         currentConnection.reset();
@@ -189,6 +196,7 @@ void Connector::finish(std::shared_ptr<Connection> connection) {
 
 void Connector::handleTimeout() {
     SWIFT_LOG(debug) << "Timeout" << std::endl;
+    SWIFT_LOG_ASSERT(currentConnection, error) << "Connection not valid but triggered a timeout" <<std::endl;
     handleConnectionConnectFinished(true);
 }
 
diff --git a/Swiften/SConscript b/Swiften/SConscript
index 18458ee..5fb3b47 100644
--- a/Swiften/SConscript
+++ b/Swiften/SConscript
@@ -639,8 +639,9 @@ if env["SCONS_STAGE"] == "build" :
 
     # Install swiften
     if swiften_env.get("SWIFTEN_INSTALLDIR", "") :
-        swiften_env.Install(os.path.join(swiften_env["SWIFTEN_INSTALLDIR"], "lib"), swiften_lib)
+        swiften_libdir = swiften_env.get("SWIFTEN_LIBDIR", "lib")
+        swiften_env.Install(os.path.join(swiften_env["SWIFTEN_INSTALLDIR"], swiften_libdir), swiften_lib)
         for alias in myenv["SWIFTEN_LIBRARY_ALIASES"] :
-            myenv.Command(myenv.File(os.path.join(swiften_env["SWIFTEN_INSTALLDIR"], "lib", alias)), [env.Value(swiften_lib[0].name), swiften_lib[0]], symlink)
+            myenv.Command(myenv.File(os.path.join(swiften_env["SWIFTEN_INSTALLDIR"], swiften_libdir, alias)), [env.Value(swiften_lib[0].name), swiften_lib[0]], symlink)
         for include in swiften_includes :
             swiften_env.Install(os.path.join(swiften_env["SWIFTEN_INSTALLDIR"], "include", os.path.dirname(include)), "#/" + include)
diff --git a/Swiften/TLS/CAPICertificate.cpp b/Swiften/TLS/CAPICertificate.cpp
index a46b9f6..f10ad47 100644
--- a/Swiften/TLS/CAPICertificate.cpp
+++ b/Swiften/TLS/CAPICertificate.cpp
@@ -14,6 +14,7 @@
 #include <Swiften/Base/Log.h>
 #include <Swiften/Network/TimerFactory.h>
 #include <Swiften/StringCodecs/Hexify.h>
+#include <Swiften/TLS/Schannel/SchannelUtil.h>
 
 // Size of the SHA1 hash
 #define SHA1_HASH_LEN 20
@@ -43,6 +44,7 @@ CAPICertificate::CAPICertificate(const std::string& capiUri, TimerFactory* timer
 }
 
 CAPICertificate::~CAPICertificate() {
+    SWIFT_LOG(debug) << "Destroying the CAPICertificate" << std::endl;
     if (smartCardTimer_) {
         smartCardTimer_->stop();
         smartCardTimer_->onTick.disconnect(boost::bind(&CAPICertificate::handleSmartCardTimerTick, this));
@@ -50,7 +52,9 @@ CAPICertificate::~CAPICertificate() {
     }
 
     if (certStoreHandle_) {
-        CertCloseStore(certStoreHandle_, 0);
+        if (CertCloseStore(certStoreHandle_, 0) == FALSE) {
+            SWIFT_LOG(debug) << "Failed to close the certificate store handle" << std::endl;
+        }
     }
 
     if (cardHandle_) {
@@ -80,7 +84,7 @@ const std::string& CAPICertificate::getSmartCardReaderName() const {
     return smartCardReaderName_;
 }
 
-PCCERT_CONTEXT findCertificateInStore (HCERTSTORE certStoreHandle, const std::string &certName) {
+PCCERT_CONTEXT findCertificateInStore(HCERTSTORE certStoreHandle, const std::string &certName) {
     if (!boost::iequals(certName.substr(0, 5), "sha1:")) {
 
         // Find client certificate. Note that this sample just searches for a
@@ -105,8 +109,7 @@ PCCERT_CONTEXT findCertificateInStore (HCERTSTORE certStoreHandle, const std::st
 
 }
 
-
-void CAPICertificate::setUri (const std::string& capiUri) {
+void CAPICertificate::setUri(const std::string& capiUri) {
     valid_ = false;
 
     /* Syntax: "certstore:" <cert_store> ":" <hash> ":" <hash_of_cert> */
@@ -118,7 +121,7 @@ void CAPICertificate::setUri (const std::string& capiUri) {
     /* Substring of subject: uses "storename" */
     std::string capiIdentity = capiUri.substr(10);
     std::string newCertStoreName;
-    size_t pos = capiIdentity.find_first_of (':');
+    size_t pos = capiIdentity.find_first_of(':');
 
     if (pos == std::string::npos) {
         /* Using the default certificate store */
@@ -146,49 +149,38 @@ void CAPICertificate::setUri (const std::string& capiUri) {
 
     certStore_ = newCertStoreName;
 
-    PCCERT_CONTEXT certContext = findCertificateInStore (certStoreHandle_, certName_);
-
+    ScopedCertContext certContext(findCertificateInStore(certStoreHandle_, certName_));
     if (!certContext) {
         return;
     }
 
-
     /* Now verify that we can have access to the corresponding private key */
 
     DWORD len;
-    CRYPT_KEY_PROV_INFO *pinfo;
-    HCRYPTPROV hprov;
-    HCRYPTKEY key;
-
     if (!CertGetCertificateContextProperty(certContext,
             CERT_KEY_PROV_INFO_PROP_ID,
             NULL,
             &len)) {
-        CertFreeCertificateContext(certContext);
+        SWIFT_LOG(error) << "Error while retrieving context properties" << std::endl;
         return;
     }
 
-    pinfo = static_cast<CRYPT_KEY_PROV_INFO *>(malloc(len));
+    std::shared_ptr<CRYPT_KEY_PROV_INFO> pinfo(static_cast<CRYPT_KEY_PROV_INFO *>(malloc(len)), free);
     if (!pinfo) {
-        CertFreeCertificateContext(certContext);
         return;
     }
 
-    if (!CertGetCertificateContextProperty(certContext, CERT_KEY_PROV_INFO_PROP_ID, pinfo, &len)) {
-        CertFreeCertificateContext(certContext);
-        free(pinfo);
+    if (!CertGetCertificateContextProperty(certContext, CERT_KEY_PROV_INFO_PROP_ID, pinfo.get(), &len)) {
         return;
     }
+    certContext.FreeContext();
 
-    CertFreeCertificateContext(certContext);
-
+    HCRYPTPROV hprov;
     // Now verify if we have access to the private key
     if (!CryptAcquireContextW(&hprov, pinfo->pwszContainerName, pinfo->pwszProvName, pinfo->dwProvType, 0)) {
-        free(pinfo);
         return;
     }
 
-
     char smartCardReader[1024];
     DWORD bufferLength = sizeof(smartCardReader);
     if (!CryptGetProvParam(hprov, PP_SMARTCARD_READER, (BYTE *)&smartCardReader, &bufferLength, 0)) {
@@ -205,19 +197,19 @@ void CAPICertificate::setUri (const std::string& capiUri) {
             smartCardTimer_ = timerFactory_->createTimer(SMARTCARD_EJECTION_CHECK_FREQUENCY_MILLISECONDS);
         }
         else {
-            ///Need to handle an error here
+            CryptReleaseContext(hprov, 0);
+            return;
         }
     }
 
+    HCRYPTKEY key;
     if (!CryptGetUserKey(hprov, pinfo->dwKeySpec, &key)) {
         CryptReleaseContext(hprov, 0);
-        free(pinfo);
         return;
     }
 
     CryptDestroyKey(key);
     CryptReleaseContext(hprov, 0);
-    free(pinfo);
 
     if (smartCardTimer_) {
         smartCardTimer_->onTick.connect(boost::bind(&CAPICertificate::handleSmartCardTimerTick, this));
@@ -227,7 +219,7 @@ void CAPICertificate::setUri (const std::string& capiUri) {
     valid_ = true;
 }
 
-static void smartcard_check_status (SCARDCONTEXT hContext,
+static void smartcard_check_status(SCARDCONTEXT hContext,
         const char* pReader,
         SCARDHANDLE hCardHandle, /* Can be 0 on the first call */
         SCARDHANDLE* newCardHandle, /* The handle returned */
@@ -288,7 +280,7 @@ static void smartcard_check_status (SCARDCONTEXT hContext,
     }
 }
 
-bool CAPICertificate::checkIfSmartCardPresent () {
+bool CAPICertificate::checkIfSmartCardPresent() {
     if (!smartCardReaderName_.empty()) {
         DWORD dwState;
         smartcard_check_status(scardContext_, smartCardReaderName_.c_str(), cardHandle_, &cardHandle_, &dwState);
@@ -317,8 +309,6 @@ bool CAPICertificate::checkIfSmartCardPresent () {
                 break;
         }
 
-
-
         switch (dwState) {
             case SCARD_ABSENT:
                 return false;
@@ -342,6 +332,7 @@ bool CAPICertificate::checkIfSmartCardPresent () {
 void CAPICertificate::handleSmartCardTimerTick() {
     bool poll = checkIfSmartCardPresent();
     if (lastPollingResult_ && !poll) {
+        SWIFT_LOG(debug) << "CAPI Certificate detected that the certificate card was removed" << std::endl;
         onCertificateCardRemoved();
     }
     lastPollingResult_ = poll;
diff --git a/Swiften/TLS/Schannel/SchannelContext.cpp b/Swiften/TLS/Schannel/SchannelContext.cpp
index 5799157..c07d009 100644
--- a/Swiften/TLS/Schannel/SchannelContext.cpp
+++ b/Swiften/TLS/Schannel/SchannelContext.cpp
@@ -16,6 +16,7 @@
 
 #include <WinHTTP.h> /* For SECURITY_FLAG_IGNORE_CERT_CN_INVALID */
 
+#include <Swiften/Base/Log.h>
 #include <Swiften/TLS/CAPICertificate.h>
 #include <Swiften/TLS/Schannel/SchannelCertificate.h>
 
@@ -39,13 +40,20 @@ SchannelContext::SchannelContext(bool tls1_0Workaround) : state_(Start), secCont
 //------------------------------------------------------------------------
 
 SchannelContext::~SchannelContext() {
-    if (myCertStore_) CertCloseStore(myCertStore_, 0);
+    SWIFT_LOG(debug) << "Destroying SchannelContext" << std::endl;
+    if (myCertStore_) {
+        if (CertCloseStore(myCertStore_, 0) == FALSE) {
+            SWIFT_LOG(debug) << "Failed to close the certificate store" << std::endl;
+        }
+    }
 }
 
 //------------------------------------------------------------------------
 
 void SchannelContext::determineStreamSizes() {
-    QueryContextAttributes(contextHandle_, SECPKG_ATTR_STREAM_SIZES, &streamSizes_);
+    if (QueryContextAttributes(contextHandle_, SECPKG_ATTR_STREAM_SIZES, &streamSizes_) != SEC_E_OK) {
+        SWIFT_LOG(debug) << "QueryContextAttributes failed to determinate the stream size" << std::endl;
+    }
 }
 
 //------------------------------------------------------------------------
@@ -267,8 +275,8 @@ void SchannelContext::continueHandshake(const SafeByteArray& data) {
         SecBuffer inBuffers[2];
 
         // Provide Schannel with the remote host's handshake data
-        inBuffers[0].pvBuffer     = (char*)(&receivedData_[0]);
-        inBuffers[0].cbBuffer     = (unsigned long)receivedData_.size();
+        inBuffers[0].pvBuffer = static_cast<char*>(&receivedData_[0]);
+        inBuffers[0].cbBuffer = static_cast<unsigned long>(receivedData_.size());
         inBuffers[0].BufferType  = SECBUFFER_TOKEN;
 
         inBuffers[1].pvBuffer   = NULL;
@@ -483,8 +491,8 @@ void SchannelContext::decryptAndProcessData(const SafeByteArray& data) {
         //   contexts. Additionally, a second SECBUFFER_TOKEN type buffer that contains a security token
         //   must also be supplied.
         //
-        inBuffers[0].pvBuffer     = (char*)(&receivedData_[0]);
-        inBuffers[0].cbBuffer     = (unsigned long)receivedData_.size();
+        inBuffers[0].pvBuffer = static_cast<char*>(&receivedData_[0]);
+        inBuffers[0].cbBuffer = static_cast<unsigned long>(receivedData_.size());
         inBuffers[0].BufferType  = SECBUFFER_DATA;
 
         inBuffers[1].BufferType  = SECBUFFER_EMPTY;
@@ -578,7 +586,7 @@ void SchannelContext::encryptAndSendData(const SafeByteArray& data) {
         outBuffers[0].BufferType = SECBUFFER_STREAM_HEADER;
 
         outBuffers[1].pvBuffer     = &sendBuffer[0] + streamSizes_.cbHeader;
-        outBuffers[1].cbBuffer     = (unsigned long)bytesToSend;
+        outBuffers[1].cbBuffer     = static_cast<unsigned long>(bytesToSend);
         outBuffers[1].BufferType = SECBUFFER_DATA;
 
         outBuffers[2].pvBuffer     = &sendBuffer[0] + streamSizes_.cbHeader + bytesToSend;
@@ -645,6 +653,7 @@ std::vector<Certificate::ref> SchannelContext::getPeerCertificateChain() const {
     SECURITY_STATUS status = QueryContextAttributes(contextHandle_, SECPKG_ATTR_REMOTE_CERT_CONTEXT, pServerCert.Reset());
 
     if (status != SEC_E_OK) {
+        SWIFT_LOG(debug) << "Error while Querying the Certificate Chain" << std::endl;
         return certificateChain;
     }
     certificateChain.push_back(std::make_shared<SchannelCertificate>(pServerCert));
@@ -678,6 +687,10 @@ ByteArray SchannelContext::getFinishMessage() const {
     if (ret == SEC_E_OK) {
         return createByteArray(((unsigned char*) bindings.Bindings) + bindings.Bindings->dwApplicationDataOffset + 11 /* tls-unique:*/, bindings.Bindings->cbApplicationDataLength - 11);
     }
+    else {
+        SWIFT_LOG(debug) << "Error while retrieving Finish Message" << std::endl;
+    }
+
     return ByteArray();
 }