QHttpNetworkConnectionChannel: remove *authMethod

We store the authenticator right alongside it, which knows the method. The biggest change from this is changing a, self-proclaimed, duplicate method from QHttpNetworkReply. Finding the method name ahead-of-time is not actually needed, all we need to know is that a supported authentication method is requested. Also moved that specific functionality to a more logical location: QAuthenticatorPrivate. Change-Id: I11627803ccb42b8ec33a28ef1d1e00bf60dc6da9 Reviewed-by: Edward Welbourne <[email protected]> Reviewed-by: Timur Pocheptsov <[email protected]>
chain78 · May 20, 2021 · 55feb0d · 55feb0d
1 parent 5a701f5
commit 55feb0d
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 64 deletions.
diff --git a/src/network/access/qhttpnetworkconnection.cpp b/src/network/access/qhttpnetworkconnection.cpp
@@ -397,10 +397,12 @@ void QHttpNetworkConnectionPrivate::copyCredentials(int fromChannel, QAuthentica
     // NTLM and Negotiate do multi-phase authentication.
     // Copying credentialsbetween authenticators would mess things up.
     if (fromChannel >= 0) {
-        const QHttpNetworkConnectionChannel &channel = channels[fromChannel];
-        const QAuthenticatorPrivate::Method method = isProxy ? channel.proxyAuthMethod : channel.authMethod;
-        if (method == QAuthenticatorPrivate::Ntlm || method == QAuthenticatorPrivate::Negotiate)
+        QAuthenticatorPrivate *priv = QAuthenticatorPrivate::getPrivate(*auth);
+        if (priv
+            && (priv->method == QAuthenticatorPrivate::Ntlm
+                || priv->method == QAuthenticatorPrivate::Negotiate)) {
             return;
+        }
     }
 
     // select another channel
@@ -432,19 +434,16 @@ bool QHttpNetworkConnectionPrivate::handleAuthenticateChallenge(QAbstractSocket
     //create the response header to be used with QAuthenticatorPrivate.
     QList<QPair<QByteArray, QByteArray> > fields = reply->header();
 
-    //find out the type of authentication protocol requested.
-    QAuthenticatorPrivate::Method authMethod = reply->d_func()->authenticationMethod(isProxy);
-    if (authMethod != QAuthenticatorPrivate::None) {
+    // Check that any of the proposed authenticate methods are supported
+    const QByteArray header = isProxy ? "proxy-authenticate" : "www-authenticate";
+    const QByteArrayList &authenticationMethods = reply->d_func()->headerFieldValues(header);
+    const bool isSupported = std::any_of(authenticationMethods.begin(), authenticationMethods.end(),
+                                         QAuthenticatorPrivate::isMethodSupported);
+    if (isSupported) {
         int i = indexOf(socket);
         //Use a single authenticator for all domains. ### change later to use domain/realm
-        QAuthenticator* auth = nullptr;
-        if (isProxy) {
-            auth = &channels[i].proxyAuthenticator;
-            channels[i].proxyAuthMethod = authMethod;
-        } else {
-            auth = &channels[i].authenticator;
-            channels[i].authMethod = authMethod;
-        }
+        QAuthenticator *auth = isProxy ? &channels[i].proxyAuthenticator
+                                       : &channels[i].authenticator;
         //proceed with the authentication.
         if (auth->isNull())
             auth->detach();
@@ -453,10 +452,6 @@ bool QHttpNetworkConnectionPrivate::handleAuthenticateChallenge(QAbstractSocket
         // Update method in case it changed
         if (priv->method == QAuthenticatorPrivate::None)
             return false;
-        if (isProxy)
-            channels[i].proxyAuthMethod = priv->method;
-        else
-            channels[i].authMethod = priv->method;
 
         if (priv->phase == QAuthenticatorPrivate::Done) {
             pauseConnection();
@@ -591,28 +586,30 @@ void QHttpNetworkConnectionPrivate::createAuthorization(QAbstractSocket *socket,
 
     int i = indexOf(socket);
 
+    QAuthenticator *authenticator = &channels[i].authenticator;
+    QAuthenticatorPrivate *priv = QAuthenticatorPrivate::getPrivate(*authenticator);
     // Send "Authorization" header, but not if it's NTLM and the socket is already authenticated.
-    if (channels[i].authMethod != QAuthenticatorPrivate::None) {
-        if ((channels[i].authMethod != QAuthenticatorPrivate::Ntlm && request.headerField("Authorization").isEmpty()) || channels[i].lastStatus == 401) {
-            QAuthenticatorPrivate *priv = QAuthenticatorPrivate::getPrivate(channels[i].authenticator);
-            if (priv && priv->method != QAuthenticatorPrivate::None) {
-                QByteArray response = priv->calculateResponse(request.methodName(), request.uri(false), request.url().host());
-                request.setHeaderField("Authorization", response);
-                channels[i].authenticationCredentialsSent = true;
-            }
+    if (priv && priv->method != QAuthenticatorPrivate::None) {
+        if ((priv->method != QAuthenticatorPrivate::Ntlm
+             && request.headerField("Authorization").isEmpty())
+            || channels[i].lastStatus == 401) {
+            QByteArray response = priv->calculateResponse(request.methodName(), request.uri(false),
+                                                          request.url().host());
+            request.setHeaderField("Authorization", response);
+            channels[i].authenticationCredentialsSent = true;
         }
     }
 
 #if QT_CONFIG(networkproxy)
+    authenticator = &channels[i].proxyAuthenticator;
+    priv = QAuthenticatorPrivate::getPrivate(*authenticator);
     // Send "Proxy-Authorization" header, but not if it's NTLM and the socket is already authenticated.
-    if (channels[i].proxyAuthMethod != QAuthenticatorPrivate::None) {
-        if (!(channels[i].proxyAuthMethod == QAuthenticatorPrivate::Ntlm && channels[i].lastStatus != 407)) {
-            QAuthenticatorPrivate *priv = QAuthenticatorPrivate::getPrivate(channels[i].proxyAuthenticator);
-            if (priv && priv->method != QAuthenticatorPrivate::None) {
-                QByteArray response = priv->calculateResponse(request.methodName(), request.uri(false), networkProxy.hostName());
-                request.setHeaderField("Proxy-Authorization", response);
-                channels[i].proxyCredentialsSent = true;
-            }
+    if (priv && priv->method != QAuthenticatorPrivate::None) {
+        if (priv->method != QAuthenticatorPrivate::Ntlm || channels[i].lastStatus == 407) {
+            QByteArray response = priv->calculateResponse(request.methodName(), request.uri(false),
+                                                          networkProxy.hostName());
+            request.setHeaderField("Proxy-Authorization", response);
+            channels[i].proxyCredentialsSent = true;
         }
     }
 #endif // QT_CONFIG(networkproxy)

diff --git a/src/network/access/qhttpnetworkconnectionchannel.cpp b/src/network/access/qhttpnetworkconnectionchannel.cpp
@@ -93,8 +93,6 @@ QHttpNetworkConnectionChannel::QHttpNetworkConnectionChannel()
     , lastStatus(0)
     , pendingEncrypt(false)
     , reconnectAttempts(reconnectAttemptsDefault)
-    , authMethod(QAuthenticatorPrivate::None)
-    , proxyAuthMethod(QAuthenticatorPrivate::None)
     , authenticationCredentialsSent(false)
     , proxyCredentialsSent(false)
     , protocolHandler(nullptr)

diff --git a/src/network/access/qhttpnetworkconnectionchannel_p.h b/src/network/access/qhttpnetworkconnectionchannel_p.h
@@ -116,8 +116,6 @@ class QHttpNetworkConnectionChannel : public QObject {
     int lastStatus; // last status received on this channel
     bool pendingEncrypt; // for https (send after encrypted)
     int reconnectAttempts; // maximum 2 reconnection attempts
-    QAuthenticatorPrivate::Method authMethod;
-    QAuthenticatorPrivate::Method proxyAuthMethod;
     QAuthenticator authenticator;
     QAuthenticator proxyAuthenticator;
     bool authenticationCredentialsSent;

diff --git a/src/network/access/qhttpnetworkreply.cpp b/src/network/access/qhttpnetworkreply.cpp
@@ -411,31 +411,6 @@ bool QHttpNetworkReplyPrivate::findChallenge(bool forProxy, QByteArray &challeng
     return !challenge.isEmpty();
 }
 
-QAuthenticatorPrivate::Method QHttpNetworkReplyPrivate::authenticationMethod(bool isProxy) const
-{
-    // The logic is same as the one used in void QAuthenticatorPrivate::parseHttpResponse()
-    QAuthenticatorPrivate::Method method = QAuthenticatorPrivate::None;
-    QByteArray header = isProxy ? "proxy-authenticate" : "www-authenticate";
-    QList<QByteArray> challenges = headerFieldValues(header);
-    for (int i = 0; i<challenges.size(); i++) {
-        QByteArray line = challenges.at(i).trimmed().toLower();
-        if (method < QAuthenticatorPrivate::Basic
-            && line.startsWith("basic")) {
-            method = QAuthenticatorPrivate::Basic;
-        } else if (method < QAuthenticatorPrivate::Ntlm
-            && line.startsWith("ntlm")) {
-            method = QAuthenticatorPrivate::Ntlm;
-        } else if (method < QAuthenticatorPrivate::DigestMd5
-            && line.startsWith("digest")) {
-            method = QAuthenticatorPrivate::DigestMd5;
-        } else if (method < QAuthenticatorPrivate::Negotiate
-            && line.startsWith("negotiate")) {
-            method = QAuthenticatorPrivate::Negotiate;
-        }
-    }
-    return method;
-}
-
 qint64 QHttpNetworkReplyPrivate::readStatus(QAbstractSocket *socket)
 {
     if (fragment.isEmpty()) {

diff --git a/src/network/access/qhttpnetworkreply_p.h b/src/network/access/qhttpnetworkreply_p.h
@@ -207,7 +207,6 @@ class Q_AUTOTEST_EXPORT QHttpNetworkReplyPrivate : public QObjectPrivate, public
     qint64 readBodyVeryFast(QAbstractSocket *socket, char *b);
     qint64 readBodyFast(QAbstractSocket *socket, QByteDataBuffer *rb);
     bool findChallenge(bool forProxy, QByteArray &challenge) const;
-    QAuthenticatorPrivate::Method authenticationMethod(bool isProxy) const;
     void clear();
     void clearHttpLayerInformation();