QSslConfiguration: do not lazily construct the d-pointer

...the private class is cheap anyway; and lazy construction lead to
problems like setting an empty default configuration would crash etc.

Reviewed-by: Markus Goetz
Task-number: QTBUG-17550

7 files changed, 113 insertions, 26 deletions

diff --git a/src/network/ssl/qsslconfiguration.cpp b/src/network/ssl/qsslconfiguration.cpp
index 150f77e..c8dbaed 100644
--- a/src/network/ssl/qsslconfiguration.cpp
+++ b/src/network/ssl/qsslconfiguration.cpp
@@ -47,18 +47,6 @@
 
 QT_BEGIN_NAMESPACE
 
-template<> void QSharedDataPointer<QSslConfigurationPrivate>::detach()
-{
-    if (d && d->ref == 1)
-        return;
-    QSslConfigurationPrivate *x = (d ? new QSslConfigurationPrivate(*d)
-                                   : new QSslConfigurationPrivate);
-    x->ref.ref();
-    if (d && !d->ref.deref())
-        delete d;
-    d = x;
-}
-
 /*!
     \class QSslConfiguration
     \brief The QSslConfiguration class holds the configuration and state of an SSL connection
@@ -126,7 +114,7 @@ template<> void QSharedDataPointer<QSslConfigurationPrivate>::detach()
     Once any setter methods are called, isNull() will return false.
 */
 QSslConfiguration::QSslConfiguration()
-    : d(0)
+    : d(new QSslConfigurationPrivate)
 {
 }
 
@@ -203,7 +191,15 @@ bool QSslConfiguration::operator==(const QSslConfiguration &other) const
 */
 bool QSslConfiguration::isNull() const
 {
-    return d == 0;
+    return (d->protocol == QSsl::SecureProtocols &&
+            d->peerVerifyMode == QSslSocket::AutoVerifyPeer &&
+            d->peerVerifyDepth == 0 &&
+            d->caCertificates.count() == 0 &&
+            d->ciphers.count() == 0 &&
+            d->localCertificate.isNull() &&
+            d->privateKey.isNull() &&
+            d->peerCertificate.isNull() &&
+            d->peerCertificateChain.count() == 0);
 }
 
 /*!
@@ -213,7 +209,7 @@ bool QSslConfiguration::isNull() const
 */
 QSsl::SslProtocol QSslConfiguration::protocol() const
 {
-    return d ? d->protocol : QSsl::SecureProtocols;
+    return d->protocol;
 }
 
 /*!
@@ -243,7 +239,7 @@ void QSslConfiguration::setProtocol(QSsl::SslProtocol protocol)
 */
 QSslSocket::PeerVerifyMode QSslConfiguration::peerVerifyMode() const
 {
-    return d ? d->peerVerifyMode : QSslSocket::AutoVerifyPeer;
+    return d->peerVerifyMode;
 }
 
 /*!
@@ -276,7 +272,7 @@ void QSslConfiguration::setPeerVerifyMode(QSslSocket::PeerVerifyMode mode)
 */
 int QSslConfiguration::peerVerifyDepth() const
 {
-    return d ? d->peerVerifyDepth : 0;
+    return d->peerVerifyDepth;
 }
 
 /*!
@@ -307,7 +303,7 @@ void QSslConfiguration::setPeerVerifyDepth(int depth)
 */
 QSslCertificate QSslConfiguration::localCertificate() const
 {
-    return d ? d->localCertificate : QSslCertificate();
+    return d->localCertificate;
 }
 
 /*!
@@ -361,7 +357,7 @@ void QSslConfiguration::setLocalCertificate(const QSslCertificate &certificate)
 */
 QSslCertificate QSslConfiguration::peerCertificate() const
 {
-    return d ? d->peerCertificate : QSslCertificate();
+    return d->peerCertificate;
 }
 
 /*!
@@ -393,7 +389,7 @@ QSslCertificate QSslConfiguration::peerCertificate() const
 */
 QList<QSslCertificate> QSslConfiguration::peerCertificateChain() const
 {
-    return d ? d->peerCertificateChain : QList<QSslCertificate>();
+    return d->peerCertificateChain;
 }
 
 /*!
@@ -411,7 +407,7 @@ QList<QSslCertificate> QSslConfiguration::peerCertificateChain() const
 */
 QSslCipher QSslConfiguration::sessionCipher() const
 {
-    return d ? d->sessionCipher : QSslCipher();
+    return d->sessionCipher;
 }
 
 /*!
@@ -422,7 +418,7 @@ QSslCipher QSslConfiguration::sessionCipher() const
 */
 QSslKey QSslConfiguration::privateKey() const
 {
-    return d ? d->privateKey : QSslKey();
+    return d->privateKey;
 }
 
 /*!
@@ -464,7 +460,7 @@ void QSslConfiguration::setPrivateKey(const QSslKey &key)
 */
 QList<QSslCipher> QSslConfiguration::ciphers() const
 {
-    return d ? d->ciphers : QList<QSslCipher>();
+    return d->ciphers;
 }
 
 /*!
@@ -494,7 +490,7 @@ void QSslConfiguration::setCiphers(const QList<QSslCipher> &ciphers)
 */
 QList<QSslCertificate> QSslConfiguration::caCertificates() const
 {
-    return d ? d->caCertificates : QList<QSslCertificate>();
+    return d->caCertificates;
 }
 
 /*!
diff --git a/src/network/ssl/qsslconfiguration.h b/src/network/ssl/qsslconfiguration.h
index 69dd145..143566b 100644
--- a/src/network/ssl/qsslconfiguration.h
+++ b/src/network/ssl/qsslconfiguration.h
@@ -86,7 +86,7 @@ public:
     inline bool operator!=(const QSslConfiguration &other) const
     { return !(*this == other); }
 
-    bool isNull() const;
+    bool isNull() const; // ### Qt 5: remove; who would need this?
 
     QSsl::SslProtocol protocol() const;
     void setProtocol(QSsl::SslProtocol protocol);
diff --git a/src/network/ssl/qsslsocket.cpp b/src/network/ssl/qsslsocket.cpp
index 98e2dc5..a649758 100644
--- a/src/network/ssl/qsslsocket.cpp
+++ b/src/network/ssl/qsslsocket.cpp
@@ -902,6 +902,7 @@ void QSslSocket::setSslConfiguration(const QSslConfiguration &configuration)
     d->configuration.peerVerifyDepth = configuration.peerVerifyDepth();
     d->configuration.peerVerifyMode = configuration.peerVerifyMode();
     d->configuration.protocol = configuration.protocol();
+    d->allowRootCertOnDemandLoading = false;
 }
 
 /*!
diff --git a/tests/auto/qnetworkreply/tst_qnetworkreply.cpp b/tests/auto/qnetworkreply/tst_qnetworkreply.cpp
index 6ed8f16..650526e 100644
--- a/tests/auto/qnetworkreply/tst_qnetworkreply.cpp
+++ b/tests/auto/qnetworkreply/tst_qnetworkreply.cpp
@@ -87,6 +87,7 @@ Q_DECLARE_METATYPE(QNetworkReply::NetworkError)
 Q_DECLARE_METATYPE(QBuffer*)
 Q_DECLARE_METATYPE(QHttpMultiPart *)
 Q_DECLARE_METATYPE(QList<QFile*>) // for multiparts
+Q_DECLARE_METATYPE(QSslConfiguration)
 
 class QNetworkReplyPtr: public QSharedPointer<QNetworkReply>
 {
@@ -319,6 +320,8 @@ private Q_SLOTS:
     void ignoreSslErrorsList();
     void ignoreSslErrorsListWithSlot_data();
     void ignoreSslErrorsListWithSlot();
+    void sslConfiguration_data();
+    void sslConfiguration();
 #endif
 
     void getAndThenDeleteObject_data();
@@ -5370,6 +5373,37 @@ void tst_QNetworkReply::ignoreSslErrorsListWithSlot()
     QCOMPARE(reply->error(), expectedNetworkError);
 }
 
+void tst_QNetworkReply::sslConfiguration_data()
+{
+    QTest::addColumn<QSslConfiguration>("configuration");
+    QTest::addColumn<bool>("works");
+
+    QTest::newRow("empty") << QSslConfiguration() << false;
+    QSslConfiguration conf = QSslConfiguration::defaultConfiguration();
+    QTest::newRow("default") << conf << false; // does not contain test server cert
+    QList<QSslCertificate> testServerCert = QSslCertificate::fromPath(SRCDIR "/certs/qt-test-server-cacert.pem");
+    conf.setCaCertificates(testServerCert);
+    QTest::newRow("set-root-cert") << conf << true;
+    conf.setProtocol(QSsl::SecureProtocols);
+    QTest::newRow("secure") << conf << true;
+}
+
+void tst_QNetworkReply::sslConfiguration()
+{
+    QNetworkRequest request(QUrl("https://" + QtNetworkSettings::serverName() + "/index.html"));
+    QFETCH(QSslConfiguration, configuration);
+    request.setSslConfiguration(configuration);
+    QNetworkReplyPtr reply = manager.get(request);
+
+    connect(reply, SIGNAL(finished()), &QTestEventLoop::instance(), SLOT(exitLoop()));
+    QTestEventLoop::instance().enterLoop(10);
+    QVERIFY(!QTestEventLoop::instance().timeout());
+
+    QFETCH(bool, works);
+    QNetworkReply::NetworkError expectedError = works ? QNetworkReply::NoError : QNetworkReply::SslHandshakeFailedError;
+    QCOMPARE(reply->error(), expectedError);
+}
+
 #endif // QT_NO_OPENSSL
 
 void tst_QNetworkReply::getAndThenDeleteObject_data()
diff --git a/tests/auto/qsslsocket/tst_qsslsocket.cpp b/tests/auto/qsslsocket/tst_qsslsocket.cpp
index 472be69..8f62d87 100644
--- a/tests/auto/qsslsocket/tst_qsslsocket.cpp
+++ b/tests/auto/qsslsocket/tst_qsslsocket.cpp
@@ -67,6 +67,7 @@ typedef QList<QSslError::SslError> SslErrorList;
 Q_DECLARE_METATYPE(SslErrorList)
 Q_DECLARE_METATYPE(QSslError)
 Q_DECLARE_METATYPE(QSsl::SslProtocol)
+Q_DECLARE_METATYPE(QSslConfiguration)
 #endif
 
 #if defined Q_OS_HPUX && defined Q_CC_GNU
@@ -152,6 +153,8 @@ private slots:
     void setLocalCertificate();
     void setPrivateKey();
     void setSocketDescriptor();
+    void setSslConfiguration_data();
+    void setSslConfiguration();
     void waitForEncrypted();
     void waitForConnectedEncryptedReadyRead();
     void startClientEncryption();
@@ -1038,6 +1041,39 @@ void tst_QSslSocket::setSocketDescriptor()
     QVERIFY(client->localPort() != 0);
 }
 
+void tst_QSslSocket::setSslConfiguration_data()
+{
+    QTest::addColumn<QSslConfiguration>("configuration");
+    QTest::addColumn<bool>("works");
+
+    QTest::newRow("empty") << QSslConfiguration() << false;
+    QSslConfiguration conf = QSslConfiguration::defaultConfiguration();
+    QTest::newRow("default") << conf << false; // does not contain test server cert
+    QList<QSslCertificate> testServerCert = QSslCertificate::fromPath(SRCDIR "certs/qt-test-server-cacert.pem");
+    conf.setCaCertificates(testServerCert);
+    QTest::newRow("set-root-cert") << conf << true;
+    conf.setProtocol(QSsl::SecureProtocols);
+    QTest::newRow("secure") << conf << true;
+}
+
+void tst_QSslSocket::setSslConfiguration()
+{
+    if (!QSslSocket::supportsSsl())
+        return;
+
+    QSslSocketPtr socket = newSocket();
+    QFETCH(QSslConfiguration, configuration);
+    socket->setSslConfiguration(configuration);
+    this->socket = socket;
+    socket->connectToHostEncrypted(QtNetworkSettings::serverName(), 443);
+    QFETCH(bool, works);
+    QCOMPARE(socket->waitForEncrypted(10000), works);
+    if (works) {
+        socket->disconnectFromHost();
+        QVERIFY2(socket->waitForDisconnected(), qPrintable(socket->errorString()));
+    }
+}
+
 void tst_QSslSocket::waitForEncrypted()
 {
     if (!QSslSocket::supportsSsl())
@@ -1950,8 +1986,9 @@ void tst_QSslSocket::setEmptyDefaultConfiguration()
     QSslConfiguration::setDefaultConfiguration(emptyConf);
 
     QSslSocketPtr socket = newSocket();
+    connect(socket, SIGNAL(sslErrors(const QList<QSslError> &)), this, SLOT(ignoreErrorSlot()));
     socket->connectToHostEncrypted(QtNetworkSettings::serverName(), 443);
-
+    QVERIFY2(!socket->waitForEncrypted(4000), qPrintable(socket->errorString()));
 }
 
 #endif // QT_NO_OPENSSL
diff --git a/tests/auto/qsslsocket_onDemandCertificates_member/tst_qsslsocket_onDemandCertificates_member.cpp b/tests/auto/qsslsocket_onDemandCertificates_member/tst_qsslsocket_onDemandCertificates_member.cpp
index abdd550..6d3590a 100644
--- a/tests/auto/qsslsocket_onDemandCertificates_member/tst_qsslsocket_onDemandCertificates_member.cpp
+++ b/tests/auto/qsslsocket_onDemandCertificates_member/tst_qsslsocket_onDemandCertificates_member.cpp
@@ -217,6 +217,13 @@ void tst_QSslSocket_onDemandCertificates_member::onDemandRootCertLoadingMemberMe
     socket3->setCaCertificates(QList<QSslCertificate>());
     socket3->connectToHostEncrypted(host, 443);
     QVERIFY(!socket3->waitForEncrypted());
+
+    // setting empty SSL configuration explicitly -> should not work
+    QSslSocketPtr socket4 = newSocket();
+    this->socket = socket4;
+    socket4->setSslConfiguration(QSslConfiguration());
+    socket4->connectToHostEncrypted(host, 443);
+    QVERIFY(!socket4->waitForEncrypted());
 }
 
 #endif // QT_NO_OPENSSL
diff --git a/tests/auto/qsslsocket_onDemandCertificates_static/tst_qsslsocket_onDemandCertificates_static.cpp b/tests/auto/qsslsocket_onDemandCertificates_static/tst_qsslsocket_onDemandCertificates_static.cpp
index 5003b5c..14a7c3b 100644
--- a/tests/auto/qsslsocket_onDemandCertificates_static/tst_qsslsocket_onDemandCertificates_static.cpp
+++ b/tests/auto/qsslsocket_onDemandCertificates_static/tst_qsslsocket_onDemandCertificates_static.cpp
@@ -218,6 +218,18 @@ void tst_QSslSocket_onDemandCertificates_static::onDemandRootCertLoadingStaticMe
     this->socket = socket3;
     socket3->connectToHostEncrypted(host, 443);
     QVERIFY(!socket3->waitForEncrypted());
+
+    QSslSocket::setDefaultCaCertificates(QSslSocket::systemCaCertificates());
+
+    // setting empty default configuration -> should not work
+    QSslConfiguration conf;
+    QSslConfiguration originalDefaultConf = QSslConfiguration::defaultConfiguration();
+    QSslConfiguration::setDefaultConfiguration(conf);
+    QSslSocketPtr socket4 = newSocket();
+    this->socket = socket4;
+    socket4->connectToHostEncrypted(host, 443);
+    QVERIFY(!socket4->waitForEncrypted(4000));
+    QSslConfiguration::setDefaultConfiguration(originalDefaultConf); // restore old behaviour for run with proxies etc.
 }
 
 #endif // QT_NO_OPENSSL