Http2: fix handling unsuppported authenticate challenge

When adding/fixing parts earlier it was missed that it was not handling
the _unsupported_ case, when authentication is not handled and there is
no resend. But there _is_ a challenge header.

Pick-to: 6.5
Fixes: QTBUG-123891
Change-Id: I21470df0ce2528bad3babffc6e9f19b7afd29d20
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
(cherry picked from commit 4f9387f)
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>

src/network/access/qhttp2protocolhandler.cpp

@@ -1197,12 +1197,14 @@ void QHttp2ProtocolHandler::handleAuthorization(Stream &stream)
             // In this case IIS will fall back to HTTP/1.1."
             // Though it might be OK to ignore this. The server shouldn't let us connect with
             // HTTP/2 if it doesn't support us using it.
-        } else if (!auth.isEmpty()) {
-            // Somewhat mimics parts of QHttpNetworkConnectionChannel::handleStatus
-            bool resend = false;
-            const bool authenticateHandled = m_connection->d_func()->handleAuthenticateChallenge(
-                    m_socket, httpReply, isProxy, resend);
-            if (authenticateHandled && resend) {
+            return false;
+        }
+        // Somewhat mimics parts of QHttpNetworkConnectionChannel::handleStatus
+        bool resend = false;
+        const bool authenticateHandled = m_connection->d_func()->handleAuthenticateChallenge(
+                m_socket, httpReply, isProxy, resend);
+        if (authenticateHandled) {
+            if (resend) {
                 httpReply->d_func()->eraseData();
                 // Add the request back in queue, we'll retry later now that
                 // we've gotten some username/password set on it:
@@ -1217,11 +1219,15 @@ void QHttp2ProtocolHandler::handleAuthorization(Stream &stream)
                 // We automatically try to send new requests when the stream is
                 // closed, so we don't need to call sendRequest ourselves.
                 return true;
-            } // else: Authentication failed or was cancelled
+            } // else: we're just not resending the request.
+            // @note In the http/1.x case we (at time of writing) call close()
+            // for the connectionChannel (which is a bit weird, we could surely
+            // reuse the open socket outside "connection:close"?), but in http2
+            // we only have one channel, so we won't close anything.
         } else {
-            // No authentication header, but we got a 401/407 so we cannot
-            // succeed. We need to emit signals for headers and data, and then
-            // finishWithError.
+            // No authentication header or authentication isn't supported, but
+            // we got a 401/407 so we cannot succeed. We need to emit signals
+            // for headers and data, and then finishWithError.
             emit httpReply->headerChanged();
             emit httpReply->readyRead();
             QNetworkReply::NetworkError error = httpReply->statusCode() == 401

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

@@ -25,6 +25,7 @@
 #include <QtCore/qobject.h>
 #include <QtCore/qthread.h>
 #include <QtCore/qurl.h>
+#include <QtCore/qset.h>
 
 #include <cstdlib>
 #include <memory>
@@ -93,6 +94,8 @@ private slots:
     void authenticationRequired_data();
     void authenticationRequired();
 
+    void unsupportedAuthenticateChallenge();
+
     void h2cAllowedAttribute_data();
     void h2cAllowedAttribute();
 
@@ -1177,6 +1180,89 @@ void tst_Http2::authenticationRequired()
     QTRY_VERIFY(serverGotSettingsACK);
 }
 
+void tst_Http2::unsupportedAuthenticateChallenge()
+{
+    clearHTTP2State();
+    serverPort = 0;
+
+    if (defaultConnectionType() == H2Type::h2c)
+        QSKIP("This test requires TLS with ALPN to work");
+
+    ServerPtr targetServer(newServer(defaultServerSettings, defaultConnectionType()));
+    QByteArray responseBody = "Hello"_ba;
+    targetServer->setResponseBody(responseBody);
+    targetServer->setAuthenticationHeader("Bearer realm=\"qt.io accounts\"");
+
+    QMetaObject::invokeMethod(targetServer.data(), "startServer", Qt::QueuedConnection);
+    runEventLoop();
+
+    QVERIFY(serverPort != 0);
+
+    nRequests = 1;
+
+    QUrl url = requestUrl(defaultConnectionType());
+    url.setPath("/index.html");
+    QNetworkRequest request(url);
+
+    QByteArray expectedBody = "Hello, World!";
+    request.setHeader(QNetworkRequest::ContentTypeHeader, "application/x-www-form-urlencoded");
+    QScopedPointer<QNetworkReply> reply;
+    reply.reset(manager->post(request, expectedBody));
+
+    bool authenticationRequested = false;
+    connect(manager.get(), &QNetworkAccessManager::authenticationRequired, reply.get(),
+            [&](QNetworkReply *, QAuthenticator *auth) {
+                authenticationRequested = true;
+            });
+
+    bool finishedReceived = false;
+    connect(reply.get(), &QNetworkReply::finished, reply.get(),
+            [&]() { finishedReceived = true; });
+    bool errorReceived = false;
+    connect(reply.get(), &QNetworkReply::errorOccurred, reply.get(),
+            [&]() { errorReceived = true; });
+
+    QSet<quint32> receivedDataOnStreams;
+    connect(targetServer.get(), &Http2Server::receivedDATAFrame, reply.get(),
+            [&receivedDataOnStreams](quint32 streamID, const QByteArray &body) {
+                Q_UNUSED(body);
+                receivedDataOnStreams.insert(streamID);
+            });
+
+    // Use queued connection so that the finished signal can be emitted and the
+    // isFinished property can be set.
+    connect(reply.get(), &QNetworkReply::errorOccurred, this,
+            &tst_Http2::replyFinishedWithError, Qt::QueuedConnection);
+
+    // Since we're using self-signed certificates, ignore SSL errors:
+    reply->ignoreSslErrors();
+
+    runEventLoop();
+    STOP_ON_FAILURE
+    QVERIFY2(reply->isFinished(),
+             "The reply should error out if authentication fails, or finish if it succeeds");
+
+    QCOMPARE(reply->error(), QNetworkReply::AuthenticationRequiredError);
+    QVERIFY(reply->isFinished());
+    QVERIFY(errorReceived);
+    QVERIFY(finishedReceived);
+    QCOMPARE(receivedDataOnStreams.size(), 1);
+    QVERIFY(receivedDataOnStreams.contains(1)); // the original, failed, request
+
+    QVERIFY(!authenticationRequested);
+
+    // We should not have sent any authentication headers to the server, since
+    // we don't support the challenge.
+    const QByteArray reqAuthHeader = targetServer->requestAuthorizationHeader();
+    QVERIFY(reqAuthHeader.isEmpty());
+
+    // In the `!success` case we need to wait for the server to emit this or it might cause issues
+    // in the next test running after this. In the `success` case we anyway expect it to have been
+    // received.
+    QTRY_VERIFY(serverGotSettingsACK);
+
+}
+
 void tst_Http2::h2cAllowedAttribute_data()
 {
     QTest::addColumn<bool>("h2cAllowed");