Fix various NTLM/Digest multi-threading and usage issues

- Fix multi-threading bug where NTLM/Digest auth would fail when
concurrent requests were on the wire. The fix for this is too not
internally share QAuthenticationPrivate pointers, since the challange
values would get overridden in different threads. This was failing
because the internal QAuthenticationPrivate members would have been
set with the status/values of the current request which would mess
up the state of the new request. As currently implemented, the helper
functions inside QAuthenticationPrivate can't call detach to implement
proper copy on write symantics.
- Fix issue where if user was set via constructor, the NTLM domain
parsing would not occur. Parsing of DOMAIN\user is now redone if
proxy type is determined to be NTLM.

Task-number: QTBUG-15472
Task-number: QTBUG-17322
Task-number: QTBUG-18794
Task-number: QTBUG-13063
Task-number: QTBUG-16585
Change-Id: I8a898c51fb04fab6fb08d96d88dd73be0c87af5d
Reviewed-by: Aron Rosenberg <aronrosenberg@gmail.com>
Reviewed-by: Shane Kearns <shane.kearns@accenture.com>

2 files changed, 54 insertions, 37 deletions

diff --git a/src/network/kernel/qauthenticator.cpp b/src/network/kernel/qauthenticator.cpp
index 43b3618ea2..6808ae51ca 100644
--- a/src/network/kernel/qauthenticator.cpp
+++ b/src/network/kernel/qauthenticator.cpp
@@ -140,7 +140,7 @@ QAuthenticator::QAuthenticator()
 */
 QAuthenticator::~QAuthenticator()
 {
-    if (d && !d->ref.deref())
+    if (d)
         delete d;
 }
 
@@ -148,10 +148,10 @@ QAuthenticator::~QAuthenticator()
     Constructs a copy of \a other.
 */
 QAuthenticator::QAuthenticator(const QAuthenticator &other)
-    : d(other.d)
+    : d(0)
 {
-    if (d)
-        d->ref.ref();
+    if (other.d)
+        *this = other;
 }
 
 /*!
@@ -162,12 +162,23 @@ QAuthenticator &QAuthenticator::operator=(const QAuthenticator &other)
     if (d == other.d)
         return *this;
 
-    if (d && !d->ref.deref())
+    // Do not share the d since challange reponse/based changes
+    // could corrupt the internal store and different network requests
+    // can utilize different types of proxies.
+    detach();
+    if (other.d) {
+        d->user = other.d->user;
+        d->userDomain = other.d->userDomain;
+        d->workstation = other.d->workstation;
+        d->extractedUser = other.d->extractedUser;
+        d->password = other.d->password;
+        d->realm = other.d->realm;
+        d->method = other.d->method;
+        d->options = other.d->options;
+    } else {
         delete d;
-
-    d = other.d;
-    if (d)
-        d->ref.ref();
+        d = 0;
+    }
     return *this;
 }
 
@@ -209,28 +220,8 @@ QString QAuthenticator::user() const
 void QAuthenticator::setUser(const QString &user)
 {
     detach();
-    int separatorPosn = 0;
-
-    switch(d->method) {
-    case QAuthenticatorPrivate::Ntlm:
-        if((separatorPosn = user.indexOf(QLatin1String("\\"))) != -1) {
-            //domain name is present
-            d->realm.clear();
-            d->userDomain = user.left(separatorPosn);
-            d->extractedUser = user.mid(separatorPosn + 1);
-            d->user = user;
-        } else {
-            d->extractedUser = user;
-            d->user = user;
-	    d->realm.clear();
-            d->userDomain.clear();
-        }
-        break;
-    default:
-        d->user = user;
-        d->userDomain.clear();
-        break;
-    }
+    d->user = user;
+    d->updateCredentials();
 }
 
 /*!
@@ -259,11 +250,9 @@ void QAuthenticator::detach()
 {
     if (!d) {
         d = new QAuthenticatorPrivate;
-        d->ref.store(1);
         return;
     }
 
-    qAtomicDetach(d);
     d->phase = QAuthenticatorPrivate::Start;
 }
 
@@ -325,8 +314,7 @@ bool QAuthenticator::isNull() const
 }
 
 QAuthenticatorPrivate::QAuthenticatorPrivate()
-    : ref(0)
-    , method(None)
+    : method(None)
     , hasFailed(false)
     , phase(Start)
     , nonceCount(0)
@@ -336,6 +324,33 @@ QAuthenticatorPrivate::QAuthenticatorPrivate()
     nonceCount = 0;
 }
 
+QAuthenticatorPrivate::~QAuthenticatorPrivate()
+{
+}
+
+void QAuthenticatorPrivate::updateCredentials()
+{
+    int separatorPosn = 0;
+
+    switch (method) {
+    case QAuthenticatorPrivate::Ntlm:
+        if ((separatorPosn = user.indexOf(QLatin1String("\\"))) != -1) {
+            //domain name is present
+            realm.clear();
+            userDomain = user.left(separatorPosn);
+            extractedUser = user.mid(separatorPosn + 1);
+        } else {
+            extractedUser = user;
+            realm.clear();
+            userDomain.clear();
+        }
+        break;
+    default:
+        userDomain.clear();
+        break;
+    }
+}
+
 void QAuthenticatorPrivate::parseHttpResponse(const QList<QPair<QByteArray, QByteArray> > &values, bool isProxy)
 {
     const char *search = isProxy ? "proxy-authenticate" : "www-authenticate";
@@ -369,6 +384,8 @@ void QAuthenticatorPrivate::parseHttpResponse(const QList<QPair<QByteArray, QByt
         }
     }
 
+    // Reparse credentials since we know the method now
+    updateCredentials();
     challenge = headerVal.trimmed();
     QHash<QByteArray, QByteArray> options = parseDigestAuthenticationChallenge(challenge);
 
diff --git a/src/network/kernel/qauthenticator_p.h b/src/network/kernel/qauthenticator_p.h
index a41b31d333..b96c8c13e1 100644
--- a/src/network/kernel/qauthenticator_p.h
+++ b/src/network/kernel/qauthenticator_p.h
@@ -68,8 +68,8 @@ class Q_AUTOTEST_EXPORT QAuthenticatorPrivate
 public:
     enum Method { None, Basic, Plain, Login, Ntlm, CramMd5, DigestMd5 };
     QAuthenticatorPrivate();
+    ~QAuthenticatorPrivate();
 
-    QAtomicInt ref;
     QString user;
     QString extractedUser;
     QString password;
@@ -104,7 +104,7 @@ public:
     static QHash<QByteArray, QByteArray> parseDigestAuthenticationChallenge(const QByteArray &challenge);
 
     void parseHttpResponse(const QList<QPair<QByteArray, QByteArray> >&, bool isProxy);
-
+    void updateCredentials();
 };