From 5f6950757309b123ad46de06cf5d9282973d37c6 Mon Sep 17 00:00:00 2001 From: Andre Hartmann Date: Wed, 31 Jan 2018 20:48:21 +0100 Subject: [PATCH] JavaScriptFilter: Use QScriptEngine to avoid endless loops QJSEngine does not provide a means to interrupt the evaluation when then engine stucks in an endless loop. The (deprecated) QScriptEngine does allow this, so use this for now. As QBS also needs QScriptEngine, no extra dependency is created. For Qt installations without QtScript, the JavaScriptFilter is not compiled. Task-number: QTCREATORBUG-19690 Change-Id: I172b7e7042eea0d40a765c836b2d1c57b6a529e6 Reviewed-by: hjk Reviewed-by: Eike Ziller Reviewed-by: Orgad Shaneh --- src/plugins/coreplugin/coreplugin.qbs | 19 ++++++++++-- .../coreplugin/locator/javascriptfilter.cpp | 30 ++++++++++++++----- .../coreplugin/locator/javascriptfilter.h | 4 +-- src/plugins/coreplugin/locator/locator.cpp | 2 ++ src/plugins/coreplugin/locator/locator.pri | 16 ++++++++-- 5 files changed, 56 insertions(+), 15 deletions(-) diff --git a/src/plugins/coreplugin/coreplugin.qbs b/src/plugins/coreplugin/coreplugin.qbs index ce47658daad..9425c4ad925 100644 --- a/src/plugins/coreplugin/coreplugin.qbs +++ b/src/plugins/coreplugin/coreplugin.qbs @@ -17,11 +17,18 @@ Project { condition: qbs.targetOS.contains("windows") } + Depends { name: "Qt.script"; required: false } + Depends { name: "Utils" } Depends { name: "Aggregation" } Depends { name: "app_version_header" } + Properties { + condition: Qt.script.present + cpp.defines: base.concat("WITH_JAVASCRIPTFILTER") + } + cpp.dynamicLibraries: { if (qbs.targetOS.contains("windows")) return ["ole32", "user32"] @@ -343,8 +350,6 @@ Project { "filesystemfilter.ui", "ilocatorfilter.cpp", "ilocatorfilter.h", - "javascriptfilter.cpp", - "javascriptfilter.h", "locatorconstants.h", "locatorfiltersfilter.cpp", "locatorfiltersfilter.h", @@ -364,6 +369,16 @@ Project { ] } + Group { + name: "Locator Javascript Filter" + condition: Qt.script.present + prefix: "locator/" + files: [ + "javascriptfilter.cpp", + "javascriptfilter.h", + ] + } + Group { name: "Locator_mac" condition: qbs.targetOS.contains("macos") diff --git a/src/plugins/coreplugin/locator/javascriptfilter.cpp b/src/plugins/coreplugin/locator/javascriptfilter.cpp index 6255101dca0..a4c20e7a7e3 100644 --- a/src/plugins/coreplugin/locator/javascriptfilter.cpp +++ b/src/plugins/coreplugin/locator/javascriptfilter.cpp @@ -27,14 +27,16 @@ #include #include -#include +#include +#include namespace Core { namespace Internal { enum JavaScriptAction { - ResetEngine = QVariant::UserType + 1 + ResetEngine = QVariant::UserType + 1, + AbortEngine }; JavaScriptFilter::JavaScriptFilter() @@ -66,12 +68,24 @@ QList JavaScriptFilter::matchesFor( if (entry.trimmed().isEmpty()) { entries.append({this, tr("Reset Engine"), QVariant(ResetEngine, nullptr)}); } else { - const QString result = m_engine->evaluate(entry).toString(); - const QString expression = entry + " = " + result; + bool aborted = false; - entries.append({this, expression, QVariant()}); - entries.append({this, tr("Copy to clipboard: %1").arg(result), result}); - entries.append({this, tr("Copy to clipboard: %1").arg(expression), expression}); + QTimer::singleShot(1000, this, [this, &aborted]() { + m_engine->abortEvaluation(); + aborted = true; + }); + + const QString result = m_engine->evaluate(entry).toString(); + + if (aborted) { + const QString message = entry + " = " + tr("Engine aborted after timeout."); + entries.append({this, message, QVariant(AbortEngine, nullptr)}); + } else { + const QString expression = entry + " = " + result; + entries.append({this, expression, QVariant()}); + entries.append({this, tr("Copy to clipboard: %1").arg(result), result}); + entries.append({this, tr("Copy to clipboard: %1").arg(expression), expression}); + } } return entries; @@ -104,7 +118,7 @@ void JavaScriptFilter::refresh(QFutureInterface &future) void JavaScriptFilter::setupEngine() { - m_engine.reset(new QJSEngine); + m_engine.reset(new QScriptEngine); m_engine->evaluate( "function abs(x) { return Math.abs(x); }\n" "function acos(x) { return Math.acos(x); }\n" diff --git a/src/plugins/coreplugin/locator/javascriptfilter.h b/src/plugins/coreplugin/locator/javascriptfilter.h index 9bf3ac51949..025ce2fa6a0 100644 --- a/src/plugins/coreplugin/locator/javascriptfilter.h +++ b/src/plugins/coreplugin/locator/javascriptfilter.h @@ -30,7 +30,7 @@ #include QT_BEGIN_NAMESPACE -class QJSEngine; +class QScriptEngine; QT_END_NAMESPACE namespace Core { @@ -53,7 +53,7 @@ public: private: void setupEngine(); - mutable std::unique_ptr m_engine; + mutable std::unique_ptr m_engine; }; } // namespace Internal diff --git a/src/plugins/coreplugin/locator/locator.cpp b/src/plugins/coreplugin/locator/locator.cpp index e57f20241ac..6a4f530f3cb 100644 --- a/src/plugins/coreplugin/locator/locator.cpp +++ b/src/plugins/coreplugin/locator/locator.cpp @@ -122,8 +122,10 @@ void Locator::initialize(CorePlugin *corePlugin, const QStringList &, QString *) new LocatorManager(this); +#ifdef WITH_JAVASCRIPTFILTER m_javaScriptFilter = new JavaScriptFilter; m_corePlugin->addObject(m_javaScriptFilter); +#endif m_openDocumentsFilter = new OpenDocumentsFilter; m_corePlugin->addObject(m_openDocumentsFilter); diff --git a/src/plugins/coreplugin/locator/locator.pri b/src/plugins/coreplugin/locator/locator.pri index 183d7fde515..c5f2cfaf9c2 100644 --- a/src/plugins/coreplugin/locator/locator.pri +++ b/src/plugins/coreplugin/locator/locator.pri @@ -15,8 +15,7 @@ HEADERS += \ $$PWD/executefilter.h \ $$PWD/locatorsearchutils.h \ $$PWD/locatorsettingspage.h \ - $$PWD/externaltoolsfilter.h \ - $$PWD/javascriptfilter.h + $$PWD/externaltoolsfilter.h SOURCES += \ $$PWD/locator.cpp \ @@ -33,7 +32,18 @@ SOURCES += \ $$PWD/locatorsearchutils.cpp \ $$PWD/locatorsettingspage.cpp \ $$PWD/externaltoolsfilter.cpp \ - $$PWD/javascriptfilter.cpp + +qtHaveModule(script) { + QT *= script + + DEFINES += WITH_JAVASCRIPTFILTER + + HEADERS += \ + $$PWD/javascriptfilter.h + + SOURCES += \ + $$PWD/javascriptfilter.cpp +} FORMS += \ $$PWD/filesystemfilter.ui \