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 4f9387f2aee19e38f05cab76a71f0d067b8d80dd)
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
This commit is contained in:
Mårten Nordheim 2024-04-02 11:11:52 +02:00
parent 5be8cf4e9a
commit 8c2518bee8
2 changed files with 102 additions and 10 deletions

View File

@ -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

View File

@ -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");