Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crash. Remove unnecessary dependency injection causing crash. #6178

Merged
merged 1 commit into from
Nov 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/common/checksumcalculator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ static QCryptographicHash::Algorithm algorithmTypeToQCryptoHashAlgorithm(Checksu
return static_cast<QCryptographicHash::Algorithm>(-1);
}

ChecksumCalculator::ChecksumCalculator(QSharedPointer<QIODevice> sharedDevice, const QByteArray &checksumTypeName)
: _device(sharedDevice)
ChecksumCalculator::ChecksumCalculator(const QString &filePath, const QByteArray &checksumTypeName)
: _device(new QFile(filePath))
{
if (checksumTypeName == checkSumMD5C) {
_algorithmType = AlgorithmType::MD5;
Expand Down
7 changes: 3 additions & 4 deletions src/common/checksumcalculator.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@
#include <QByteArray>
#include <QFutureWatcher>
#include <QMutex>

#include <memory>
#include <QScopedPointer>

class QCryptographicHash;

Expand All @@ -42,14 +41,14 @@ class OCSYNC_EXPORT ChecksumCalculator
Adler32,
};

ChecksumCalculator(QSharedPointer<QIODevice> sharedDevice, const QByteArray &checksumTypeName);
ChecksumCalculator(const QString &filePath, const QByteArray &checksumTypeName);
~ChecksumCalculator();
[[nodiscard]] QByteArray calculate();

private:
void initChecksumAlgorithm();
bool addChunk(const QByteArray &chunk, const qint64 size);
QSharedPointer<QIODevice> _device;
QScopedPointer<QIODevice> _device;
QScopedPointer<QCryptographicHash> _cryptographicHash;
unsigned int _adlerHash = 0;
bool _isInitialized = false;
Expand Down
28 changes: 6 additions & 22 deletions src/common/checksums.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -187,43 +187,34 @@ QByteArray ComputeChecksum::checksumType() const
void ComputeChecksum::start(const QString &filePath)
{
qCInfo(lcChecksums) << "Computing" << checksumType() << "checksum of" << filePath << "in a thread";
startImpl(QSharedPointer<QFile>::create(filePath));
startImpl(filePath);
}

void ComputeChecksum::start(QSharedPointer<QIODevice> device)
{
ENFORCE(device);
qCInfo(lcChecksums) << "Computing" << checksumType() << "checksum of device" << device.get() << "in a thread";
ASSERT(!device->parent());

startImpl(device);
}

void ComputeChecksum::startImpl(QSharedPointer<QIODevice> device)
void ComputeChecksum::startImpl(const QString &filePath)
{
connect(&_watcher, &QFutureWatcherBase::finished,
this, &ComputeChecksum::slotCalculationDone,
Qt::UniqueConnection);

_checksumCalculator.reset(new ChecksumCalculator(device, _checksumType));
_checksumCalculator.reset(new ChecksumCalculator(filePath, _checksumType));
_watcher.setFuture(QtConcurrent::run([this]() {
return _checksumCalculator->calculate();
}));
}

QByteArray ComputeChecksum::computeNowOnFile(const QString &filePath, const QByteArray &checksumType)
{
return computeNow(QSharedPointer<QFile>::create(filePath), checksumType);
return computeNow(filePath, checksumType);
}

QByteArray ComputeChecksum::computeNow(QSharedPointer<QIODevice> device, const QByteArray &checksumType)
QByteArray ComputeChecksum::computeNow(const QString &filePath, const QByteArray &checksumType)
{
if (!checksumComputationEnabled()) {
qCWarning(lcChecksums) << "Checksum computation disabled by environment variable";
return QByteArray();
}

ChecksumCalculator checksumCalculator(device, checksumType);
ChecksumCalculator checksumCalculator(filePath, checksumType);
return checksumCalculator.calculate();
}

Expand Down Expand Up @@ -270,13 +261,6 @@ void ValidateChecksumHeader::start(const QString &filePath, const QByteArray &ch
calculator->start(filePath);
}

void ValidateChecksumHeader::start(QSharedPointer<QIODevice> device, const QByteArray &checksumHeader)
{
if (auto calculator = prepareStart(checksumHeader)) {
calculator->start(device);
}
}

QByteArray ValidateChecksumHeader::calculatedChecksumType() const
{
return _calculatedChecksumType;
Expand Down
24 changes: 2 additions & 22 deletions src/common/checksums.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,20 +81,10 @@ class OCSYNC_EXPORT ComputeChecksum : public QObject
*/
void start(const QString &filePath);

/**
* Computes the checksum for the given device.
*
* done() is emitted when the calculation finishes.
*
* The device ownership transfers into the thread that
* will compute the checksum. It must not have a parent.
*/
void start(QSharedPointer<QIODevice> device);

/**
* Computes the checksum synchronously.
*/
static QByteArray computeNow(QSharedPointer<QIODevice> device, const QByteArray &checksumType);
static QByteArray computeNow(const QString &filePath, const QByteArray &checksumType);

/**
* Computes the checksum synchronously on file. Convenience wrapper for computeNow().
Expand All @@ -108,7 +98,7 @@ private slots:
void slotCalculationDone();

private:
void startImpl(QSharedPointer<QIODevice> device);
void startImpl(const QString &filePath);

QByteArray _checksumType;

Expand Down Expand Up @@ -145,16 +135,6 @@ class OCSYNC_EXPORT ValidateChecksumHeader : public QObject
*/
void start(const QString &filePath, const QByteArray &checksumHeader);

/**
* Check a device's actual checksum against the provided checksumHeader
*
* Like the other start() but works on an device.
*
* The device ownership transfers into the thread that
* will compute the checksum. It must not have a parent.
*/
void start(QSharedPointer<QIODevice> device, const QByteArray &checksumHeader);

[[nodiscard]] QByteArray calculatedChecksumType() const;
[[nodiscard]] QByteArray calculatedChecksum() const;

Expand Down
22 changes: 6 additions & 16 deletions test/testchecksumvalidator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,7 @@ using namespace OCC::Utility;
QFileInfo fi(file);
QVERIFY(fi.exists());

auto sharedFile(QSharedPointer<QFile>::create(file));
ChecksumCalculator checksumCalculator(sharedFile, OCC::checkSumMD5C);
ChecksumCalculator checksumCalculator(file, OCC::checkSumMD5C);

const auto sum = checksumCalculator.calculate();

Expand All @@ -106,8 +105,7 @@ using namespace OCC::Utility;
QFileInfo fi(file);
QVERIFY(fi.exists());

auto sharedFile(QSharedPointer<QFile>::create(file));
ChecksumCalculator checksumCalculator(sharedFile, OCC::checkSumSHA1C);
ChecksumCalculator checksumCalculator(file, OCC::checkSumSHA1C);

const auto sum = checksumCalculator.calculate();

Expand All @@ -129,8 +127,7 @@ using namespace OCC::Utility;

connect(vali, &ComputeChecksum::done, this, &TestChecksumValidator::slotUpValidated);

auto sharedFile(QSharedPointer<QFile>::create(_testfile));
ChecksumCalculator checksumCalculator(sharedFile, OCC::checkSumAdlerC);
ChecksumCalculator checksumCalculator(_testfile, OCC::checkSumAdlerC);

_expected = checksumCalculator.calculate();

Expand All @@ -152,8 +149,7 @@ using namespace OCC::Utility;
vali->setChecksumType(_expectedType);
connect(vali, &ComputeChecksum::done, this, &TestChecksumValidator::slotUpValidated);

auto sharedFile(QSharedPointer<QFile>::create(_testfile));
ChecksumCalculator checksumCalculator(sharedFile, OCC::checkSumMD5C);
ChecksumCalculator checksumCalculator(_testfile, OCC::checkSumMD5C);

_expected = checksumCalculator.calculate();
vali->start(_testfile);
Expand All @@ -172,8 +168,7 @@ using namespace OCC::Utility;
vali->setChecksumType(_expectedType);
connect(vali, &ComputeChecksum::done, this, &TestChecksumValidator::slotUpValidated);

auto sharedFile(QSharedPointer<QFile>::create(_testfile));
ChecksumCalculator checksumCalculator(sharedFile, OCC::checkSumSHA1C);
ChecksumCalculator checksumCalculator(_testfile, OCC::checkSumSHA1C);
_expected = checksumCalculator.calculate();

vali->start(_testfile);
Expand All @@ -193,16 +188,13 @@ using namespace OCC::Utility;
connect(vali, &ValidateChecksumHeader::validated, this, &TestChecksumValidator::slotDownValidated);
connect(vali, &ValidateChecksumHeader::validationFailed, this, &TestChecksumValidator::slotDownError);

auto sharedFile(QSharedPointer<QFile>::create(_testfile));
ChecksumCalculator checksumCalculator(sharedFile, OCC::checkSumAdlerC);
ChecksumCalculator checksumCalculator(_testfile, OCC::checkSumAdlerC);
_expected = checksumCalculator.calculate();

QByteArray adler = checkSumAdlerC;
adler.append(":");
adler.append(_expected);

sharedFile->open(QIODevice::ReadOnly);
sharedFile->seek(0);
_successDown = false;
vali->start(_testfile, adler);

Expand All @@ -211,14 +203,12 @@ using namespace OCC::Utility;
_expectedError = QStringLiteral("The downloaded file does not match the checksum, it will be resumed. \"543345\" != \"%1\"").arg(QString::fromUtf8(_expected));
_expectedFailureReason = ValidateChecksumHeader::FailureReason::ChecksumMismatch;
_errorSeen = false;
sharedFile->seek(0);
vali->start(_testfile, "Adler32:543345");
QTRY_VERIFY(_errorSeen);

_expectedError = QLatin1String("The checksum header contained an unknown checksum type \"Klaas32\"");
_expectedFailureReason = ValidateChecksumHeader::FailureReason::ChecksumTypeUnknown;
_errorSeen = false;
sharedFile->seek(0);
vali->start(_testfile, "Klaas32:543345");
QTRY_VERIFY(_errorSeen);

Expand Down