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>
This commit is contained in:
Aron Rosenberg 2012-04-25 09:57:43 -07:00 committed by Qt by Nokia
parent 92d4e843bd
commit 6ccbfd6ca4
2 changed files with 54 additions and 37 deletions

View File

@ -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);

View File

@ -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();
};