From 2997b89e6480abfed356fbd6fe248bc6d2accbe2 Mon Sep 17 00:00:00 2001 From: mread Date: Wed, 4 Jan 2012 15:19:49 +0000 Subject: QThreads on Symbian are named to allow them to be opened externally The Qt 4.7 implementation of QThread on Symbian used libpthread. This automatically generated a random name for a thread. The Qt 4.8 implmentation was leaving threads unnamed. This is a change in behaviour, in that unnamed/anonymous threads cannot be opened outside of the owning process. This was causing a bug in some client/server situations. The fix is to generate a name for the RThread underlying the QThread as follows: QThread object name + QThread object address + random number Task-number: ou1cimx1#959586 Reviewed-by: Shane Kearns --- src/corelib/thread/qthread_symbian.cpp | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/corelib/thread/qthread_symbian.cpp b/src/corelib/thread/qthread_symbian.cpp index 2ea1b44..78bb293 100644 --- a/src/corelib/thread/qthread_symbian.cpp +++ b/src/corelib/thread/qthread_symbian.cpp @@ -46,10 +46,12 @@ #include "qthreadstorage.h" #include "qthread_p.h" #include +#include #include #include #include +#include // You only find these enumerations on Symbian^3 onwards, so we need to provide our own // to remain compatible with older releases. They won't be called by pre-Sym^3 SDKs. @@ -509,7 +511,21 @@ void QThread::start(Priority priority) // operations like file I/O fail, so we increase it by default. d->stackSize = 0x14000; // Maximum stack size on Symbian. - int code = d->data->symbian_thread_handle.Create(KNullDesC, (TThreadFunction) QThreadPrivate::start, d->stackSize, NULL, this); + int code = KErrAlreadyExists; + QString objName = objectName(); + TPtrC objNamePtr(qt_QString2TPtrC(objName)); + TName name; + objNamePtr.Set(objNamePtr.Left(qMin(objNamePtr.Length(), name.MaxLength() - 16))); + const int MaxRetries = 10; + for (int i=0; idata->symbian_thread_handle.Create(name, (TThreadFunction) QThreadPrivate::start, d->stackSize, NULL, this); + } if (code == KErrNone) { d->thread_id = d->data->symbian_thread_handle.Id(); TThreadPriority symPriority = calculateSymbianPriority(priority); -- cgit v0.12 From b6b862050039c53e5f9bb25b86047dd0335c0d81 Mon Sep 17 00:00:00 2001 From: Shane Kearns Date: Tue, 13 Dec 2011 15:50:59 +0000 Subject: Test case for QTBUG-22875 Test the authentication cache works properly with "cancelled dialogs" or if the user enters username/password incorrectly. Expected behaviour is based on web browsers: If cancelled, current request fails, and prompt again the next time. If wrong password is given, prompt again and retry the current request. If bad credentials are in the cache, prompt again Task-number: QTBUG-22875 Change-Id: Ic02ccac8dbeb3f2580ca4ffe47d0773982c4ab25 Reviewed-by: Peter Hartmann (cherry picked from 4be2430) --- tests/auto/qnetworkreply/tst_qnetworkreply.cpp | 185 +++++++++++++++++++++++++ 1 file changed, 185 insertions(+) diff --git a/tests/auto/qnetworkreply/tst_qnetworkreply.cpp b/tests/auto/qnetworkreply/tst_qnetworkreply.cpp index 371ac57..929917b 100644 --- a/tests/auto/qnetworkreply/tst_qnetworkreply.cpp +++ b/tests/auto/qnetworkreply/tst_qnetworkreply.cpp @@ -380,6 +380,8 @@ private Q_SLOTS: void httpAbort(); void dontInsertPartialContentIntoTheCache(); + void authenticationCacheAfterCancel_data(); + void authenticationCacheAfterCancel(); void synchronousAuthenticationCache(); void pipelining(); @@ -6026,6 +6028,189 @@ void tst_QNetworkReply::qtbug4121unknownAuthentication() QCOMPARE(reply->error(), QNetworkReply::AuthenticationRequiredError); } +void tst_QNetworkReply::authenticationCacheAfterCancel_data() +{ + QTest::addColumn("proxy"); + QTest::addColumn("proxyAuth"); + QTest::addColumn("url"); + for (int i = 0; i < proxies.count(); ++i) { + QTest::newRow("http" + proxies.at(i).tag) << proxies.at(i).proxy << proxies.at(i).requiresAuthentication << QUrl("http://" + QtNetworkSettings::serverName() + "/qtest/rfcs-auth/rfc3252.txt"); +#ifndef QT_NO_OPENSSL + QTest::newRow("https" + proxies.at(i).tag) << proxies.at(i).proxy << proxies.at(i).requiresAuthentication << QUrl("https://" + QtNetworkSettings::serverName() + "/qtest/rfcs-auth/rfc3252.txt"); +#endif + } +} + +class AuthenticationCacheHelper : public QObject +{ + Q_OBJECT +public: + AuthenticationCacheHelper() + {} +public slots: + void proxyAuthenticationRequired(const QNetworkProxy &, QAuthenticator *auth) + { + if (!proxyPassword.isNull()) { + auth->setUser(proxyUserName); + auth->setPassword(proxyPassword); + //clear credentials, if they are asked again, they were bad + proxyUserName.clear(); + proxyPassword.clear(); + } + } + void authenticationRequired(QNetworkReply*,QAuthenticator *auth) + { + if (!httpPassword.isNull()) { + auth->setUser(httpUserName); + auth->setPassword(httpPassword); + //clear credentials, if they are asked again, they were bad + httpUserName.clear(); + httpPassword.clear(); + } + } +public: + QString httpUserName; + QString httpPassword; + QString proxyUserName; + QString proxyPassword; +}; + +/* Purpose of this test is to check credentials are cached correctly. + - If user cancels authentication dialog (i.e. nothing is set to the QAuthenticator by the callback) then this is not cached + - if user supplies a wrong password, then this is not cached + - if user supplies a correct user/password combination then this is cached + + Test is checking both the proxyAuthenticationRequired and authenticationRequired signals. + */ +void tst_QNetworkReply::authenticationCacheAfterCancel() +{ + QFETCH(QNetworkProxy, proxy); + QFETCH(bool, proxyAuth); + QFETCH(QUrl, url); + QNetworkAccessManager manager; +#ifndef QT_NO_OPENSSL + connect(&manager, SIGNAL(sslErrors(QNetworkReply*,QList)), + SLOT(sslErrors(QNetworkReply*,QList))); +#endif + manager.setProxy(proxy); + QSignalSpy authSpy(&manager, SIGNAL(authenticationRequired(QNetworkReply*,QAuthenticator*))); + QSignalSpy proxyAuthSpy(&manager, SIGNAL(proxyAuthenticationRequired(const QNetworkProxy &, QAuthenticator *))); + + AuthenticationCacheHelper helper; + connect(&manager, SIGNAL(proxyAuthenticationRequired(const QNetworkProxy &, QAuthenticator *)), &helper, SLOT(proxyAuthenticationRequired(const QNetworkProxy &, QAuthenticator *))); + connect(&manager, SIGNAL(authenticationRequired(QNetworkReply*,QAuthenticator*)), &helper, SLOT(authenticationRequired(QNetworkReply*,QAuthenticator*))); + + QNetworkRequest request(url); + QNetworkReplyPtr reply; + if (proxyAuth) { + //should fail due to no credentials + reply = manager.get(request); + connect(reply, SIGNAL(finished()), &QTestEventLoop::instance(), SLOT(exitLoop()), Qt::QueuedConnection); + QTestEventLoop::instance().enterLoop(10); + QVERIFY(!QTestEventLoop::instance().timeout()); + + QCOMPARE(reply->error(), QNetworkReply::ProxyAuthenticationRequiredError); + QCOMPARE(authSpy.count(), 0); + QCOMPARE(proxyAuthSpy.count(), 1); + proxyAuthSpy.clear(); + + //should fail due to bad credentials + helper.proxyUserName = "qsockstest"; + helper.proxyPassword = "badpassword"; + reply = manager.get(request); + connect(reply, SIGNAL(finished()), &QTestEventLoop::instance(), SLOT(exitLoop()), Qt::QueuedConnection); + QTestEventLoop::instance().enterLoop(10); + QVERIFY(!QTestEventLoop::instance().timeout()); + + QEXPECT_FAIL("http+socksauth", "QTBUG-23136 - danted accepts bad authentication but blocks the connection", Continue); + QEXPECT_FAIL("https+socksauth", "QTBUG-23136 - danted accepts bad authentication but blocks the connection", Continue); + + QCOMPARE(reply->error(), QNetworkReply::ProxyAuthenticationRequiredError); + QCOMPARE(authSpy.count(), 0); + QVERIFY(proxyAuthSpy.count() > 0); + proxyAuthSpy.clear(); + + //QTBUG-23136 workaround + if (proxy.port() == 1081) { +#ifdef QT_BUILD_INTERNAL + QNetworkAccessManagerPrivate::clearCache(&manager); +#else + return; //XFAIL result above +#endif + } + + //next proxy auth should succeed, due to correct credentials + helper.proxyUserName = "qsockstest"; + helper.proxyPassword = "password"; + } + + //should fail due to no credentials + reply = manager.get(request); + connect(reply, SIGNAL(finished()), &QTestEventLoop::instance(), SLOT(exitLoop()), Qt::QueuedConnection); + QTestEventLoop::instance().enterLoop(10); + QVERIFY(!QTestEventLoop::instance().timeout()); + + QCOMPARE(reply->error(), QNetworkReply::AuthenticationRequiredError); + QVERIFY(authSpy.count() > 0); + authSpy.clear(); + if (proxyAuth) { + QVERIFY(proxyAuthSpy.count() > 0); + proxyAuthSpy.clear(); + } + + //should fail due to bad credentials + helper.httpUserName = "baduser"; + helper.httpPassword = "badpassword"; + reply = manager.get(request); + connect(reply, SIGNAL(finished()), &QTestEventLoop::instance(), SLOT(exitLoop()), Qt::QueuedConnection); + QTestEventLoop::instance().enterLoop(10); + QVERIFY(!QTestEventLoop::instance().timeout()); + + QCOMPARE(reply->error(), QNetworkReply::AuthenticationRequiredError); + QVERIFY(authSpy.count() > 0); + authSpy.clear(); + if (proxyAuth) { + //should be supplied from cache + QCOMPARE(proxyAuthSpy.count(), 0); + proxyAuthSpy.clear(); + } + + //next auth should succeed, due to correct credentials + helper.httpUserName = "httptest"; + helper.httpPassword = "httptest"; + + reply = manager.get(request); + connect(reply, SIGNAL(finished()), &QTestEventLoop::instance(), SLOT(exitLoop()), Qt::QueuedConnection); + QTestEventLoop::instance().enterLoop(10); + QVERIFY(!QTestEventLoop::instance().timeout()); + + QCOMPARE(reply->error(), QNetworkReply::NoError); + QVERIFY(authSpy.count() > 0); + authSpy.clear(); + if (proxyAuth) { + //should be supplied from cache + QCOMPARE(proxyAuthSpy.count(), 0); + proxyAuthSpy.clear(); + } + + //next auth should use cached credentials + reply = manager.get(request); + connect(reply, SIGNAL(finished()), &QTestEventLoop::instance(), SLOT(exitLoop()), Qt::QueuedConnection); + QTestEventLoop::instance().enterLoop(10); + QVERIFY(!QTestEventLoop::instance().timeout()); + + QCOMPARE(reply->error(), QNetworkReply::NoError); + //should be supplied from cache + QCOMPARE(authSpy.count(), 0); + authSpy.clear(); + if (proxyAuth) { + //should be supplied from cache + QCOMPARE(proxyAuthSpy.count(), 0); + proxyAuthSpy.clear(); + } + +} + class QtBug13431Helper : public QObject { Q_OBJECT public: -- cgit v0.12 From 5da61168e9f2eb3db9e4f6398eb784ab3ce21a50 Mon Sep 17 00:00:00 2001 From: Shane Kearns Date: Thu, 22 Dec 2011 13:35:34 +0000 Subject: Fix faulty logic in http connection pipelining The code which prevents pipelining of requests when authentication is in use had || where && should have been used. Also check for blank user with a password. Change-Id: Ic278cedd370c9d81377f49a0af43aef415cb49ad Reviewed-by: Peter Hartmann (cherry picked from commit 058fb94afff8a1a9989ab6d18dacc1fe43769fdb) --- src/network/access/qhttpnetworkconnection.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/network/access/qhttpnetworkconnection.cpp b/src/network/access/qhttpnetworkconnection.cpp index 0365703..5ef4df0 100644 --- a/src/network/access/qhttpnetworkconnection.cpp +++ b/src/network/access/qhttpnetworkconnection.cpp @@ -582,9 +582,13 @@ void QHttpNetworkConnectionPrivate::fillPipeline(QAbstractSocket *socket) // we do not like authentication stuff // ### make sure to be OK with this in later releases - if (!channels[i].authenticator.isNull() || !channels[i].authenticator.user().isEmpty()) + if (!channels[i].authenticator.isNull() + && (!channels[i].authenticator.user().isEmpty() + || !channels[i].authenticator.password().isEmpty())) return; - if (!channels[i].proxyAuthenticator.isNull() || !channels[i].proxyAuthenticator.user().isEmpty()) + if (!channels[i].proxyAuthenticator.isNull() + && (!channels[i].proxyAuthenticator.user().isEmpty() + || !channels[i].proxyAuthenticator.password().isEmpty())) return; // must be in ReadingState or WaitingState -- cgit v0.12 From ddb295b7b5c68c0c34cc3c2ec26e0f8e3cc371c9 Mon Sep 17 00:00:00 2001 From: Shane Kearns Date: Thu, 15 Dec 2011 17:31:52 +0000 Subject: Fix for assertion failure Change-Id: I97b9ecc37e938a3050793fc746288243a1cb40b7 Reviewed-by: Peter Hartmann (cherry picked from commit 96cda705dcbeb79429055c1acca91f149d318820) --- src/network/access/qnetworkaccessauthenticationmanager.cpp | 5 +++++ src/network/access/qnetworkaccessauthenticationmanager_p.h | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/network/access/qnetworkaccessauthenticationmanager.cpp b/src/network/access/qnetworkaccessauthenticationmanager.cpp index b618ccc..9551b76 100644 --- a/src/network/access/qnetworkaccessauthenticationmanager.cpp +++ b/src/network/access/qnetworkaccessauthenticationmanager.cpp @@ -159,6 +159,11 @@ void QNetworkAccessAuthenticationManager::cacheProxyCredentials(const QNetworkPr QString realm = authenticator->realm(); QNetworkProxy proxy = p; proxy.setUser(authenticator->user()); + + // don't cache null passwords, empty password may be valid though + if (authenticator->password().isNull()) + return; + // Set two credentials: one with the username and one without do { // Set two credentials actually: one with and one without the realm diff --git a/src/network/access/qnetworkaccessauthenticationmanager_p.h b/src/network/access/qnetworkaccessauthenticationmanager_p.h index ddfc116..718c58f 100644 --- a/src/network/access/qnetworkaccessauthenticationmanager_p.h +++ b/src/network/access/qnetworkaccessauthenticationmanager_p.h @@ -73,7 +73,7 @@ public: QString user; QString password; bool isNull() { - return domain.isNull(); + return domain.isNull() && user.isNull() && password.isNull(); } }; Q_DECLARE_TYPEINFO(QNetworkAuthenticationCredential, Q_MOVABLE_TYPE); -- cgit v0.12 From 26f1ca8681db995e82c0f4c0fa9363c842520700 Mon Sep 17 00:00:00 2001 From: Shane Kearns Date: Mon, 19 Dec 2011 15:10:45 +0000 Subject: Handle plain socket write errors in SSL When an ssl socket is closed during connecting, and it is using a proxy then it is possible for the plain socket to be in pending close state when transmit() is called. As errors were not handled, this caused the socket (and https request) to "hang". It now propagates the error from plain socket. Change-Id: I6fb86815a2a63e197cea582f4b153e487543477c Reviewed-by: Peter Hartmann Reviewed-by: Richard J. Moore Reviewed-by: Thiago Macieira (cherry picked from commit 2cc78885b0b7d08f965998d156945a077e56c1d8) --- src/network/ssl/qsslsocket_openssl.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/network/ssl/qsslsocket_openssl.cpp b/src/network/ssl/qsslsocket_openssl.cpp index 872b19c..900bfdb 100644 --- a/src/network/ssl/qsslsocket_openssl.cpp +++ b/src/network/ssl/qsslsocket_openssl.cpp @@ -1044,10 +1044,17 @@ void QSslSocketBackendPrivate::transmit() int encryptedBytesRead = q_BIO_read(writeBio, data.data(), pendingBytes); // Write encrypted data from the buffer to the socket. - plainSocket->write(data.constData(), encryptedBytesRead); + qint64 actualWritten = plainSocket->write(data.constData(), encryptedBytesRead); #ifdef QSSLSOCKET_DEBUG - qDebug() << "QSslSocketBackendPrivate::transmit: wrote" << encryptedBytesRead << "encrypted bytes to the socket"; + qDebug() << "QSslSocketBackendPrivate::transmit: wrote" << encryptedBytesRead << "encrypted bytes to the socket" << actualWritten << "actual."; #endif + if (actualWritten < 0) { + //plain socket write fails if it was in the pending close state. + q->setErrorString(plainSocket->errorString()); + q->setSocketError(plainSocket->error()); + emit q->error(plainSocket->error()); + return; + } transmitting = true; } -- cgit v0.12 From 682555cb0104f90117f0e492999c41362ab7aad9 Mon Sep 17 00:00:00 2001 From: Shane Kearns Date: Thu, 15 Dec 2011 17:32:47 +0000 Subject: Don't fetch credentials from cache following a failed proxy authentication Add variable to QAuthenticatorPrivate for tracking failure Track authentication success/failure in http proxy socket engine Track authentication success/failure in http connection channel Task-number: QTBUG-22875 Change-Id: Id5d39e839428271ad687e9da12fbbdea9c478f4f Reviewed-by: Peter Hartmann (cherry-picked from d24aad82896addce88f1ffb4040560e406acf083) --- src/network/access/qhttpnetworkconnection.cpp | 16 ++++++++++++++++ src/network/access/qhttpnetworkconnectionchannel.cpp | 12 +++++++++++- src/network/access/qhttpnetworkconnectionchannel_p.h | 2 ++ src/network/access/qnetworkaccessmanager.cpp | 12 +++--------- src/network/kernel/qauthenticator.cpp | 1 + src/network/kernel/qauthenticator_p.h | 1 + src/network/socket/qhttpsocketengine.cpp | 20 +++++++++++++++++--- src/network/socket/qhttpsocketengine_p.h | 1 + 8 files changed, 52 insertions(+), 13 deletions(-) diff --git a/src/network/access/qhttpnetworkconnection.cpp b/src/network/access/qhttpnetworkconnection.cpp index 5ef4df0..a79b10f 100644 --- a/src/network/access/qhttpnetworkconnection.cpp +++ b/src/network/access/qhttpnetworkconnection.cpp @@ -375,9 +375,23 @@ bool QHttpNetworkConnectionPrivate::handleAuthenticateChallenge(QAbstractSocket if (priv->phase == QAuthenticatorPrivate::Done) { pauseConnection(); if (!isProxy) { + if (channels[i].authenticationCredentialsSent) { + auth->detach(); + priv = QAuthenticatorPrivate::getPrivate(*auth); + priv->hasFailed = true; + priv->phase = QAuthenticatorPrivate::Done; + channels[i].authenticationCredentialsSent = false; + } emit reply->authenticationRequired(reply->request(), auth); #ifndef QT_NO_NETWORKPROXY } else { + if (channels[i].proxyCredentialsSent) { + auth->detach(); + priv = QAuthenticatorPrivate::getPrivate(*auth); + priv->hasFailed = true; + priv->phase = QAuthenticatorPrivate::Done; + channels[i].proxyCredentialsSent = false; + } emit reply->proxyAuthenticationRequired(networkProxy, auth); #endif } @@ -438,6 +452,7 @@ void QHttpNetworkConnectionPrivate::createAuthorization(QAbstractSocket *socket, if (priv && priv->method != QAuthenticatorPrivate::None) { QByteArray response = priv->calculateResponse(request.d->methodName(), request.d->uri(false)); request.setHeaderField("Authorization", response); + channels[i].authenticationCredentialsSent = true; } } } @@ -449,6 +464,7 @@ void QHttpNetworkConnectionPrivate::createAuthorization(QAbstractSocket *socket, if (priv && priv->method != QAuthenticatorPrivate::None) { QByteArray response = priv->calculateResponse(request.d->methodName(), request.d->uri(false)); request.setHeaderField("Proxy-Authorization", response); + channels[i].proxyCredentialsSent = true; } } } diff --git a/src/network/access/qhttpnetworkconnectionchannel.cpp b/src/network/access/qhttpnetworkconnectionchannel.cpp index b9db7fe..e4f0f12 100644 --- a/src/network/access/qhttpnetworkconnectionchannel.cpp +++ b/src/network/access/qhttpnetworkconnectionchannel.cpp @@ -73,6 +73,8 @@ QHttpNetworkConnectionChannel::QHttpNetworkConnectionChannel() , lastStatus(0) , pendingEncrypt(false) , reconnectAttempts(2) + , authenticationCredentialsSent(false) + , proxyCredentialsSent(false) , authMethod(QAuthenticatorPrivate::None) , proxyAuthMethod(QAuthenticatorPrivate::None) #ifndef QT_NO_OPENSSL @@ -555,6 +557,14 @@ bool QHttpNetworkConnectionChannel::ensureConnection() // reset state pipeliningSupported = PipeliningSupportUnknown; + authenticationCredentialsSent = false; + proxyCredentialsSent = false; + authenticator.detach(); + QAuthenticatorPrivate *priv = QAuthenticatorPrivate::getPrivate(authenticator); + priv->hasFailed = false; + proxyAuthenticator.detach(); + priv = QAuthenticatorPrivate::getPrivate(proxyAuthenticator); + priv->hasFailed = false; // This workaround is needed since we use QAuthenticator for NTLM authentication. The "phase == Done" // is the usual criteria for emitting authentication signals. The "phase" is set to "Done" when the @@ -562,7 +572,7 @@ bool QHttpNetworkConnectionChannel::ensureConnection() // check the "phase" for generating the Authorization header. NTLM authentication is a two stage // process & needs the "phase". To make sure the QAuthenticator uses the current username/password // the phase is reset to Start. - QAuthenticatorPrivate *priv = QAuthenticatorPrivate::getPrivate(authenticator); + priv = QAuthenticatorPrivate::getPrivate(authenticator); if (priv && priv->phase == QAuthenticatorPrivate::Done) priv->phase = QAuthenticatorPrivate::Start; priv = QAuthenticatorPrivate::getPrivate(proxyAuthenticator); diff --git a/src/network/access/qhttpnetworkconnectionchannel_p.h b/src/network/access/qhttpnetworkconnectionchannel_p.h index f635cc9..31a286b 100644 --- a/src/network/access/qhttpnetworkconnectionchannel_p.h +++ b/src/network/access/qhttpnetworkconnectionchannel_p.h @@ -113,6 +113,8 @@ public: QAuthenticatorPrivate::Method proxyAuthMethod; QAuthenticator authenticator; QAuthenticator proxyAuthenticator; + bool authenticationCredentialsSent; + bool proxyCredentialsSent; #ifndef QT_NO_OPENSSL bool ignoreAllSslErrors; QList ignoreSslErrorsList; diff --git a/src/network/access/qnetworkaccessmanager.cpp b/src/network/access/qnetworkaccessmanager.cpp index 8fc8eb7..689441e 100644 --- a/src/network/access/qnetworkaccessmanager.cpp +++ b/src/network/access/qnetworkaccessmanager.cpp @@ -61,7 +61,7 @@ #include "QtCore/qbuffer.h" #include "QtCore/qurl.h" #include "QtCore/qvector.h" -#include "QtNetwork/qauthenticator.h" +#include "QtNetwork/private/qauthenticator_p.h" #include "QtNetwork/qsslconfiguration.h" #include "QtNetwork/qnetworkconfigmanager.h" #include "QtNetwork/qhttpmultipart.h" @@ -1093,14 +1093,8 @@ void QNetworkAccessManagerPrivate::proxyAuthenticationRequired(QNetworkAccessBac QAuthenticator *authenticator) { Q_Q(QNetworkAccessManager); - // ### FIXME Tracking of successful authentications - // This code is a bit broken right now for SOCKS authentication - // first request: proxyAuthenticationRequired gets emitted, credentials gets saved - // second request: (proxy != backend->reply->lastProxyAuthentication) does not evaluate to true, - // proxyAuthenticationRequired gets emitted again - // possible solution: some tracking inside the authenticator - // or a new function proxyAuthenticationSucceeded(true|false) - if (proxy != backend->reply->lastProxyAuthentication) { + QAuthenticatorPrivate *priv = QAuthenticatorPrivate::getPrivate(*authenticator); + if (proxy != backend->reply->lastProxyAuthentication && (!priv || !priv->hasFailed)) { QNetworkAuthenticationCredential cred = authenticationManager->fetchCachedProxyCredentials(proxy); if (!cred.isNull()) { authenticator->setUser(cred.user); diff --git a/src/network/kernel/qauthenticator.cpp b/src/network/kernel/qauthenticator.cpp index 7b63567..ac8b4b9 100644 --- a/src/network/kernel/qauthenticator.cpp +++ b/src/network/kernel/qauthenticator.cpp @@ -326,6 +326,7 @@ bool QAuthenticator::isNull() const QAuthenticatorPrivate::QAuthenticatorPrivate() : ref(0) , method(None) + , hasFailed(false) , phase(Start) , nonceCount(0) { diff --git a/src/network/kernel/qauthenticator_p.h b/src/network/kernel/qauthenticator_p.h index 88a3051..937f4e0 100644 --- a/src/network/kernel/qauthenticator_p.h +++ b/src/network/kernel/qauthenticator_p.h @@ -77,6 +77,7 @@ public: Method method; QString realm; QByteArray challenge; + bool hasFailed; //credentials have been tried but rejected by server. enum Phase { Start, diff --git a/src/network/socket/qhttpsocketengine.cpp b/src/network/socket/qhttpsocketengine.cpp index b62bc05..c582c57 100644 --- a/src/network/socket/qhttpsocketengine.cpp +++ b/src/network/socket/qhttpsocketengine.cpp @@ -136,6 +136,8 @@ bool QHttpSocketEngine::connectInternal() { Q_D(QHttpSocketEngine); + d->credentialsSent = false; + // If the handshake is done, enter ConnectedState state and return true. if (d->state == Connected) { qWarning("QHttpSocketEngine::connectToHost: called when already connected"); @@ -514,6 +516,7 @@ void QHttpSocketEngine::slotSocketConnected() QAuthenticatorPrivate *priv = QAuthenticatorPrivate::getPrivate(d->authenticator); //qDebug() << "slotSocketConnected: priv=" << priv << (priv ? (int)priv->method : -1); if (priv && priv->method != QAuthenticatorPrivate::None) { + d->credentialsSent = true; data += "Proxy-Authorization: " + priv->calculateResponse(method, path); data += "\r\n"; } @@ -591,15 +594,26 @@ void QHttpSocketEngine::slotSocketReadNotification() d->readBuffer.clear(); // we parsed the proxy protocol response. from now on direct socket reading will be done int statusCode = responseHeader.statusCode(); + QAuthenticatorPrivate *priv = 0; if (statusCode == 200) { d->state = Connected; setLocalAddress(d->socket->localAddress()); setLocalPort(d->socket->localPort()); setState(QAbstractSocket::ConnectedState); + d->authenticator.detach(); + priv = QAuthenticatorPrivate::getPrivate(d->authenticator); + priv->hasFailed = false; } else if (statusCode == 407) { - if (d->authenticator.isNull()) + if (d->credentialsSent) { + //407 response again means the provided username/password were invalid. + d->authenticator = QAuthenticator(); //this is needed otherwise parseHttpResponse won't set the state, and then signal isn't emitted. + d->authenticator.detach(); + priv = QAuthenticatorPrivate::getPrivate(d->authenticator); + priv->hasFailed = true; + } + else if (d->authenticator.isNull()) d->authenticator.detach(); - QAuthenticatorPrivate *priv = QAuthenticatorPrivate::getPrivate(d->authenticator); + priv = QAuthenticatorPrivate::getPrivate(d->authenticator); priv->parseHttpResponse(responseHeader, true); @@ -639,7 +653,6 @@ void QHttpSocketEngine::slotSocketReadNotification() if (priv->phase == QAuthenticatorPrivate::Done) emit proxyAuthenticationRequired(d->proxy, &d->authenticator); - // priv->phase will get reset to QAuthenticatorPrivate::Start if the authenticator got modified in the signal above. if (priv->phase == QAuthenticatorPrivate::Done) { setError(QAbstractSocket::ProxyAuthenticationRequiredError, tr("Authentication required")); @@ -796,6 +809,7 @@ QHttpSocketEnginePrivate::QHttpSocketEnginePrivate() , readNotificationPending(false) , writeNotificationPending(false) , connectionNotificationPending(false) + , credentialsSent(false) , pendingResponseData(0) { socket = 0; diff --git a/src/network/socket/qhttpsocketengine_p.h b/src/network/socket/qhttpsocketengine_p.h index d7cc7c1..476d689 100644 --- a/src/network/socket/qhttpsocketengine_p.h +++ b/src/network/socket/qhttpsocketengine_p.h @@ -182,6 +182,7 @@ public: bool readNotificationPending; bool writeNotificationPending; bool connectionNotificationPending; + bool credentialsSent; uint pendingResponseData; }; -- cgit v0.12 From ef91776735216f23b3d708032b81435d045e8e56 Mon Sep 17 00:00:00 2001 From: Shane Kearns Date: Mon, 19 Dec 2011 15:35:52 +0000 Subject: Fix race in http connection channel When authentication is cancelled, close the channel instead of the underlying socket. The previous behaviour could result in further requests being sent on the closed socket, which caused errors in case of https over a proxy. Change-Id: I3dbfc164de4fb29a426c06acaac8f29b9da1d705 Reviewed-by: Peter Hartmann (cherry picked from commit a7b99151f4445755c91d5227607d9ea2f785301f) --- src/network/access/qhttpnetworkconnection.cpp | 1 - src/network/access/qhttpnetworkconnectionchannel.cpp | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/network/access/qhttpnetworkconnection.cpp b/src/network/access/qhttpnetworkconnection.cpp index a79b10f..33e3be9 100644 --- a/src/network/access/qhttpnetworkconnection.cpp +++ b/src/network/access/qhttpnetworkconnection.cpp @@ -429,7 +429,6 @@ bool QHttpNetworkConnectionPrivate::handleAuthenticateChallenge(QAbstractSocket reply->d_func()->errorString = errorDetail(errorCode, socket); emit reply->finishedWithError(errorCode, reply->d_func()->errorString); // ### at this point the reply could be deleted - socket->close(); return true; } //resend the request diff --git a/src/network/access/qhttpnetworkconnectionchannel.cpp b/src/network/access/qhttpnetworkconnectionchannel.cpp index e4f0f12..628114a 100644 --- a/src/network/access/qhttpnetworkconnectionchannel.cpp +++ b/src/network/access/qhttpnetworkconnectionchannel.cpp @@ -847,6 +847,9 @@ void QHttpNetworkConnectionChannel::handleStatus() closeAndResendCurrentRequest(); QMetaObject::invokeMethod(connection, "_q_startNextRequest", Qt::QueuedConnection); } + } else { + //authentication cancelled, close the channel. + close(); } } else { emit reply->headerChanged(); -- cgit v0.12 From c5ddcb3387f968ce0c2ac6420a05c5ebc528b260 Mon Sep 17 00:00:00 2001 From: Shane Kearns Date: Thu, 22 Dec 2011 14:08:17 +0000 Subject: Fix http authentication to a different realm on the same server This is a regression caused by the NTLMv2 authentication patch. I have manually tested NTLMv2 authentication against MS IIS and reverting these two lines does not break it. Task-number: QT-5209 Change-Id: I64159cbe468e1a7f834f8726fd0c9d4ab4c54b38 Reviewed-by: Peter Hartmann (cherry-picked from 4954f71648aa7f74a4cb8b1dd26470b5da44459e) --- src/network/kernel/qauthenticator.cpp | 6 ++--- tests/auto/qnetworkreply/tst_qnetworkreply.cpp | 33 ++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/src/network/kernel/qauthenticator.cpp b/src/network/kernel/qauthenticator.cpp index ac8b4b9..d0524ee 100644 --- a/src/network/kernel/qauthenticator.cpp +++ b/src/network/kernel/qauthenticator.cpp @@ -388,8 +388,7 @@ void QAuthenticatorPrivate::parseHttpResponse(const QListoptions[QLatin1String("realm")] = realm = QString::fromLatin1(options.value("realm")); + this->options[QLatin1String("realm")] = realm = QString::fromLatin1(options.value("realm")); if (user.isEmpty() && password.isEmpty()) phase = Done; break; @@ -397,8 +396,7 @@ void QAuthenticatorPrivate::parseHttpResponse(const QListoptions[QLatin1String("realm")] = realm = QString::fromLatin1(options.value("realm")); + this->options[QLatin1String("realm")] = realm = QString::fromLatin1(options.value("realm")); if (options.value("stale").toLower() == "true") phase = Start; if (user.isEmpty() && password.isEmpty()) diff --git a/tests/auto/qnetworkreply/tst_qnetworkreply.cpp b/tests/auto/qnetworkreply/tst_qnetworkreply.cpp index 929917b..6760b73 100644 --- a/tests/auto/qnetworkreply/tst_qnetworkreply.cpp +++ b/tests/auto/qnetworkreply/tst_qnetworkreply.cpp @@ -382,6 +382,7 @@ private Q_SLOTS: void dontInsertPartialContentIntoTheCache(); void authenticationCacheAfterCancel_data(); void authenticationCacheAfterCancel(); + void authenticationWithDifferentRealm(); void synchronousAuthenticationCache(); void pipelining(); @@ -6211,6 +6212,38 @@ void tst_QNetworkReply::authenticationCacheAfterCancel() } +void tst_QNetworkReply::authenticationWithDifferentRealm() +{ + AuthenticationCacheHelper helper; + QNetworkAccessManager manager; +#ifndef QT_NO_OPENSSL + connect(&manager, SIGNAL(sslErrors(QNetworkReply*,QList)), + SLOT(sslErrors(QNetworkReply*,QList))); +#endif + connect(&manager, SIGNAL(proxyAuthenticationRequired(const QNetworkProxy &, QAuthenticator *)), &helper, SLOT(proxyAuthenticationRequired(const QNetworkProxy &, QAuthenticator *))); + connect(&manager, SIGNAL(authenticationRequired(QNetworkReply*,QAuthenticator*)), &helper, SLOT(authenticationRequired(QNetworkReply*,QAuthenticator*))); + + helper.httpUserName = "httptest"; + helper.httpPassword = "httptest"; + + QNetworkRequest request(QUrl("http://" + QtNetworkSettings::serverName() + "/qtest/rfcs-auth/rfc3252.txt")); + QNetworkReply* reply = manager.get(request); + connect(reply, SIGNAL(finished()), &QTestEventLoop::instance(), SLOT(exitLoop()), Qt::QueuedConnection); + QTestEventLoop::instance().enterLoop(10); + QVERIFY(!QTestEventLoop::instance().timeout()); + QCOMPARE(reply->error(), QNetworkReply::NoError); + + helper.httpUserName = "httptest"; + helper.httpPassword = "httptest"; + + request.setUrl(QUrl("http://" + QtNetworkSettings::serverName() + "/qtest/auth-digest/")); + reply = manager.get(request); + connect(reply, SIGNAL(finished()), &QTestEventLoop::instance(), SLOT(exitLoop()), Qt::QueuedConnection); + QTestEventLoop::instance().enterLoop(10); + QVERIFY(!QTestEventLoop::instance().timeout()); + QCOMPARE(reply->error(), QNetworkReply::NoError); +} + class QtBug13431Helper : public QObject { Q_OBJECT public: -- cgit v0.12