From 231259c3d519a55880563b12f5796723fa99e522 Mon Sep 17 00:00:00 2001 From: Timur Pocheptsov Date: Mon, 2 Jan 2017 15:47:15 +0100 Subject: Add a user-controlled auto-redirect policy With this new policy, after emitting 'redirected', QNetworkReplyHttpImpl waits for client code to decide if QNAM should follow this redirect or not. The client can either allow this redirect by emitting 'redirectAllowed' or abort the reply. Task-number: QTPM-236 Change-Id: Ia04619f6bd1f0caa477833ae859b24033027b2e1 Reviewed-by: Timur Pocheptsov Reviewed-by: Edward Welbourne --- src/network/access/qhttpnetworkconnection.cpp | 2 + src/network/access/qnetworkreply.cpp | 14 +++++ src/network/access/qnetworkreply.h | 1 + src/network/access/qnetworkreplyhttpimpl.cpp | 21 +++++-- src/network/access/qnetworkreplyhttpimpl_p.h | 3 + src/network/access/qnetworkrequest.cpp | 10 ++++ src/network/access/qnetworkrequest.h | 3 +- .../access/qnetworkreply/tst_qnetworkreply.cpp | 65 ++++++++++++++++++++-- 8 files changed, 107 insertions(+), 12 deletions(-) diff --git a/src/network/access/qhttpnetworkconnection.cpp b/src/network/access/qhttpnetworkconnection.cpp index e5c6c2f81c..327eaf91bc 100644 --- a/src/network/access/qhttpnetworkconnection.cpp +++ b/src/network/access/qhttpnetworkconnection.cpp @@ -567,6 +567,8 @@ QUrl QHttpNetworkConnectionPrivate::parseRedirectResponse(QAbstractSocket *socke return QUrl(); } break; + case QNetworkRequest::UserVerifiedRedirectsPolicy: + break; default: Q_ASSERT(!"Unexpected redirect policy"); } diff --git a/src/network/access/qnetworkreply.cpp b/src/network/access/qnetworkreply.cpp index b35cfe03d7..17990f02f5 100644 --- a/src/network/access/qnetworkreply.cpp +++ b/src/network/access/qnetworkreply.cpp @@ -304,6 +304,20 @@ QNetworkReplyPrivate::QNetworkReplyPrivate() \sa QNetworkRequest::FollowRedirectsAttribute */ +/*! + \fn void QNetworkReply::redirectAllowed() + \since 5.9 + + When client code handling the redirected() signal has verified the new URL, + it emits this signal to allow the redirect to go ahead. This protocol applies + to network requests whose redirects policy is set to + QNetworkRequest::UserVerifiedRedirectsPolicy. + + \sa QNetworkRequest::UserVerifiedRedirectsPolicy + QNetworkAccessManager::setRedirectsPolicy(), + QNetworkRequest::RedirectsPolicyAttribute +*/ + /*! \fn void QNetworkReply::metaDataChanged() diff --git a/src/network/access/qnetworkreply.h b/src/network/access/qnetworkreply.h index 1419db8597..d858e07d84 100644 --- a/src/network/access/qnetworkreply.h +++ b/src/network/access/qnetworkreply.h @@ -163,6 +163,7 @@ Q_SIGNALS: void preSharedKeyAuthenticationRequired(QSslPreSharedKeyAuthenticator *authenticator); #endif void redirected(const QUrl &url); + void redirectAllowed(); void uploadProgress(qint64 bytesSent, qint64 bytesTotal); void downloadProgress(qint64 bytesReceived, qint64 bytesTotal); diff --git a/src/network/access/qnetworkreplyhttpimpl.cpp b/src/network/access/qnetworkreplyhttpimpl.cpp index 03ed596586..19f424b35f 100644 --- a/src/network/access/qnetworkreplyhttpimpl.cpp +++ b/src/network/access/qnetworkreplyhttpimpl.cpp @@ -824,12 +824,16 @@ void QNetworkReplyHttpImplPrivate::postRequest(const QNetworkRequest &newHttpReq QObject::connect(delegate, SIGNAL(redirected(QUrl,int,int)), q, SLOT(onRedirected(QUrl,int,int)), Qt::QueuedConnection); + + QObject::connect(q, SIGNAL(redirectAllowed()), q, SLOT(followRedirect()), + Qt::QueuedConnection); + #ifndef QT_NO_SSL QObject::connect(delegate, SIGNAL(sslConfigurationChanged(QSslConfiguration)), q, SLOT(replySslConfigurationChanged(QSslConfiguration)), Qt::QueuedConnection); #endif - // Those need to report back, therefire BlockingQueuedConnection + // Those need to report back, therefore BlockingQueuedConnection QObject::connect(delegate, SIGNAL(authenticationRequired(QHttpNetworkRequest,QAuthenticator*)), q, SLOT(httpAuthenticationRequired(QHttpNetworkRequest,QAuthenticator*)), Qt::BlockingQueuedConnection); @@ -1121,19 +1125,26 @@ void QNetworkReplyHttpImplPrivate::onRedirected(const QUrl &redirectUrl, int htt if (httpRequest.isFollowRedirects()) // update the reply's url as it could've changed url = redirectUrl; - QNetworkRequest redirectRequest = createRedirectRequest(originalRequest, redirectUrl, maxRedirectsRemaining); + redirectRequest = createRedirectRequest(originalRequest, redirectUrl, maxRedirectsRemaining); operation = getRedirectOperation(operation, httpStatus); + if (httpRequest.redirectsPolicy() != QNetworkRequest::UserVerifiedRedirectsPolicy) + followRedirect(); + + emit q->redirected(redirectUrl); +} + +void QNetworkReplyHttpImplPrivate::followRedirect() +{ + Q_Q(QNetworkReplyHttpImpl); + cookedHeaders.clear(); if (managerPrivate->thread) managerPrivate->thread->disconnect(); - // Recurse QMetaObject::invokeMethod(q, "start", Qt::QueuedConnection, Q_ARG(QNetworkRequest, redirectRequest)); - - emit q->redirected(redirectUrl); } void QNetworkReplyHttpImplPrivate::checkForRedirect(const int statusCode) diff --git a/src/network/access/qnetworkreplyhttpimpl_p.h b/src/network/access/qnetworkreplyhttpimpl_p.h index 255c23006e..9383149124 100644 --- a/src/network/access/qnetworkreplyhttpimpl_p.h +++ b/src/network/access/qnetworkreplyhttpimpl_p.h @@ -136,6 +136,7 @@ public: Q_PRIVATE_SLOT(d_func(), void _q_cacheSaveDeviceAboutToClose()) Q_PRIVATE_SLOT(d_func(), void _q_metaDataChanged()) Q_PRIVATE_SLOT(d_func(), void onRedirected(const QUrl &, int, int)) + Q_PRIVATE_SLOT(d_func(), void followRedirect()) #ifndef QT_NO_SSL protected: @@ -212,6 +213,7 @@ public: QSharedPointer outgoingDataBuffer; void emitReplyUploadProgress(qint64 bytesSent, qint64 bytesTotal); // dup? void onRedirected(const QUrl &redirectUrl, int httpStatus, int maxRedirectsRemainig); + void followRedirect(); qint64 bytesUploaded; @@ -263,6 +265,7 @@ public: QList pendingIgnoreSslErrorsList; #endif + QNetworkRequest redirectRequest; bool loadFromCacheIfAllowed(QHttpNetworkRequest &httpRequest); void invalidateCache(); diff --git a/src/network/access/qnetworkrequest.cpp b/src/network/access/qnetworkrequest.cpp index abc924d0e2..169695fa27 100644 --- a/src/network/access/qnetworkrequest.cpp +++ b/src/network/access/qnetworkrequest.cpp @@ -360,6 +360,16 @@ QT_BEGIN_NAMESPACE Note, http://example.com and http://example.com:80 will fail with this policy (implicit/explicit ports are considered to be a mismatch). + + \value UserVerifiedRedirectsPolicy Client decides whether to follow each + redirect by handling the redirected() + signal, emitting redirectAllowed() on + the QNetworkReply object to allow + the redirect or aborting/finishing it to + reject the redirect. This can be used, + for example, to ask the user whether to + accept the redirect, or to decide + based on some app-specific configuration. */ class QNetworkRequestPrivate: public QSharedData, public QNetworkHeadersPrivate diff --git a/src/network/access/qnetworkrequest.h b/src/network/access/qnetworkrequest.h index 52c398663a..06c895af5f 100644 --- a/src/network/access/qnetworkrequest.h +++ b/src/network/access/qnetworkrequest.h @@ -116,7 +116,8 @@ public: enum RedirectsPolicy { ManualRedirectsPolicy, NoLessSafeRedirectsPolicy, - SameOriginRedirectsPolicy + SameOriginRedirectsPolicy, + UserVerifiedRedirectsPolicy }; diff --git a/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp b/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp index 7d07ae888c..f19ce4ac75 100644 --- a/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp +++ b/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp @@ -134,6 +134,8 @@ class tst_QNetworkReply: public QObject return s; } + static const QByteArray httpEmpty200Response; + QEventLoop *loop; enum RunSimpleRequestReturn { Timeout = 0, Success, Failure }; int returnCode; @@ -480,6 +482,8 @@ private Q_SLOTS: void ioHttpRedirectPolicy(); void ioHttpRedirectPolicyErrors_data(); void ioHttpRedirectPolicyErrors(); + void ioHttpUserVerifiedRedirect_data(); + void ioHttpUserVerifiedRedirect(); #ifndef QT_NO_SSL void putWithServerClosingConnectionImmediately(); #endif @@ -493,6 +497,8 @@ private: bool notEnoughDataForFastSender; }; +const QByteArray tst_QNetworkReply::httpEmpty200Response = + "HTTP/1.1 200 OK\r\nContent-Length: 0\r\n\r\n"; bool tst_QNetworkReply::seedCreated = false; #define RUN_REQUEST(call) \ @@ -8034,10 +8040,9 @@ void tst_QNetworkReply::putWithRateLimiting() void tst_QNetworkReply::ioHttpSingleRedirect() { QUrl localhost = QUrl("http://localhost"); - QByteArray http200Reply = "HTTP/1.1 200 OK\r\nContent-Length: 0\r\n\r\n"; // Setup server to which the second server will redirect to - MiniHttpServer server2(http200Reply); + MiniHttpServer server2(httpEmpty200Response); QUrl redirectUrl = QUrl(localhost); redirectUrl.setPort(server2.serverPort()); @@ -8079,11 +8084,9 @@ void tst_QNetworkReply::ioHttpChangeMaxRedirects() { QUrl localhost = QUrl("http://localhost"); - QByteArray http200Reply = "HTTP/1.1 200 OK\r\nContent-Length: 0\r\n\r\n"; - MiniHttpServer server1(""); MiniHttpServer server2(""); - MiniHttpServer server3(http200Reply); + MiniHttpServer server3(httpEmpty200Response); QUrl server2Url(localhost); server2Url.setPort(server2.serverPort()); @@ -8227,7 +8230,7 @@ void tst_QNetworkReply::ioHttpRedirectPolicy() "http://localhost")); url.setPort(redirectServer.serverPort()); - redirectServer.responses.push_back("HTTP/1.1 200 OK\r\nContent-Length: 0\r\n\r\n"); + redirectServer.responses.push_back(httpEmpty200Response); redirectServer.responses.push_back(tempRedirectReplyStr().arg(QString(url.toEncoded())).toLatin1()); // This is the default one we preserve between tests. @@ -8332,6 +8335,56 @@ void tst_QNetworkReply::ioHttpRedirectPolicyErrors() QCOMPARE(reply->error(), expectedError); } +void tst_QNetworkReply::ioHttpUserVerifiedRedirect_data() +{ + QTest::addColumn("followRedirect"); + QTest::addColumn("statusCode"); + + QTest::newRow("allow-redirect") << true << 200; + QTest::newRow("reject-redirect") << false << 307; +} + +void tst_QNetworkReply::ioHttpUserVerifiedRedirect() +{ + QFETCH(const bool, followRedirect); + QFETCH(const int, statusCode); + + // Setup HTTP server. + MiniHttpServer target(httpEmpty200Response, false); + QUrl url("http://localhost"); + url.setPort(target.serverPort()); + + MiniHttpServer redirectServer("", false); + redirectServer.setDataToTransmit(tempRedirectReplyStr().arg(QString(url.toEncoded())).toLatin1()); + url.setPort(redirectServer.serverPort()); + + QCOMPARE(manager.redirectsPolicy(), QNetworkRequest::ManualRedirectsPolicy); + manager.setRedirectsPolicy(QNetworkRequest::UserVerifiedRedirectsPolicy); + QCOMPARE(manager.redirectsPolicy(), QNetworkRequest::UserVerifiedRedirectsPolicy); + + QNetworkReplyPtr reply(manager.get(QNetworkRequest(url))); + reply->connect(reply.data(), &QNetworkReply::redirected, + [&](const QUrl &redirectUrl) { + qDebug() << "redirect to:" << redirectUrl; + if (followRedirect) { + qDebug() << "confirmed."; + emit reply->redirectAllowed(); + } else{ + qDebug() << "rejected."; + emit reply->abort(); + } + }); + + // Before any test failed, reset the policy to default: + manager.setRedirectsPolicy(QNetworkRequest::ManualRedirectsPolicy); + QCOMPARE(manager.redirectsPolicy(), QNetworkRequest::ManualRedirectsPolicy); + + QSignalSpy finishedSpy(reply.data(), SIGNAL(finished())); + waitForFinish(reply); + QCOMPARE(finishedSpy.count(), 1); + QCOMPARE(reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(), statusCode); +} + #ifndef QT_NO_SSL class PutWithServerClosingConnectionImmediatelyHandler: public QObject -- cgit v1.2.3