From d50bdd6aa9c8557b92609497ff50dac5231dbcf1 Mon Sep 17 00:00:00 2001 From: Liv Date: Mon, 22 May 2017 16:53:41 -0700 Subject: [PATCH 1/6] Initial check that the script url has a supported extension, currently JS only --- libraries/script-engine/src/ScriptEngines.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/libraries/script-engine/src/ScriptEngines.cpp b/libraries/script-engine/src/ScriptEngines.cpp index 2076657288..2e30e614c2 100644 --- a/libraries/script-engine/src/ScriptEngines.cpp +++ b/libraries/script-engine/src/ScriptEngines.cpp @@ -9,6 +9,7 @@ #include "ScriptEngines.h" #include +#include #include @@ -449,6 +450,7 @@ ScriptEngine* ScriptEngines::loadScript(const QUrl& scriptFilename, bool isUserL return result; } QUrl scriptUrl; + if (!scriptFilename.isValid() || (scriptFilename.scheme() != "http" && scriptFilename.scheme() != "https" && @@ -472,8 +474,11 @@ ScriptEngine* ScriptEngines::loadScript(const QUrl& scriptFilename, bool isUserL scriptEngine->deleteLater(); }, Qt::QueuedConnection); + // Check that the script is actually a script + QFileInfo fileInfo(scriptFilename.toString()); + bool hasValidScriptSuffix = (fileInfo.completeSuffix() == "js"); - if (scriptFilename.isEmpty() || !scriptUrl.isValid()) { + if (scriptFilename.isEmpty() || !scriptUrl.isValid() || !hasValidScriptSuffix) { launchScriptEngine(scriptEngine); } else { // connect to the appropriate signals of this script engine From 1fa3e6bda2d392ce62f6db3bce2258c9cd84587f Mon Sep 17 00:00:00 2001 From: Liv Date: Mon, 22 May 2017 17:47:16 -0700 Subject: [PATCH 2/6] Revert "Initial check that the script url has a supported extension, currently JS only" This reverts commit d50bdd6aa9c8557b92609497ff50dac5231dbcf1. --- libraries/script-engine/src/ScriptEngines.cpp | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/libraries/script-engine/src/ScriptEngines.cpp b/libraries/script-engine/src/ScriptEngines.cpp index 2e30e614c2..2076657288 100644 --- a/libraries/script-engine/src/ScriptEngines.cpp +++ b/libraries/script-engine/src/ScriptEngines.cpp @@ -9,7 +9,6 @@ #include "ScriptEngines.h" #include -#include #include @@ -450,7 +449,6 @@ ScriptEngine* ScriptEngines::loadScript(const QUrl& scriptFilename, bool isUserL return result; } QUrl scriptUrl; - if (!scriptFilename.isValid() || (scriptFilename.scheme() != "http" && scriptFilename.scheme() != "https" && @@ -474,11 +472,8 @@ ScriptEngine* ScriptEngines::loadScript(const QUrl& scriptFilename, bool isUserL scriptEngine->deleteLater(); }, Qt::QueuedConnection); - // Check that the script is actually a script - QFileInfo fileInfo(scriptFilename.toString()); - bool hasValidScriptSuffix = (fileInfo.completeSuffix() == "js"); - if (scriptFilename.isEmpty() || !scriptUrl.isValid() || !hasValidScriptSuffix) { + if (scriptFilename.isEmpty() || !scriptUrl.isValid()) { launchScriptEngine(scriptEngine); } else { // connect to the appropriate signals of this script engine From e9fac38bbb19aa997a2fd530c997038613b0923e Mon Sep 17 00:00:00 2001 From: Liv Date: Tue, 23 May 2017 13:08:50 -0700 Subject: [PATCH 3/6] Add function hasValidScriptSuffix and check if script extensions are JavaScript or JSON files --- libraries/script-engine/src/ScriptEngine.cpp | 15 +++++++++++++++ libraries/script-engine/src/ScriptEngine.h | 1 + 2 files changed, 16 insertions(+) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 8bbb9a3e2c..ffd26cb9fb 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -425,6 +425,15 @@ QString ScriptEngine::getFilename() const { return lastPart; } +bool ScriptEngine::hasValidScriptSuffix(const QString& scriptFileName) { + QFileInfo fileInfo(scriptFileName); + QString scriptSuffixToLower = fileInfo.completeSuffix().toLower(); + if (scriptSuffixToLower == "js" || scriptSuffixToLower == "json") { + return true; + } + return false; +} + void ScriptEngine::loadURL(const QUrl& scriptURL, bool reload) { if (_isRunning) { return; @@ -434,6 +443,12 @@ void ScriptEngine::loadURL(const QUrl& scriptURL, bool reload) { _fileNameString = url.toString(); _isReloading = reload; + // Check that script is an actual script + if (!hasValidScriptSuffix(_fileNameString)) { + qCDebug(scriptengine) << "File extension of file: " + _fileNameString + " is not a currently supported script type"; + return; + } + const auto maxRetries = 0; // for consistency with previous scriptCache->getScript() behavior auto scriptCache = DependencyManager::get(); scriptCache->getScriptContents(url.toString(), [this](const QString& url, const QString& scriptContents, bool isURL, bool success, const QString&status) { diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h index bdb14dfe8c..0323ff19e5 100644 --- a/libraries/script-engine/src/ScriptEngine.h +++ b/libraries/script-engine/src/ScriptEngine.h @@ -146,6 +146,7 @@ public: /// to run... NOTE - this is used by Application currently to load the url. We don't really want it to be exposed /// to scripts. we may not need this to be invokable void loadURL(const QUrl& scriptURL, bool reload); + bool hasValidScriptSuffix(const QString& scriptFileName); Q_INVOKABLE QString getContext() const; Q_INVOKABLE bool isClientScript() const { return _context == CLIENT_SCRIPT; } From 3340211455fc504d655edb527e4fedcfb6384671 Mon Sep 17 00:00:00 2001 From: Liv Date: Tue, 23 May 2017 13:54:38 -0700 Subject: [PATCH 4/6] Cleanup and better error handling --- libraries/script-engine/src/ScriptEngine.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index ffd26cb9fb..2b39e47273 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -428,10 +428,7 @@ QString ScriptEngine::getFilename() const { bool ScriptEngine::hasValidScriptSuffix(const QString& scriptFileName) { QFileInfo fileInfo(scriptFileName); QString scriptSuffixToLower = fileInfo.completeSuffix().toLower(); - if (scriptSuffixToLower == "js" || scriptSuffixToLower == "json") { - return true; - } - return false; + return ((scriptSuffixToLower == "js") || (scriptSuffixToLower == "json")); } void ScriptEngine::loadURL(const QUrl& scriptURL, bool reload) { @@ -443,9 +440,10 @@ void ScriptEngine::loadURL(const QUrl& scriptURL, bool reload) { _fileNameString = url.toString(); _isReloading = reload; - // Check that script is an actual script + // Check that script has a supported file extension if (!hasValidScriptSuffix(_fileNameString)) { - qCDebug(scriptengine) << "File extension of file: " + _fileNameString + " is not a currently supported script type"; + scriptErrorMessage("File extension of file: " + _fileNameString + " is not a currently supported script type"); + emit errorLoadingScript(_fileNameString); return; } From 09c7a615993a2701b32157887e70bffd4bf3aa25 Mon Sep 17 00:00:00 2001 From: Liv Date: Tue, 23 May 2017 14:42:07 -0700 Subject: [PATCH 5/6] Fix from string equals to contains to account for working formats not correctly detected with suffix --- libraries/script-engine/src/ScriptEngine.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 2b39e47273..80ded52122 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -428,7 +428,7 @@ QString ScriptEngine::getFilename() const { bool ScriptEngine::hasValidScriptSuffix(const QString& scriptFileName) { QFileInfo fileInfo(scriptFileName); QString scriptSuffixToLower = fileInfo.completeSuffix().toLower(); - return ((scriptSuffixToLower == "js") || (scriptSuffixToLower == "json")); + return scriptSuffixToLower.contains(QString("js"), Qt::CaseInsensitive); } void ScriptEngine::loadURL(const QUrl& scriptURL, bool reload) { From b2bbf21fb004fac6652351a9b7d7bb111fa0dd1f Mon Sep 17 00:00:00 2001 From: Liv Date: Tue, 23 May 2017 15:21:21 -0700 Subject: [PATCH 6/6] style fixes --- libraries/script-engine/src/ScriptEngine.cpp | 14 +++++++------- libraries/script-engine/src/ScriptEngine.h | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 80ded52122..8a53d0300f 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -426,9 +426,9 @@ QString ScriptEngine::getFilename() const { } bool ScriptEngine::hasValidScriptSuffix(const QString& scriptFileName) { - QFileInfo fileInfo(scriptFileName); - QString scriptSuffixToLower = fileInfo.completeSuffix().toLower(); - return scriptSuffixToLower.contains(QString("js"), Qt::CaseInsensitive); + QFileInfo fileInfo(scriptFileName); + QString scriptSuffixToLower = fileInfo.completeSuffix().toLower(); + return scriptSuffixToLower.contains(QString("js"), Qt::CaseInsensitive); } void ScriptEngine::loadURL(const QUrl& scriptURL, bool reload) { @@ -441,10 +441,10 @@ void ScriptEngine::loadURL(const QUrl& scriptURL, bool reload) { _isReloading = reload; // Check that script has a supported file extension - if (!hasValidScriptSuffix(_fileNameString)) { - scriptErrorMessage("File extension of file: " + _fileNameString + " is not a currently supported script type"); - emit errorLoadingScript(_fileNameString); - return; + if (!hasValidScriptSuffix(_fileNameString)) { + scriptErrorMessage("File extension of file: " + _fileNameString + " is not a currently supported script type"); + emit errorLoadingScript(_fileNameString); + return; } const auto maxRetries = 0; // for consistency with previous scriptCache->getScript() behavior diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h index 0323ff19e5..010cdfbc75 100644 --- a/libraries/script-engine/src/ScriptEngine.h +++ b/libraries/script-engine/src/ScriptEngine.h @@ -146,7 +146,7 @@ public: /// to run... NOTE - this is used by Application currently to load the url. We don't really want it to be exposed /// to scripts. we may not need this to be invokable void loadURL(const QUrl& scriptURL, bool reload); - bool hasValidScriptSuffix(const QString& scriptFileName); + bool hasValidScriptSuffix(const QString& scriptFileName); Q_INVOKABLE QString getContext() const; Q_INVOKABLE bool isClientScript() const { return _context == CLIENT_SCRIPT; }