From ae3c0f6646d3623db698651ff15b9d3cf21a7619 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 9 Mar 2016 18:01:10 -0800 Subject: [PATCH] add hash and path checking --- assignment-client/src/assets/AssetServer.cpp | 18 +++++++++++------- .../networking/src/AssetResourceRequest.cpp | 18 +++++++++--------- .../networking/src/AssetResourceRequest.h | 2 +- libraries/networking/src/AssetUtils.cpp | 10 ++++++++++ libraries/networking/src/AssetUtils.h | 6 ++++++ 5 files changed, 37 insertions(+), 17 deletions(-) diff --git a/assignment-client/src/assets/AssetServer.cpp b/assignment-client/src/assets/AssetServer.cpp index e094b0d749..428920a44c 100644 --- a/assignment-client/src/assets/AssetServer.cpp +++ b/assignment-client/src/assets/AssetServer.cpp @@ -117,7 +117,7 @@ void AssetServer::completeSetup() { // Check the asset directory to output some information about what we have auto files = _filesDirectory.entryList(QDir::Files); - QRegExp hashFileRegex { "^[a-f0-9]{" + QString::number(SHA256_HASH_HEX_LENGTH) + "}$" }; + QRegExp hashFileRegex { ASSET_HASH_REGEX_STRING }; auto hashedFiles = files.filter(hashFileRegex); qInfo() << "There are" << hashedFiles.size() << "asset files in the asset directory."; @@ -459,14 +459,13 @@ void AssetServer::loadMappingsFromFile() { while (it != _fileMappings.end()) { bool shouldDrop = false; - if (it.key()[0] != '/') { - qWarning() << "Will not keep mapping for" << it.key() << "since it does not have a leading forward slash."; + if (!isValidPath(it.key())) { + qWarning() << "Will not keep mapping for" << it.key() << "since it is not a valid path."; shouldDrop = true; } - QRegExp hashFileRegex { "^[A-Fa-f0-9]{" + QString::number(SHA256_HASH_HEX_LENGTH) + "}$" }; - if (!hashFileRegex.exactMatch(it.value().toString())) { + if (!isValidHash(it.value().toString())) { qWarning() << "Will not keep mapping for" << it.key() << "since it does not have a valid hash."; shouldDrop = true; } @@ -513,8 +512,13 @@ bool AssetServer::writeMappingsToFile() { bool AssetServer::setMapping(const AssetPath& path, const AssetHash& hash) { - if (path[0] != '/') { - qWarning() << "Cannot set a mapping that does not have a leading forward slash:" << path; + if (!isValidPath(path)) { + qWarning() << "Cannot set a mapping for invalid path:" << path << "=>" << hash; + return false; + } + + if (!isValidHash(hash)) { + qWarning() << "Cannot set a mapping for invalid hash" << path << "=>" << hash; return false; } diff --git a/libraries/networking/src/AssetResourceRequest.cpp b/libraries/networking/src/AssetResourceRequest.cpp index a588bfb0d8..3cc7bdd4c7 100644 --- a/libraries/networking/src/AssetResourceRequest.cpp +++ b/libraries/networking/src/AssetResourceRequest.cpp @@ -25,11 +25,11 @@ AssetResourceRequest::~AssetResourceRequest() { } } -bool AssetResourceRequest::urlIsAssetPath() const { +bool AssetResourceRequest::urlIsAssetHash() const { static const QString ATP_HASH_REGEX_STRING = "^atp:([A-Fa-f0-9]{64})(\\.[\\w]+)?$"; QRegExp hashRegex { ATP_HASH_REGEX_STRING }; - return !hashRegex.exactMatch(_url.toString()); + return hashRegex.exactMatch(_url.toString()); } void AssetResourceRequest::doSend() { @@ -39,18 +39,18 @@ void AssetResourceRequest::doSend() { // We'll either have a hash or an ATP path to a file (that maps to a hash) - if (urlIsAssetPath()) { - // This is an ATP path, we'll need to figure out what the mapping is. - // This may incur a roundtrip to the asset-server, or it may return immediately from the cache in AssetClient. - - auto path = _url.path(); - requestMappingForPath(path); - } else { + if (urlIsAssetHash()) { // We've detected that this is a hash - simply use AssetClient to request that asset auto parts = _url.path().split(".", QString::SkipEmptyParts); auto hash = parts.length() > 0 ? parts[0] : ""; requestHash(hash); + } else { + // This is an ATP path, we'll need to figure out what the mapping is. + // This may incur a roundtrip to the asset-server, or it may return immediately from the cache in AssetClient. + + auto path = _url.path(); + requestMappingForPath(path); } } diff --git a/libraries/networking/src/AssetResourceRequest.h b/libraries/networking/src/AssetResourceRequest.h index 2fb8cfd3df..f517c57030 100644 --- a/libraries/networking/src/AssetResourceRequest.h +++ b/libraries/networking/src/AssetResourceRequest.h @@ -30,7 +30,7 @@ private slots: void onDownloadProgress(qint64 bytesReceived, qint64 bytesTotal); private: - bool urlIsAssetPath() const; + bool urlIsAssetHash() const; void requestMappingForPath(const AssetPath& path); void requestHash(const AssetHash& hash); diff --git a/libraries/networking/src/AssetUtils.cpp b/libraries/networking/src/AssetUtils.cpp index cf1bdbe8f2..e25f357fcc 100644 --- a/libraries/networking/src/AssetUtils.cpp +++ b/libraries/networking/src/AssetUtils.cpp @@ -63,3 +63,13 @@ bool saveToCache(const QUrl& url, const QByteArray& file) { } return false; } + +bool isValidPath(const AssetPath& path) { + QRegExp pathRegex { ASSET_PATH_REGEX_STRING }; + return pathRegex.exactMatch(path); +} + +bool isValidHash(const AssetHash& hash) { + QRegExp hashRegex { ASSET_HASH_REGEX_STRING }; + return hashRegex.exactMatch(hash); +} diff --git a/libraries/networking/src/AssetUtils.h b/libraries/networking/src/AssetUtils.h index 2495326b67..52ed7336c2 100644 --- a/libraries/networking/src/AssetUtils.h +++ b/libraries/networking/src/AssetUtils.h @@ -31,6 +31,9 @@ const size_t SHA256_HASH_LENGTH = 32; const size_t SHA256_HASH_HEX_LENGTH = 64; const uint64_t MAX_UPLOAD_SIZE = 1000 * 1000 * 1000; // 1GB +const QString ASSET_PATH_REGEX_STRING = "^\\/(?:[^\\/]|\\/(?!\\/))*$"; +const QString ASSET_HASH_REGEX_STRING = QString("^[a-fA-F0-9]{%1}$").arg(SHA256_HASH_HEX_LENGTH); + enum AssetServerError : uint8_t { NoError = 0, AssetNotFound, @@ -55,4 +58,7 @@ QByteArray hashData(const QByteArray& data); QByteArray loadFromCache(const QUrl& url); bool saveToCache(const QUrl& url, const QByteArray& file); +bool isValidPath(const AssetPath& path); +bool isValidHash(const QString& hashString); + #endif