From 973fd80c1dee08d68f92654ae3cc1f5dc9abd356 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 10 Sep 2015 15:24:37 -0700 Subject: [PATCH] fail asset request if hash does not match --- interface/src/ui/AssetUploadDialogFactory.cpp | 6 +-- libraries/networking/src/AssetClient.h | 6 +-- libraries/networking/src/AssetRequest.cpp | 44 ++++++++++++++----- libraries/networking/src/AssetRequest.h | 14 ++++-- .../networking/src/AssetResourceRequest.cpp | 4 +- libraries/networking/src/AssetUpload.cpp | 10 ++--- libraries/networking/src/AssetUpload.h | 10 ++--- 7 files changed, 63 insertions(+), 31 deletions(-) diff --git a/interface/src/ui/AssetUploadDialogFactory.cpp b/interface/src/ui/AssetUploadDialogFactory.cpp index 7d87c17b16..2ffbb298d0 100644 --- a/interface/src/ui/AssetUploadDialogFactory.cpp +++ b/interface/src/ui/AssetUploadDialogFactory.cpp @@ -69,7 +69,7 @@ void AssetUploadDialogFactory::showDialog() { } void AssetUploadDialogFactory::handleUploadFinished(AssetUpload* upload, const QString& hash) { - if (upload->getResult() == AssetUpload::Success) { + if (upload->getError() == AssetUpload::NoError) { // show message box for successful upload, with copiable text for ATP hash QDialog* hashCopyDialog = new QDialog(_dialogParent); @@ -124,14 +124,14 @@ void AssetUploadDialogFactory::handleUploadFinished(AssetUpload* upload, const Q // figure out the right error message for the message box QString additionalError; - switch (upload->getResult()) { + switch (upload->getError()) { case AssetUpload::PermissionDenied: additionalError = PERMISSION_DENIED_ERROR; break; case AssetUpload::TooLarge: additionalError = "The uploaded content was too large and could not be stored in the asset-server."; break; - case AssetUpload::ErrorLoadingFile: + case AssetUpload::FileOpenError: additionalError = "The file could not be opened. Please check your permissions and try again."; break; default: diff --git a/libraries/networking/src/AssetClient.h b/libraries/networking/src/AssetClient.h index 7fa1771873..d2f0bcddf6 100644 --- a/libraries/networking/src/AssetClient.h +++ b/libraries/networking/src/AssetClient.h @@ -29,9 +29,9 @@ struct AssetInfo { int64_t size; }; -using ReceivedAssetCallback = std::function; -using GetInfoCallback = std::function; -using UploadResultCallback = std::function; +using ReceivedAssetCallback = std::function; +using GetInfoCallback = std::function; +using UploadResultCallback = std::function; class AssetClient : public QObject, public Dependency { Q_OBJECT diff --git a/libraries/networking/src/AssetRequest.cpp b/libraries/networking/src/AssetRequest.cpp index f989f10ae8..b2442cb33d 100644 --- a/libraries/networking/src/AssetRequest.cpp +++ b/libraries/networking/src/AssetRequest.cpp @@ -41,14 +41,17 @@ void AssetRequest::start() { _state = WAITING_FOR_INFO; auto assetClient = DependencyManager::get(); - assetClient->getAssetInfo(_hash, _extension, [this](AssetServerError error, AssetInfo info) { + assetClient->getAssetInfo(_hash, _extension, [this](AssetServerError serverError, AssetInfo info) { _info = info; - _error = error; - if (_error != NoError) { + if (serverError != AssetServerError::NoError) { qCDebug(networking) << "Got error retrieving asset info for" << _hash; + _state = FINISHED; emit finished(this); + + _error = (serverError == AssetServerError::AssetNotFound) ? NotFound : UnknownError; + return; } @@ -60,17 +63,38 @@ void AssetRequest::start() { int start = 0, end = _info.size; auto assetClient = DependencyManager::get(); - assetClient->getAsset(_hash, _extension, start, end, [this, start, end](AssetServerError error, + assetClient->getAsset(_hash, _extension, start, end, [this, start, end](AssetServerError serverError, const QByteArray& data) { Q_ASSERT(data.size() == (end - start)); - _error = error; - if (_error == NoError) { - memcpy(_data.data() + start, data.constData(), data.size()); - _totalReceived += data.size(); - emit progress(_totalReceived, _info.size); + if (serverError == AssetServerError::NoError) { + + // we need to check the hash of the received data to make sure it matches what we expect + if (hashData(data).toHex() == _hash) { + memcpy(_data.data() + start, data.constData(), data.size()); + _totalReceived += data.size(); + emit progress(_totalReceived, _info.size); + } else { + // hash doesn't match - we have an error + _error = HashVerificationFailed; + } + } else { - qCDebug(networking) << "Got error retrieving asset" << _hash; + switch (serverError) { + case AssetServerError::AssetNotFound: + _error = NotFound; + break; + case AssetServerError::InvalidByteRange: + _error = InvalidByteRange; + break; + default: + _error = UnknownError; + break; + } + } + + if (_error != NoError) { + qCDebug(networking) << "Got error retrieving asset" << _hash << "- error code" << _error; } _state = FINISHED; diff --git a/libraries/networking/src/AssetRequest.h b/libraries/networking/src/AssetRequest.h index b33954d34e..ba3863ffdd 100644 --- a/libraries/networking/src/AssetRequest.h +++ b/libraries/networking/src/AssetRequest.h @@ -29,14 +29,22 @@ public: WAITING_FOR_DATA, FINISHED }; + + enum Error { + NoError, + NotFound, + InvalidByteRange, + HashVerificationFailed, + UnknownError + }; AssetRequest(QObject* parent, const QString& hash, const QString& extension); Q_INVOKABLE void start(); const QByteArray& getData() const { return _data; } - State getState() const { return _state; } - AssetServerError getError() const { return _error; } + const State& getState() const { return _state; } + const Error& getError() const { return _error; } signals: void finished(AssetRequest* thisRequest); @@ -44,7 +52,7 @@ signals: private: State _state = NOT_STARTED; - AssetServerError _error; + Error _error = NoError; AssetInfo _info; uint64_t _totalReceived { 0 }; QString _hash; diff --git a/libraries/networking/src/AssetResourceRequest.cpp b/libraries/networking/src/AssetResourceRequest.cpp index b9adba99b7..df5cb6464b 100644 --- a/libraries/networking/src/AssetResourceRequest.cpp +++ b/libraries/networking/src/AssetResourceRequest.cpp @@ -48,11 +48,11 @@ void AssetResourceRequest::doSend() { Q_ASSERT(req->getState() == AssetRequest::FINISHED); switch (req->getError()) { - case NoError: + case AssetRequest::Error::NoError: _data = req->getData(); _result = Success; break; - case AssetNotFound: + case AssetRequest::Error::NotFound: _result = NotFound; break; default: diff --git a/libraries/networking/src/AssetUpload.cpp b/libraries/networking/src/AssetUpload.cpp index 03c8707517..f51cb4a7b3 100644 --- a/libraries/networking/src/AssetUpload.cpp +++ b/libraries/networking/src/AssetUpload.cpp @@ -46,23 +46,23 @@ void AssetUpload::start() { assetClient->uploadAsset(data, _extension, [this](AssetServerError error, const QString& hash){ switch (error) { case AssetServerError::NoError: - _result = Success; + _error = NoError; break; case AssetServerError::AssetTooLarge: - _result = TooLarge; + _error = TooLarge; break; case AssetServerError::PermissionDenied: - _result = PermissionDenied; + _error = PermissionDenied; break; default: - _result = ErrorLoadingFile; + _error = FileOpenError; break; } emit finished(this, hash); }); } else { // we couldn't open the file - set the error result - _result = ErrorLoadingFile; + _error = FileOpenError; // emit that we are done emit finished(this, QString()); diff --git a/libraries/networking/src/AssetUpload.h b/libraries/networking/src/AssetUpload.h index 0fef814a07..2dfada06f2 100644 --- a/libraries/networking/src/AssetUpload.h +++ b/libraries/networking/src/AssetUpload.h @@ -26,12 +26,12 @@ class AssetUpload : public QObject { Q_OBJECT public: - enum Result { - Success = 0, + enum Error { + NoError = 0, Timeout, TooLarge, PermissionDenied, - ErrorLoadingFile + FileOpenError }; AssetUpload(QObject* parent, const QString& filename); @@ -40,7 +40,7 @@ public: const QString& getFilename() const { return _filename; } const QString& getExtension() const { return _extension; } - const Result& getResult() const { return _result; } + const Error& getError() const { return _error; } signals: void finished(AssetUpload* upload, const QString& hash); @@ -49,7 +49,7 @@ signals: private: QString _filename; QString _extension; - Result _result; + Error _error; }; #endif // hifi_AssetUpload_h