From c93f7b69486a0d12b475e31eeb699ae07aa928c2 Mon Sep 17 00:00:00 2001 From: Shane Kearns Date: Mon, 5 Mar 2012 16:54:12 +0000 Subject: [PATCH] Fix tst_QNetworkReply::httpWithNoCredentialUsage autotest The test was testing the wrong thing, and passing even though QNetworkRequest::AuthenticationReuseAttribute was not being respected, until recently when I fixed username/password in URLs Now the cache is properly bypassed when this attribute is set to manual, and the autotest is updated to check this. Change-Id: I87943515562d0b16b03504f0758ba265758d1c22 Reviewed-by: Martin Petersson --- src/network/access/qnetworkaccessmanager.cpp | 10 ++-- src/network/access/qnetworkaccessmanager_p.h | 3 +- src/network/access/qnetworkreplyhttpimpl.cpp | 4 +- .../qnetworkreply/tst_qnetworkreply.cpp | 55 +++++++++++++------ 4 files changed, 49 insertions(+), 23 deletions(-) diff --git a/src/network/access/qnetworkaccessmanager.cpp b/src/network/access/qnetworkaccessmanager.cpp index 60c28274c61..c65edb66730 100644 --- a/src/network/access/qnetworkaccessmanager.cpp +++ b/src/network/access/qnetworkaccessmanager.cpp @@ -1089,15 +1089,16 @@ void QNetworkAccessManagerPrivate::authenticationRequired(QAuthenticator *authen QNetworkReply *reply, bool synchronous, QUrl &url, - QUrl *urlForLastAuthentication) + QUrl *urlForLastAuthentication, + bool allowAuthenticationReuse) { Q_Q(QNetworkAccessManager); // don't try the cache for the same URL twice in a row // being called twice for the same URL means the authentication failed // also called when last URL is empty, e.g. on first call - if (urlForLastAuthentication->isEmpty() - || url != *urlForLastAuthentication) { + if (allowAuthenticationReuse && (urlForLastAuthentication->isEmpty() + || url != *urlForLastAuthentication)) { // if credentials are included in the url, then use them if (!url.userName().isEmpty() && !url.password().isEmpty()) { @@ -1124,7 +1125,8 @@ void QNetworkAccessManagerPrivate::authenticationRequired(QAuthenticator *authen *urlForLastAuthentication = url; emit q->authenticationRequired(reply, authenticator); - authenticationManager->cacheCredentials(url, authenticator); + if (allowAuthenticationReuse) + authenticationManager->cacheCredentials(url, authenticator); } #ifndef QT_NO_NETWORKPROXY diff --git a/src/network/access/qnetworkaccessmanager_p.h b/src/network/access/qnetworkaccessmanager_p.h index 0733756be42..b0bcaabacc2 100644 --- a/src/network/access/qnetworkaccessmanager_p.h +++ b/src/network/access/qnetworkaccessmanager_p.h @@ -98,7 +98,8 @@ public: QNetworkReply *reply, bool synchronous, QUrl &url, - QUrl *urlForLastAuthentication); + QUrl *urlForLastAuthentication, + bool allowAuthenticationReuse = true); void cacheCredentials(const QUrl &url, const QAuthenticator *auth); QNetworkAuthenticationCredential *fetchCachedCredentials(const QUrl &url, const QAuthenticator *auth = 0); diff --git a/src/network/access/qnetworkreplyhttpimpl.cpp b/src/network/access/qnetworkreplyhttpimpl.cpp index 2124395de35..1f456746ae5 100644 --- a/src/network/access/qnetworkreplyhttpimpl.cpp +++ b/src/network/access/qnetworkreplyhttpimpl.cpp @@ -1179,10 +1179,10 @@ void QNetworkReplyHttpImplPrivate::replyDownloadProgressSlot(qint64 bytesReceive emit q->downloadProgress(bytesDownloaded, bytesTotal); } -void QNetworkReplyHttpImplPrivate::httpAuthenticationRequired(const QHttpNetworkRequest &, +void QNetworkReplyHttpImplPrivate::httpAuthenticationRequired(const QHttpNetworkRequest &request, QAuthenticator *auth) { - managerPrivate->authenticationRequired(auth, q_func(), synchronous, url, &urlForLastAuthentication); + managerPrivate->authenticationRequired(auth, q_func(), synchronous, url, &urlForLastAuthentication, request.withCredentials()); } #ifndef QT_NO_NETWORKPROXY diff --git a/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp b/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp index 00b3fd1973d..8685546a5f1 100644 --- a/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp +++ b/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp @@ -6335,29 +6335,52 @@ void tst_QNetworkReply::qtbug13431replyThrottling() void tst_QNetworkReply::httpWithNoCredentialUsage() { - QNetworkRequest request(QUrl("http://httptest:httptest@" + QtNetworkSettings::serverName() + "/qtest/protected/cgi-bin/md5sum.cgi")); - // Do not use credentials - request.setAttribute(QNetworkRequest::AuthenticationReuseAttribute, QNetworkRequest::Manual); QNetworkAccessManager manager; - QNetworkReplyPtr reply = manager.get(request); - qRegisterMetaType("QNetworkReply*"); - qRegisterMetaType("QAuthenticator*"); QSignalSpy authSpy(&manager, SIGNAL(authenticationRequired(QNetworkReply*,QAuthenticator*))); QSignalSpy finishedSpy(&manager, SIGNAL(finished(QNetworkReply*))); - qRegisterMetaType("QNetworkReply::NetworkError"); - QSignalSpy errorSpy(reply, SIGNAL(error(QNetworkReply::NetworkError))); - connect(reply, SIGNAL(finished()), &QTestEventLoop::instance(), SLOT(exitLoop()), Qt::QueuedConnection); - QTestEventLoop::instance().enterLoop(10); - QVERIFY(!QTestEventLoop::instance().timeout()); + // Get with credentials, to preload authentication cache + { + QNetworkRequest request(QUrl("http://httptest:httptest@" + QtNetworkSettings::serverName() + "/qtest/protected/cgi-bin/md5sum.cgi")); + QNetworkReplyPtr reply = manager.get(request); + QVERIFY(waitForFinish(reply) == Success); + // credentials in URL, so don't expect authentication signal + QCOMPARE(authSpy.count(), 0); + QCOMPARE(finishedSpy.count(), 1); + finishedSpy.clear(); + } - // We check if authenticationRequired was emitted, however we do not anything in it so it should be 401 - QCOMPARE(authSpy.count(), 1); - QCOMPARE(finishedSpy.count(), 1); - QCOMPARE(errorSpy.count(), 1); + // Get with cached credentials (normal usage) + { + QNetworkRequest request(QUrl("http://" + QtNetworkSettings::serverName() + "/qtest/protected/cgi-bin/md5sum.cgi")); + QNetworkReplyPtr reply = manager.get(request); + QVERIFY(waitForFinish(reply) == Success); + // credentials in cache, so don't expect authentication signal + QCOMPARE(authSpy.count(), 0); + QCOMPARE(finishedSpy.count(), 1); + finishedSpy.clear(); + } - QCOMPARE(reply->error(), QNetworkReply::AuthenticationRequiredError); + // Do not use cached credentials (webkit cross origin usage) + { + QNetworkRequest request(QUrl("http://" + QtNetworkSettings::serverName() + "/qtest/protected/cgi-bin/md5sum.cgi")); + request.setAttribute(QNetworkRequest::AuthenticationReuseAttribute, QNetworkRequest::Manual); + QNetworkReplyPtr reply = manager.get(request); + + QSignalSpy errorSpy(reply, SIGNAL(error(QNetworkReply::NetworkError))); + + connect(reply, SIGNAL(finished()), &QTestEventLoop::instance(), SLOT(exitLoop()), Qt::QueuedConnection); + QTestEventLoop::instance().enterLoop(10); + QVERIFY(!QTestEventLoop::instance().timeout()); + + // We check if authenticationRequired was emitted, however we do not anything in it so it should be 401 + QCOMPARE(authSpy.count(), 1); + QCOMPARE(finishedSpy.count(), 1); + QCOMPARE(errorSpy.count(), 1); + + QCOMPARE(reply->error(), QNetworkReply::AuthenticationRequiredError); + } } void tst_QNetworkReply::qtbug15311doubleContentLength()