diff --git a/sources/qet.cpp b/sources/qet.cpp index a4c5dee52..84982c179 100644 --- a/sources/qet.cpp +++ b/sources/qet.cpp @@ -546,12 +546,40 @@ QString QET::joinWithSpaces(const QStringList &string_list) { QStringList QET::splitWithSpaces(const QString &string) { // les chaines sont separees par des espaces non echappes // = avec un nombre nul ou pair de backslashes devant - - QStringList escaped_strings = string.split(QRegularExpression("[^\\]?(?:\\\\)* "),Qt::SkipEmptyParts); - + // + // This was a QRegularExpression("[^\\]?(?:\\\\)* ") split, which never + // worked: "[^\]" opens a character class whose "\]" is an escaped + // bracket, so the class is never closed and the pattern is invalid. + // QRegularExpression::isValid() was false, QString::split() warned + // "invalid QRegularExpression object" and returned an EMPTY list for + // every input -- so a second instance's file arguments were always + // dropped (bugtracker #248), not just ones containing spaces. + // + // A correct pattern is not expressible here either: the separator is a + // space preceded by an even-length run of backslashes, and PCRE2 has no + // variable-length lookbehind. Scanning explicitly is both correct and + // easier to read than the alternatives. QStringList returned_list; - foreach(QString escaped_string, escaped_strings) { - returned_list << QET::unescapeSpaces(escaped_string); + QString current; + int backslashes = 0; + for (const QChar &c : string) { + if (c == QLatin1Char('\\')) { + ++backslashes; + current += c; + continue; + } + if (c == QLatin1Char(' ') && backslashes % 2 == 0) { + if (!current.isEmpty()) { + returned_list << QET::unescapeSpaces(current); + } + current.clear(); + } else { + current += c; + } + backslashes = 0; + } + if (!current.isEmpty()) { + returned_list << QET::unescapeSpaces(current); } return(returned_list); } diff --git a/tests/qttest/CMakeLists.txt b/tests/qttest/CMakeLists.txt index 4fec2b9a3..11bbde5c7 100644 --- a/tests/qttest/CMakeLists.txt +++ b/tests/qttest/CMakeLists.txt @@ -100,3 +100,16 @@ add_executable( add_test(NAME tst_smart_device COMMAND tst_smart_device) target_include_directories(tst_smart_device PRIVATE ${QET_DIR}/sources) target_link_libraries(tst_smart_device PRIVATE Qt::Test Qt::Sql) + +# qet.cpp carries the SingleApplication argument wire format +# (joinWithSpaces/splitWithSpaces); it pulls in qeticons and shortcutmanager, +# so those are compiled alongside rather than linking the whole application. +add_executable( + tst_qetstrings + tst_qetstrings.cpp + ${QET_DIR}/sources/qet.cpp + ${QET_DIR}/sources/qeticons.cpp + ${QET_DIR}/sources/shortcutmanager.cpp) +add_test(NAME tst_qetstrings COMMAND tst_qetstrings) +target_include_directories(tst_qetstrings PRIVATE ${QET_DIR}/sources) +target_link_libraries(tst_qetstrings PRIVATE Qt::Test Qt::Widgets Qt::Xml) diff --git a/tests/qttest/tst_qetstrings.cpp b/tests/qttest/tst_qetstrings.cpp new file mode 100644 index 000000000..f7e68011d --- /dev/null +++ b/tests/qttest/tst_qetstrings.cpp @@ -0,0 +1,54 @@ +#include + +#include "qet.h" + +/** + QET::joinWithSpaces() / QET::splitWithSpaces() are the wire format for the + SingleApplication message a secondary instance sends to the running one + (main.cpp: "launched-with-args: " + joinWithSpaces(...), received by + QETApp::receiveMessage()). If the round trip loses arguments, opening a + file while QET is already running silently does nothing. + + splitWithSpaces() used to split on QRegularExpression("[^\\]?(?:\\\\)* "), + which is not a valid pattern: "[^\\]" opens a character class whose "\\]" + is an escaped bracket, so the class never closes. QRegularExpression + reported isValid() == false and QString::split() returned an empty list for + every input -- bugtracker #248. +*/ +class tst_qetstrings : public QObject +{ + Q_OBJECT + +private slots: + void roundTrips_data() + { + QTest::addColumn("input"); + + QTest::newRow("single plain") << QStringList{"one.qet"}; + QTest::newRow("two plain") << QStringList{"one.qet", "two.qet"}; + QTest::newRow("space in name") << QStringList{"my file.qet"}; + QTest::newRow("space then plain") << QStringList{"my file.qet", "other.qet"}; + QTest::newRow("spaces in path") << QStringList{"/home/a b/c d.qet", "/tmp/x.qet"}; + QTest::newRow("backslash in name") << QStringList{"back\\slash.qet"}; + QTest::newRow("trailing backslash") << QStringList{"trailing\\"}; + QTest::newRow("mixed") << QStringList{"a b", "c\\d", "e"}; + } + + /// What the IPC actually needs: whatever went in comes back out. + void roundTrips() + { + QFETCH(QStringList, input); + QCOMPARE(QET::splitWithSpaces(QET::joinWithSpaces(input)), input); + } + + /// The specific regression: the old implementation returned an empty list + /// for every input, so this passed nothing on to openFiles(). + void splitIsNotEmptyForPlainArguments() + { + QVERIFY(!QET::splitWithSpaces(QStringLiteral("one.qet")).isEmpty()); + QCOMPARE(QET::splitWithSpaces(QStringLiteral("a.qet b.qet")).count(), 2); + } +}; + +QTEST_APPLESS_MAIN(tst_qetstrings) +#include "tst_qetstrings.moc"