Skip to content

Commit 1806a8e

Browse files
committed
MxcReply: fix file download regression
Also: make Quotest actually test that the downloaded file contents are the same as what has been uploaded.
1 parent aea8126 commit 1806a8e

File tree

2 files changed

+21
-14
lines changed

2 files changed

+21
-14
lines changed

lib/mxcreply.cpp

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,18 +47,21 @@ MxcReply::MxcReply(QNetworkReply* reply, Room* room, const QString &eventId)
4747
d->m_encryptedFile = *efm;
4848
}
4949
}
50-
if (!d->m_encryptedFile)
5150
#endif
52-
d->prepareForReading(this, d->m_reply);
53-
// Above 3 lines: prepare for reading upfront if Quotient is built
54-
// without E2EE or if it's built with E2EE but that specific payload has no
55-
// associated encrypted file metadata
5651
}
5752

5853
void MxcReply::setNetworkReply(QNetworkReply* newReply)
5954
{
6055
d->m_reply = newReply;
6156
d->m_reply->setParent(this);
57+
// Prepare for reading upfront if Quotient is built without E2EE or if it's
58+
// built with E2EE but that specific payload has no associated encrypted
59+
// file metadata
60+
#ifdef Quotient_E2EE_ENABLED
61+
if (!d->m_encryptedFile)
62+
#endif
63+
d->prepareForReading(this, d->m_reply);
64+
6265
connect(d->m_reply, &QNetworkReply::finished, this, [this] {
6366
setError(d->m_reply->error(), d->m_reply->errorString());
6467
#ifdef Quotient_E2EE_ENABLED

quotest/quotest.cpp

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -402,14 +402,16 @@ TEST_IMPL(sendReaction)
402402
return false;
403403
}
404404

405+
static constexpr auto fileContent = "Test";
406+
405407
TEST_IMPL(sendFile)
406408
{
407409
auto* tf = new QTemporaryFile;
408410
if (!tf->open()) {
409411
clog << "Failed to create a temporary file" << endl;
410412
FAIL_TEST();
411413
}
412-
tf->write("Test");
414+
tf->write(fileContent);
413415
tf->close();
414416
QFileInfo tfi { *tf };
415417
// QFileInfo::fileName brings only the file name; QFile::fileName brings
@@ -454,9 +456,11 @@ TEST_IMPL(sendFile)
454456
struct DownloadRunner {
455457
QUrl url;
456458

457-
using result_type = QNetworkReply::NetworkError;
459+
using result_type = std::pair<QNetworkReply::NetworkError, bool>;
460+
461+
static constexpr result_type Success { QNetworkReply::NoError, true };
458462

459-
QNetworkReply::NetworkError operator()(int) const
463+
result_type operator()(int) const
460464
{
461465
QEventLoop el;
462466
QScopedPointer<QNetworkReply, QScopedPointerDeleteLater> reply {
@@ -466,10 +470,10 @@ struct DownloadRunner {
466470
reply.data(), &QNetworkReply::finished, &el, [&el] { el.exit(); },
467471
Qt::QueuedConnection);
468472
el.exec();
469-
return reply->error();
473+
return { reply->error(), reply->readAll() == fileContent };
470474
}
471475

472-
static QVector<QNetworkReply::NetworkError> run(const QUrl& url, int threads)
476+
static QVector<result_type> run(const QUrl& url, int threads)
473477
{
474478
return QtConcurrent::blockingMapped(QVector<int>(threads),
475479
DownloadRunner{ url });
@@ -481,15 +485,15 @@ bool TestSuite::testDownload(const TestToken& thisTest, const QUrl& mxcUrl)
481485
// Testing direct media requests needs explicit allowance
482486
NetworkAccessManager::allowDirectMediaRequests(true);
483487
if (const auto result = DownloadRunner::run(mxcUrl, 1);
484-
result.back() != QNetworkReply::NoError) {
488+
result.back() != DownloadRunner::Success) {
485489
clog << "Direct media request to "
486490
<< mxcUrl.toDisplayString().toStdString()
487491
<< " was allowed but failed" << endl;
488492
FAIL_TEST();
489493
}
490494
NetworkAccessManager::allowDirectMediaRequests(false);
491495
if (const auto result = DownloadRunner::run(mxcUrl, 1);
492-
result.back() == QNetworkReply::NoError) {
496+
result.back() == DownloadRunner::Success) {
493497
clog << "Direct media request to "
494498
<< mxcUrl.toDisplayString().toStdString()
495499
<< " was disallowed but succeeded" << endl;
@@ -500,8 +504,8 @@ bool TestSuite::testDownload(const TestToken& thisTest, const QUrl& mxcUrl)
500504
const auto results = DownloadRunner::run(httpUrl, 3);
501505
// Move out actual test from the multithreaded code to help debugging
502506
FINISH_TEST(std::all_of(results.begin(), results.end(),
503-
[](QNetworkReply::NetworkError ne) {
504-
return ne == QNetworkReply::NoError;
507+
[](DownloadRunner::result_type result) {
508+
return result == DownloadRunner::Success;
505509
}));
506510
}
507511

0 commit comments

Comments
 (0)