Git-Mirrors/keepassxc
1
0
Fork 0
mirror of https://github.com/keepassxreboot/keepassxc.git synced 2025-01-13 08:19:50 -05:00
Code Issues Packages Projects Releases Wiki Activity

Default scheme to https if none is given. Add test.

Browse Source
This commit is contained in:
Patrick Sean Klein 2023-05-31 23:50:32 +01:00
parent 6a314f848f
commit 728acb77cf
No known key found for this signature in database
GPG Key ID: B6D50F39A56F6906
4 changed files with 104 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
src/core/NetworkRequest.cpp
View File

@ -52,8 +52,24 @@ void NetworkRequest::fetch(const QUrl& url)
{
m_finished = false;
if(url.scheme() == "http") {
if(!m_allowInsecure) {
fail();
emit failure();
return;
}
} else if (url.scheme() != "https") {
fail();
emit failure();
return;
}
QNetworkRequest request(url);
QSslConfiguration sslConfiguration = request.sslConfiguration();
sslConfiguration.setProtocol(QSsl::SecureProtocols);
request.setSslConfiguration(sslConfiguration);
// Set headers
for (const auto& [header, value] : qAsConst(m_headers)) {
request.setRawHeader(header.toUtf8(), value.toUtf8());
@ -78,7 +94,7 @@ void NetworkRequest::fetchFinished()
if (error != QNetworkReply::NoError) {
// Do not emit on abort.
if (error != QNetworkReply::OperationCanceledError) {
emit failure();
fail();
}
return;
}
@ -95,7 +111,7 @@ void NetworkRequest::fetchFinished()
fetch(redirectTarget);
return;
} else {
emit failure();
fail();
return;
}
}
@ -160,15 +176,12 @@ NetworkRequest::NetworkRequest(
, m_redirects(0)
, m_headers(headers)
, m_requested_url(targetURL)
, m_allowInsecure(allowInsecure)
{
connect(&m_timeout, &QTimer::timeout, this, &NetworkRequest::fetchTimeout);
m_timeout.setInterval(timeoutDuration);
m_timeout.setSingleShot(true);
if(!allowInsecure) {
// TODO
}
}
const QString& NetworkRequest::ContentType() const
@ -186,9 +199,7 @@ void NetworkRequest::fetchTimeout()
if(m_finished)
return;
m_finished = true;
// Cancel request on timeout
cancel();
emit failure();
fail();
}
QNetworkReply* NetworkRequest::Reply() const
@ -202,6 +213,12 @@ void NetworkRequest::fetch()
fetch(m_requested_url);
}
void NetworkRequest::fail()
{
cancel();
emit failure();
}
NetworkRequestBuilder::NetworkRequestBuilder()
{
this->setAllowInsecure(false);
@ -250,6 +267,13 @@ NetworkRequestBuilder& NetworkRequestBuilder::setTimeout(std::chrono::millisecon
NetworkRequestBuilder& NetworkRequestBuilder::setUrl(QUrl url)
{
m_url = url;
// Default scheme to https.
if(m_url.scheme().isEmpty()) {
// Necessary to ensure that setScheme adds // (QUrl needs a valid authority section)
// For example, QUrl("example.com").setScheme("https") would result in "https:example.com" (invalid
m_url = QUrl::fromUserInput(m_url.toString(QUrl::FullyEncoded));
m_url.setScheme("https");
}
return *this;
}

2
src/core/NetworkRequest.h
View File

@ -51,6 +51,7 @@ class NetworkRequest : public QObject
unsigned int m_redirects;
QList<QPair<QString, QString>> m_headers;
QUrl m_requested_url;
bool m_allowInsecure;
public:
static constexpr auto UNLIMITED_REDIRECTS = std::numeric_limits<unsigned int>::max();
@ -89,6 +90,7 @@ public:
private:
void reset();
void fetch(const QUrl& url);
void fail();
private slots:
void fetchFinished();
void fetchReadyRead();

73
tests/TestNetworkRequest.cpp
View File

@ -17,6 +17,7 @@ static constexpr auto TIMEOUT_GRACE_MS = 25;
void TestNetworkRequest::testNetworkRequest()
{
QFETCH(QUrl, requestedURL);
QFETCH(QUrl, expectedURL);
QFETCH(QByteArray, expectedContent);
QFETCH(QString, responseContentType);
QFETCH(QString, expectedContentType);
@ -30,7 +31,7 @@ void TestNetworkRequest::testNetworkRequest()
MockNetworkAccess::Manager<QNetworkAccessManager> manager;
auto& reply = manager
.whenGet(requestedURL)
.whenGet(expectedURL)
// Has right user agent?
.has(MockNetworkAccess::Predicates::HeaderMatching(QNetworkRequest::UserAgentHeader,
QRegularExpression(expectedUserAgent)))
@ -62,7 +63,7 @@ void TestNetworkRequest::testNetworkRequest()
// Ensures that predicates match - i.e., the header was set correctly
QCOMPARE(manager.matchedRequests().length(), 1);
QCOMPARE(request.URL(), requestedURL);
QCOMPARE(request.URL(), expectedURL);
if(!expectError) {
QCOMPARE(actualContent, expectedContent);
QCOMPARE(request.ContentType(), expectedContentType);
@ -78,6 +79,7 @@ void TestNetworkRequest::testNetworkRequest()
void TestNetworkRequest::testNetworkRequest_data()
{
QTest::addColumn<QUrl>("requestedURL");
QTest::addColumn<QUrl>("expectedURL");
QTest::addColumn<QByteArray>("expectedContent");
QTest::addColumn<QString>("responseContentType");
QTest::addColumn<QString>("expectedContentType");
@ -88,23 +90,30 @@ void TestNetworkRequest::testNetworkRequest_data()
QString defaultUserAgent("KeePassXC");
//const QUrl& exampleURL = QUrl{"https://example.com"};
const QUrl& exampleURL = QUrl{"https://example.com"};
const QByteArray& exampleContent = QString{"test-content"}.toUtf8();
QTest::newRow("successful request") << exampleURL << exampleContent << "text/plain"
QTest::newRow("successful request") << exampleURL << exampleURL << exampleContent << "text/plain"
<< "text/plain" << ContentTypeParameters_t{}
<< defaultUserAgent << false << QNetworkReply::NetworkError::NoError;
QTest::newRow("content type") << exampleURL << exampleContent << "application/test-content-type"
QTest::newRow("content type") << exampleURL << exampleURL << exampleContent << "application/test-content-type"
<< "application/test-content-type" << ContentTypeParameters_t{}
<< defaultUserAgent << false << QNetworkReply::NetworkError::NoError;
QTest::newRow("empty content type") << exampleURL << QByteArray{} << "" << "" << ContentTypeParameters_t{}
QTest::newRow("empty content type") << exampleURL << exampleURL << QByteArray{} << "" << "" << ContentTypeParameters_t{}
<< defaultUserAgent << false << QNetworkReply::NetworkError::NoError;
QTest::newRow("content type parameters") << exampleURL << exampleContent << "application/test-content-type;test-param=test-value"
QTest::newRow("content type parameters") << exampleURL << exampleURL << exampleContent << "application/test-content-type;test-param=test-value"
<< "application/test-content-type" << ContentTypeParameters_t {{"test-param", "test-value"}}
<< defaultUserAgent << false << QNetworkReply::NetworkError::NoError;
QTest::newRow("content type parameters trimmed") << exampleURL << exampleContent << "application/test-content-type; test-param = test-value"
QTest::newRow("content type parameters trimmed") << exampleURL << exampleURL << exampleContent << "application/test-content-type; test-param = test-value"
<< "application/test-content-type" << ContentTypeParameters_t {{"test-param", "test-value"}}
<< defaultUserAgent << false << QNetworkReply::NetworkError::NoError;
QTest::newRow("request without schema should add https") << QUrl("example.com") << QUrl("https://example.com") << exampleContent << "text/plain"
<< "text/plain" << ContentTypeParameters_t{}
<< defaultUserAgent << false << QNetworkReply::NetworkError::NoError;
QTest::newRow("request without schema should add https (edge case with // but no scheme)") << QUrl("//example.com") << QUrl("https://example.com") << exampleContent << "text/plain"
<< "text/plain" << ContentTypeParameters_t{}
<< defaultUserAgent << false << QNetworkReply::NetworkError::NoError;
}
void TestNetworkRequest::testNetworkRequestTimeout()
@ -312,3 +321,53 @@ void TestNetworkRequest::testNetworkRequestTimeoutWithRedirects()
QCOMPARE(didSucceed, false);
QCOMPARE(didError, true);
}
void TestNetworkRequest::testNetworkRequestSecurityParameter()
{
// Test that requests with allowInsecure() set to false fail when the URL uses an insecure schema
QFETCH(QUrl, targetURL);
QFETCH(bool, allowInsecure);
QFETCH(bool, shouldSucceed);
// Create mock reply
// Create and configure the mocked network access manager
MockNetworkAccess::Manager<QNetworkAccessManager> manager;
QStringList requestedUrls;
auto* reply = &manager.whenGet(targetURL).reply();
reply->withBody(QString{"test-content"}.toUtf8());
// Create request
NetworkRequest request = buildRequest(targetURL).setManager(&manager)
.setAllowInsecure(allowInsecure).build();
bool didSucceed = false, didError = false;
// Check request
QSignalSpy spy(&request, &NetworkRequest::success);
connect(&request, &NetworkRequest::success, [&didSucceed](const QByteArray&) {
didSucceed = true;
});
QSignalSpy errorSpy(&request, &NetworkRequest::failure);
connect(&request, &NetworkRequest::failure, [&didError]() { didError = true; });
request.fetch();
QTest::qWait(300);
QTEST_ASSERT(didError || didSucceed);
QCOMPARE(didSucceed, shouldSucceed);
QCOMPARE(didError, !shouldSucceed);
}
void TestNetworkRequest::testNetworkRequestSecurityParameter_data()
{
QTest::addColumn<QUrl>("targetURL");
QTest::addColumn<bool>("allowInsecure");
QTest::addColumn<bool>("shouldSucceed");
QTest::newRow("secure protocol with allowInsecure=false succeeds") << QUrl("https://example.com") << false << true;
QTest::newRow("secure protocol with allowInsecure=true succeeds") << QUrl("https://example.com") << true << true;
QTest::newRow("insecure protocol with allowInsecure=false fails") << QUrl("http://example.com") << false << false;
QTest::newRow("insecure protocol with allowInsecure=true succeeds") << QUrl("http://example.com") << true << true;
}

3
tests/TestNetworkRequest.h
View File

@ -19,6 +19,9 @@ private slots:
void testNetworkRequestRedirects_data();
void testNetworkRequestTimeoutWithRedirects();
void testNetworkRequestSecurityParameter();
void testNetworkRequestSecurityParameter_data();
};
#endif // KEEPASSXC_TESTNETWORKREQUEST_HPP