tst_http2: increase testing for authenticationRequired
Make sure the reply is marked as finished and that the body is received even if we didn't succeed. In a real scenario that would include some text like Access Denied. Also, no longer clear() the authenticationHeader in the server, since that meant the server would not send the header again if the client failed to authenticate. Luckily this wasn't actually causing any problems before, since we only tested the expected www-authenticate header. As a drive-by: clang-tidy complained about not using const-ref for a lambda. Pick-to: 6.6 6.5 6.2 Change-Id: Ia4452fff7d9370d7d460433257d84eff0a6f469b Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
This commit is contained in:
parent
668ce80e9e
commit
f4c8e11ad7
@ -864,7 +864,6 @@ void Http2Server::sendResponse(quint32 streamID, bool emptyBody)
|
|||||||
} else if (!authenticationHeader.isEmpty() && !hasAuth) {
|
} else if (!authenticationHeader.isEmpty() && !hasAuth) {
|
||||||
header.push_back({ ":status", "401" });
|
header.push_back({ ":status", "401" });
|
||||||
header.push_back(HPack::HeaderField("www-authenticate", authenticationHeader));
|
header.push_back(HPack::HeaderField("www-authenticate", authenticationHeader));
|
||||||
authenticationHeader.clear();
|
|
||||||
} else {
|
} else {
|
||||||
header.push_back({":status", "200"});
|
header.push_back({":status", "200"});
|
||||||
}
|
}
|
||||||
|
@ -1083,7 +1083,8 @@ void tst_Http2::authenticationRequired()
|
|||||||
QFETCH(const bool, success);
|
QFETCH(const bool, success);
|
||||||
|
|
||||||
ServerPtr targetServer(newServer(defaultServerSettings, defaultConnectionType()));
|
ServerPtr targetServer(newServer(defaultServerSettings, defaultConnectionType()));
|
||||||
targetServer->setResponseBody("Hello");
|
QByteArray responseBody = "Hello"_ba;
|
||||||
|
targetServer->setResponseBody(responseBody);
|
||||||
targetServer->setAuthenticationHeader("Basic realm=\"Shadow\"");
|
targetServer->setAuthenticationHeader("Basic realm=\"Shadow\"");
|
||||||
|
|
||||||
QMetaObject::invokeMethod(targetServer.data(), "startServer", Qt::QueuedConnection);
|
QMetaObject::invokeMethod(targetServer.data(), "startServer", Qt::QueuedConnection);
|
||||||
@ -1120,16 +1121,22 @@ void tst_Http2::authenticationRequired()
|
|||||||
receivedBody += body;
|
receivedBody += body;
|
||||||
});
|
});
|
||||||
|
|
||||||
if (success)
|
if (success) {
|
||||||
connect(reply.get(), &QNetworkReply::finished, this, &tst_Http2::replyFinished);
|
connect(reply.get(), &QNetworkReply::finished, this, &tst_Http2::replyFinished);
|
||||||
else
|
} else {
|
||||||
connect(reply.get(), &QNetworkReply::errorOccurred, this, &tst_Http2::replyFinishedWithError);
|
// 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,
|
// Since we're using self-signed certificates,
|
||||||
// ignore SSL errors:
|
// ignore SSL errors:
|
||||||
reply->ignoreSslErrors();
|
reply->ignoreSslErrors();
|
||||||
|
|
||||||
runEventLoop();
|
runEventLoop();
|
||||||
STOP_ON_FAILURE
|
STOP_ON_FAILURE
|
||||||
|
QVERIFY2(reply->isFinished(),
|
||||||
|
"The reply should error out if authentication fails, or finish if it succeeds");
|
||||||
|
|
||||||
if (!success)
|
if (!success)
|
||||||
QCOMPARE(reply->error(), QNetworkReply::AuthenticationRequiredError);
|
QCOMPARE(reply->error(), QNetworkReply::AuthenticationRequiredError);
|
||||||
@ -1137,7 +1144,7 @@ void tst_Http2::authenticationRequired()
|
|||||||
|
|
||||||
QVERIFY(authenticationRequested);
|
QVERIFY(authenticationRequested);
|
||||||
|
|
||||||
const auto isAuthenticated = [](QByteArray bv) {
|
const auto isAuthenticated = [](const QByteArray &bv) {
|
||||||
return bv == "Basic YWRtaW46YWRtaW4="; // admin:admin
|
return bv == "Basic YWRtaW46YWRtaW4="; // admin:admin
|
||||||
};
|
};
|
||||||
// Get the "authorization" header out from the server and make sure it's as expected:
|
// Get the "authorization" header out from the server and make sure it's as expected:
|
||||||
@ -1145,6 +1152,16 @@ void tst_Http2::authenticationRequired()
|
|||||||
QCOMPARE(isAuthenticated(reqAuthHeader), success);
|
QCOMPARE(isAuthenticated(reqAuthHeader), success);
|
||||||
if (success)
|
if (success)
|
||||||
QCOMPARE(receivedBody, expectedBody);
|
QCOMPARE(receivedBody, expectedBody);
|
||||||
|
if (responseHEADOnly) {
|
||||||
|
const QVariant contentLenHeader = reply->header(QNetworkRequest::ContentLengthHeader);
|
||||||
|
QVERIFY2(!contentLenHeader.isValid(), "We expect no DATA frames to be received");
|
||||||
|
QCOMPARE(reply->readAll(), QByteArray());
|
||||||
|
} else {
|
||||||
|
const qint32 contentLen = reply->header(QNetworkRequest::ContentLengthHeader).toInt();
|
||||||
|
QCOMPARE(contentLen, responseBody.length());
|
||||||
|
QCOMPARE(reply->bytesAvailable(), responseBody.length());
|
||||||
|
QCOMPARE(reply->readAll(), QByteArray("Hello"));
|
||||||
|
}
|
||||||
// In the `!success` case we need to wait for the server to emit this or it might cause issues
|
// 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
|
// in the next test running after this. In the `success` case we anyway expect it to have been
|
||||||
// received.
|
// received.
|
||||||
|
Loading…
Reference in New Issue
Block a user