Set default redirect policy to NoLessSafeRedirectPolicy

Not following redirects is not a feature, but just a hastle for everyone.

The main issue with switching this default is that applications that
actually do manual redirect handling will break in various ways.

FollowRedirectsAttribute was removed as it no longer serves any
purpose beyond duplicating the default value.

[ChangeLog][Network] QNetworkAccessManager now follows redirects by
default with the NoLessSafeRedirectPolicy.
[ChangeLog][Potentially Source-Incompatible Changes]
QNetworkRequest::FollowRedirectsAttribute was removed and has been
superseded by QNetworkRequest::RedirectsPolicyAttribute

Fixes: QTBUG-85901
Change-Id: Ic5b776180a4b84ac4fc895158bb5a66a3c91a042
Reviewed-by: Qt CI Bot <[email protected]>
Reviewed-by: Timur Pocheptsov <[email protected]>

src/network/access/qnetworkaccessmanager.cpp

@@ -1019,16 +1019,13 @@ void QNetworkAccessManager::connectToHost(const QString &hostName, quint16 port)
     Use this function to enable or disable HTTP redirects on the manager's level.
 
     \note When creating a request QNetworkRequest::RedirectAttributePolicy has
-    the highest priority, next by priority is QNetworkRequest::FollowRedirectsAttribute.
-    Finally, the manager's policy has the lowest priority.
+    the highest priority, next by priority the manager's policy.
 
-    For backwards compatibility the default value is QNetworkRequest::ManualRedirectPolicy.
-    This may change in the future and some type of auto-redirect policy will become
-    the default; clients relying on manual redirect handling are encouraged to set
+    The default value is QNetworkRequest::NoLessSafeRedirectPolicy.
+    Clients relying on manual redirect handling are encouraged to set
     this policy explicitly in their code.
 
-    \sa redirectPolicy(), QNetworkRequest::RedirectPolicy,
-    QNetworkRequest::FollowRedirectsAttribute
+    \sa redirectPolicy(), QNetworkRequest::RedirectPolicy
 */
 void QNetworkAccessManager::setRedirectPolicy(QNetworkRequest::RedirectPolicy policy)
 {
@@ -1138,9 +1135,8 @@ QNetworkReply *QNetworkAccessManager::createRequest(QNetworkAccessManager::Opera
     Q_D(QNetworkAccessManager);
 
     QNetworkRequest req(originalReq);
-    if (redirectPolicy() != QNetworkRequest::ManualRedirectPolicy
-        && req.attribute(QNetworkRequest::RedirectPolicyAttribute).isNull()
-        && req.attribute(QNetworkRequest::FollowRedirectsAttribute).isNull()) {
+    if (redirectPolicy() != QNetworkRequest::NoLessSafeRedirectPolicy
+        && req.attribute(QNetworkRequest::RedirectPolicyAttribute).isNull()) {
         req.setAttribute(QNetworkRequest::RedirectPolicyAttribute, redirectPolicy());
     }
 

src/network/access/qnetworkaccessmanager_p.h

@@ -85,7 +85,7 @@ class QNetworkAccessManagerPrivate: public QObjectPrivate
 #endif
           cookieJarCreated(false),
           defaultAccessControl(true),
-          redirectPolicy(QNetworkRequest::ManualRedirectPolicy),
+          redirectPolicy(QNetworkRequest::NoLessSafeRedirectPolicy),
           authenticationManager(QSharedPointer<QNetworkAccessAuthenticationManager>::create())
     {
     }
@@ -145,7 +145,7 @@ class QNetworkAccessManagerPrivate: public QObjectPrivate
 
     bool cookieJarCreated;
     bool defaultAccessControl;
-    QNetworkRequest::RedirectPolicy redirectPolicy;
+    QNetworkRequest::RedirectPolicy redirectPolicy = QNetworkRequest::NoLessSafeRedirectPolicy;
 
     // The cache with authorization data:
     QSharedPointer<QNetworkAccessAuthenticationManager> authenticationManager;

src/network/access/qnetworkreply.cpp

@@ -298,13 +298,13 @@ QNetworkReplyPrivate::QNetworkReplyPrivate()
     \fn void QNetworkReply::redirected(const QUrl &url)
     \since 5.6
 
-    This signal is emitted if the QNetworkRequest::FollowRedirectsAttribute was
+    This signal is emitted if the QNetworkRequest::ManualRedirectPolicy was
     set in the request and the server responded with a 3xx status (specifically
     301, 302, 303, 305, 307 or 308 status code) with a valid url in the location
     header, indicating a HTTP redirect. The \a url parameter contains the new
     redirect url as returned by the server in the location header.
 
-    \sa QNetworkRequest::FollowRedirectsAttribute
+    \sa QNetworkRequest::RedirectPolicy
 */
 
 /*!
@@ -596,10 +596,10 @@ bool QNetworkReply::isRunning() const
 
 /*!
     Returns the URL of the content downloaded or uploaded. Note that
-    the URL may be different from that of the original request. If the
-    QNetworkRequest::FollowRedirectsAttribute was set in the request, then this
+    the URL may be different from that of the original request.
+    If redirections were enabled in the request, then this
     function returns the current url that the network API is accessing, i.e the
-    url emitted in the QNetworkReply::redirected signal.
+    url of the resource the request got redirected to.
 
     \sa request(), setUrl(), QNetworkRequest::url(), redirected()
 */

src/network/access/qnetworkreplyhttpimpl.cpp

@@ -670,12 +670,10 @@ void QNetworkReplyHttpImplPrivate::postRequest(const QNetworkRequest &newHttpReq
     }
 #endif
 
-    auto redirectPolicy = QNetworkRequest::ManualRedirectPolicy;
+    auto redirectPolicy = QNetworkRequest::NoLessSafeRedirectPolicy;
     const QVariant value = newHttpRequest.attribute(QNetworkRequest::RedirectPolicyAttribute);
     if (value.isValid())
         redirectPolicy = qvariant_cast<QNetworkRequest::RedirectPolicy>(value);
-    else if (newHttpRequest.attribute(QNetworkRequest::FollowRedirectsAttribute).toBool())
-        redirectPolicy = QNetworkRequest::NoLessSafeRedirectPolicy;
 
     httpRequest.setRedirectPolicy(redirectPolicy);
 

src/network/access/qnetworkrequest.cpp

@@ -168,13 +168,11 @@ QT_BEGIN_NAMESPACE
     \value RedirectionTargetAttribute
         Replies only, type: QMetaType::QUrl (no default)
         If present, it indicates that the server is redirecting the
-        request to a different URL. The Network Access API does not by
-        default follow redirections: the application can
-        determine if the requested redirection should be allowed,
-        according to its security policies, or it can set
-        QNetworkRequest::FollowRedirectsAttribute to true (in which case
-        the redirection will be followed and this attribute will not
-        be present in the reply).
+        request to a different URL. The Network Access API does follow
+        redirections by default, but if
+        QNetworkRequest::ManualRedirectPolicy is enabled and
+        the redirect was not handled in redirected() then this
+        attribute will be present.
         The returned URL might be relative. Use QUrl::resolved()
         to create an absolute URL out of it.
 
@@ -288,13 +286,6 @@ QT_BEGIN_NAMESPACE
         in 100 millisecond intervals.
         (This value was introduced in 5.5.)
 
-    \value FollowRedirectsAttribute
-        Requests only, type: QMetaType::Bool (default: false)
-        Indicates whether the Network Access API should automatically follow a
-        HTTP redirect response or not. Currently redirects that are insecure,
-        that is redirecting from "https" to "http" protocol, are not allowed.
-        (This value was introduced in 5.6.)
-
     \value OriginalContentLengthAttribute
         Replies only, type QMetaType::Int
         Holds the original content-length attribute before being invalidated and
@@ -304,8 +295,8 @@ QT_BEGIN_NAMESPACE
 
     \value RedirectPolicyAttribute
         Requests only, type: QMetaType::Int, should be one of the
-        QNetworkRequest::RedirectPolicy values (default: ManualRedirectPolicy).
-        This attribute obsoletes FollowRedirectsAttribute.
+        QNetworkRequest::RedirectPolicy values
+        (default: NoLessSafeRedirectPolicy).
         (This value was introduced in 5.9.)
 
     \value Http2DirectAttribute
@@ -386,8 +377,6 @@ QT_BEGIN_NAMESPACE
 
     \value NoLessSafeRedirectPolicy    Only "http"->"http", "http" -> "https"
                                        or "https" -> "https" redirects are allowed.
-                                       Equivalent to setting the old FollowRedirectsAttribute
-                                       to true
 
     \value SameOriginRedirectPolicy    Require the same protocol, host and port.
                                        Note, http://example.com and http://example.com:80
@@ -493,6 +482,7 @@ class QNetworkRequestPrivate: public QSharedData, public QNetworkHeadersPrivate
 QNetworkRequest::QNetworkRequest()
     : d(new QNetworkRequestPrivate)
 {
+
 #if QT_CONFIG(http)
     // Initial values proposed by RFC 7540 are quite draconian,
     // so unless an application will set its own parameters, we

src/network/access/qnetworkrequest.h

@@ -90,7 +90,6 @@ class Q_NETWORK_EXPORT QNetworkRequest
         SynchronousRequestAttribute, // internal
         BackgroundRequestAttribute,
         EmitAllUploadProgressSignalsAttribute = BackgroundRequestAttribute + 3,
-        FollowRedirectsAttribute,
         Http2AllowedAttribute,
         Http2WasUsedAttribute,
         OriginalContentLengthAttribute,

tests/auto/network/access/http2/tst_http2.cpp

@@ -935,7 +935,7 @@ void tst_Http2::sendRequest(int streamNumber,
 
     QNetworkRequest request(url);
     request.setAttribute(QNetworkRequest::Http2AllowedAttribute, QVariant(true));
-    request.setAttribute(QNetworkRequest::FollowRedirectsAttribute, QVariant(true));
+    request.setAttribute(QNetworkRequest::RedirectPolicyAttribute, QNetworkRequest::NoLessSafeRedirectPolicy);
     request.setHeader(QNetworkRequest::ContentTypeHeader, QVariant("text/plain"));
     request.setPriority(priority);
     request.setHttp2Configuration(h2Config);

tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp

@@ -8178,7 +8178,7 @@ void tst_QNetworkReply::ioHttpSingleRedirect()
 
     localhost.setPort(server.serverPort());
     QNetworkRequest request(localhost);
-    request.setAttribute(QNetworkRequest::FollowRedirectsAttribute, true);
+    request.setAttribute(QNetworkRequest::RedirectPolicyAttribute, QNetworkRequest::NoLessSafeRedirectPolicy);
 
     QNetworkReplyPtr reply(manager.get(request));
     QSignalSpy redSpy(reply.data(), SIGNAL(redirected(QUrl)));
@@ -8223,7 +8223,7 @@ void tst_QNetworkReply::ioHttpChangeMaxRedirects()
 
     localhost.setPort(server1.serverPort());
     QNetworkRequest request(localhost);
-    request.setAttribute(QNetworkRequest::FollowRedirectsAttribute, true);
+    request.setAttribute(QNetworkRequest::RedirectPolicyAttribute, QNetworkRequest::NoLessSafeRedirectPolicy);
 
     // Set Max redirects to 1. This will cause TooManyRedirectsError
     request.setMaximumRedirectsAllowed(1);
@@ -8285,7 +8285,7 @@ void tst_QNetworkReply::ioHttpRedirectErrors()
     server.setDataToTransmit(d2s);
 
     QNetworkRequest request(localhost);
-    request.setAttribute(QNetworkRequest::FollowRedirectsAttribute, true);
+    request.setAttribute(QNetworkRequest::RedirectPolicyAttribute, QNetworkRequest::NoLessSafeRedirectPolicy);
     QNetworkReplyPtr reply(manager.get(request));
     if (localhost.scheme() == "https")
         reply.data()->ignoreSslErrors();
@@ -8366,7 +8366,7 @@ void tst_QNetworkReply::ioHttpRedirectPolicy()
     redirectServer.responses.push_back(tempRedirectReplyStr().arg(QString(url.toEncoded())).toLatin1());
 
     // This is the default one we preserve between tests.
-    QCOMPARE(manager.redirectPolicy(), QNetworkRequest::ManualRedirectPolicy);
+    QCOMPARE(manager.redirectPolicy(), QNetworkRequest::NoLessSafeRedirectPolicy);
 
     manager.setRedirectPolicy(policy);
     QCOMPARE(manager.redirectPolicy(), policy);
@@ -8375,7 +8375,7 @@ void tst_QNetworkReply::ioHttpRedirectPolicy()
         reply->ignoreSslErrors();
 
     // Restore default:
-    manager.setRedirectPolicy(QNetworkRequest::ManualRedirectPolicy);
+    manager.setRedirectPolicy(QNetworkRequest::NoLessSafeRedirectPolicy);
     QSignalSpy redirectSpy(reply.data(), SIGNAL(redirected(QUrl)));
     QSignalSpy finishedSpy(reply.data(), SIGNAL(finished()));
     QVERIFY2(waitForFinish(reply) == Success, msgWaitForFinished(reply));
@@ -8447,15 +8447,15 @@ void tst_QNetworkReply::ioHttpRedirectPolicyErrors()
 
     QNetworkRequest request(url);
     request.setMaximumRedirectsAllowed(maxRedirects);
-    // We always reset the policy to the default one ('Manual') after any related
+    // We always reset the policy to the default one ('NoLessSafe') after any related
     // test is finished:
-    QCOMPARE(manager.redirectPolicy(), QNetworkRequest::ManualRedirectPolicy);
+    QCOMPARE(manager.redirectPolicy(), QNetworkRequest::NoLessSafeRedirectPolicy);
     manager.setRedirectPolicy(policy);
     QCOMPARE(manager.redirectPolicy(), policy);
 
     QNetworkReplyPtr reply(manager.get(request));
     // Set it back to default:
-    manager.setRedirectPolicy(QNetworkRequest::ManualRedirectPolicy);
+    manager.setRedirectPolicy(QNetworkRequest::NoLessSafeRedirectPolicy);
 
     if (ssl)
         reply->ignoreSslErrors();
@@ -8490,7 +8490,7 @@ void tst_QNetworkReply::ioHttpUserVerifiedRedirect()
     redirectServer.setDataToTransmit(tempRedirectReplyStr().arg(QString(url.toEncoded())).toLatin1());
     url.setPort(redirectServer.serverPort());
 
-    QCOMPARE(manager.redirectPolicy(), QNetworkRequest::ManualRedirectPolicy);
+    QCOMPARE(manager.redirectPolicy(), QNetworkRequest::NoLessSafeRedirectPolicy);
     manager.setRedirectPolicy(QNetworkRequest::UserVerifiedRedirectPolicy);
     QCOMPARE(manager.redirectPolicy(), QNetworkRequest::UserVerifiedRedirectPolicy);
 
@@ -8508,8 +8508,8 @@ void tst_QNetworkReply::ioHttpUserVerifiedRedirect()
                    });
 
     // Before any test failed, reset the policy to default:
-    manager.setRedirectPolicy(QNetworkRequest::ManualRedirectPolicy);
-    QCOMPARE(manager.redirectPolicy(), QNetworkRequest::ManualRedirectPolicy);
+    manager.setRedirectPolicy(QNetworkRequest::NoLessSafeRedirectPolicy);
+    QCOMPARE(manager.redirectPolicy(), QNetworkRequest::NoLessSafeRedirectPolicy);
 
     QSignalSpy finishedSpy(reply.data(), SIGNAL(finished()));
     waitForFinish(reply);