Editor: Validate trigger from snippets xml

Fixes: QTCREATORBUG-24415
Change-Id: Ic99355220bf1948af8a17d6ce017924eba3fc3ba
Reviewed-by: Christian Stenger <christian.stenger@qt.io>

src/plugins/texteditor/snippets/snippet.cpp

@@ -25,6 +25,7 @@
 
 #include "snippet.h"
 
+#include <utils/algorithm.h>
 #include <utils/templateengine.h>
 
 #include <QTextDocument>
@@ -105,6 +106,13 @@ const QString &Snippet::trigger() const
     return m_trigger;
 }
 
+bool Snippet::isValidTrigger(const QString &trigger)
+{
+    if (trigger.isEmpty() || trigger.at(0).isNumber())
+        return false;
+    return Utils::allOf(trigger, [](const QChar &c) { return c.isLetterOrNumber() || c == '_'; });
+}
+
 void Snippet::setContent(const QString &content)
 {
     m_content = content;

src/plugins/texteditor/snippets/snippet.h

@@ -57,6 +57,7 @@ public:
 
     void setTrigger(const QString &trigger);
     const QString &trigger() const;
+    static bool isValidTrigger(const QString &trigger);
 
     void setContent(const QString &content);
     const QString &content() const;

src/plugins/texteditor/snippets/snippetscollection.cpp

@@ -362,9 +362,18 @@ QList<Snippet> SnippetsCollection::readXML(const QString &fileName, const QStrin
                         const QXmlStreamAttributes &atts = xml.attributes();
                         const QString &id = atts.value(kId).toString();
                         const QString &groupId = atts.value(kGroup).toString();
-                        if (isGroupKnown(groupId) && (snippetId.isEmpty() || snippetId == id)) {
+                        const QString &trigger = atts.value(kTrigger).toString();
+                        if (!Snippet::isValidTrigger(trigger)) {
+                            qWarning()
+                                << fileName << "ignore snippet for invalid trigger" << trigger
+                                << "A valid trigger can only contain letters, "
+                                   "numbers, or underscores, where the first character is "
+                                   "limited to letter or underscore.";
+
+                            xml.skipCurrentElement();
+                        } else if (isGroupKnown(groupId) && (snippetId.isEmpty() || snippetId == id)) {
                             Snippet snippet(groupId, id);
-                            snippet.setTrigger(atts.value(kTrigger).toString());
+                            snippet.setTrigger(trigger);
                             snippet.setComplement(QCoreApplication::translate(
                                                       "TextEditor::Internal::Snippets",
                                                       atts.value(kComplement).toString().toLatin1(),

src/plugins/texteditor/snippets/snippetssettingspage.cpp

@@ -79,7 +79,6 @@ public:
 
 private:
     void replaceSnippet(const Snippet &snippet, const QModelIndex &modelIndex);
-    static bool isValidTrigger(const QString &s);
 
     SnippetsCollection* m_collection;
     QString m_activeGroupId;
@@ -130,9 +129,13 @@ bool SnippetsTableModel::setData(const QModelIndex &modelIndex, const QVariant &
         Snippet snippet(m_collection->snippet(modelIndex.row(), m_activeGroupId));
         if (modelIndex.column() == 0) {
             const QString &s = value.toString();
-            if (!isValidTrigger(s)) {
+            if (!Snippet::isValidTrigger(s)) {
-                QMessageBox::critical(Core::ICore::dialogParent(), tr("Error"),
+                QMessageBox::critical(
-                                      tr("Not a valid trigger."));
+                    Core::ICore::dialogParent(),
+                    tr("Error"),
+                    tr("Not a valid trigger. A valid trigger can only contain letters, "
+                       "numbers, or underscores, where the first character is "
+                       "limited to letter or underscore."));
                 if (snippet.trigger().isEmpty())
                     removeSnippet(modelIndex);
                 return false;
@@ -251,16 +254,6 @@ void SnippetsTableModel::replaceSnippet(const Snippet &snippet, const QModelInde
     }
 }
 
-bool SnippetsTableModel::isValidTrigger(const QString &s)
-{
-    if (s.isEmpty())
-        return false;
-    for (int i = 0; i < s.length(); ++i)
-        if (!s.at(i).isLetter() && s.at(i) != QLatin1Char('_') && (!s.at(i).isDigit() || i == 0))
-            return false;
-    return true;
-}
-
 // SnippetsSettingsPagePrivate
 class SnippetsSettingsPagePrivate : public QObject
 {