diff --git a/sources/scripting/qetscriptapi.cpp b/sources/scripting/qetscriptapi.cpp index 54b0d9054..34a6d8ee4 100644 --- a/sources/scripting/qetscriptapi.cpp +++ b/sources/scripting/qetscriptapi.cpp @@ -86,6 +86,7 @@ #include #include #include +#include #include #include #include @@ -677,6 +678,17 @@ QString QetScriptApi::addElement(int folioIndex, const QString &locationPath, do const QString import_path = location.isFileSystem() ? QStringLiteral("import/") + location.collectionPath(false) : location.collectionPath(false); + // An element file that exists but cannot be read gives a null + // uuid(), which the collision check below would misreport as "a + // different element" -- say what actually went wrong instead. + if (location.isFileSystem() + && location.pugiXml().document_element().empty()) { + const QString file = QDir::toNativeSeparators( + QFileInfo(location.fileSystemPath()).absoluteFilePath()); + log(QStringLiteral("qet.addElement: could not read element '%1' (file '%2', " + "%3 characters)").arg(locationPath, file).arg(file.size())); + return QString(); + } const ElementsLocation existing(import_path, m_project); if (existing.exist() && existing.uuid() != location.uuid()) { log(QStringLiteral("qet.addElement: '%1' would collide with a different element " diff --git a/tests/qttest/CMakeLists.txt b/tests/qttest/CMakeLists.txt index 3b1085990..89fbe8c97 100644 --- a/tests/qttest/CMakeLists.txt +++ b/tests/qttest/CMakeLists.txt @@ -429,6 +429,18 @@ if(QET_HAS_SCRIPTING) "QET_TEST_BINARY_PATH=\"$\"" "QET_ELEMENTS_DIR=\"${QET_DIR}/elements\"") + # qet.addElement() on a symbol that exists but cannot be read says so, + # rather than reporting a collision with the copy already embedded. + add_executable( + tst_unreadableelement + tst_unreadableelement.cpp) + add_test(NAME tst_unreadableelement COMMAND tst_unreadableelement) + add_dependencies(tst_unreadableelement qelectrotech) + target_link_libraries(tst_unreadableelement PRIVATE Qt::Test) + target_compile_definitions(tst_unreadableelement PRIVATE + "QET_TEST_BINARY_PATH=\"$\"" + "QET_ELEMENTS_DIR=\"${QET_DIR}/elements\"") + # The project database filled from the file as it is read holds exactly # what it holds filled from the built folios (QET_DATABASE_FROM_FOLIOS=1), # on every example saved once; an older file falls back, saying why. diff --git a/tests/qttest/tst_unreadableelement.cpp b/tests/qttest/tst_unreadableelement.cpp new file mode 100644 index 000000000..906b8da58 --- /dev/null +++ b/tests/qttest/tst_unreadableelement.cpp @@ -0,0 +1,115 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +#include + +#include +#include +#include +#include +#include + +// qet.addElement() compares the uuid of the symbol it is given with the copy +// already embedded under the same name. A symbol file that exists but cannot +// be read has no uuid, and was reported as "would collide with a different +// element" (#1178 follow-up: on Windows, any symbol whose full path reaches +// 260 characters). Made unreadable here with the file's permissions, which +// gives the same null uuid on Linux. +class tst_unreadableelement : public QObject +{ + Q_OBJECT + + QTemporaryDir m_dir; + QString m_collection; + QString m_symbol; + + // Runs a script placing common://custom/my_siren.elmt into project and + // saving it to output; returns everything the run printed. + QString placeTheSymbol(const QString &project, const QString &output) + { + const QString root = m_dir.path(); + const QString home = root + QStringLiteral("/home"); + const QString settings = root + QStringLiteral("/settings"); + QDir().mkpath(settings + QStringLiteral("/QElectroTech")); + QDir().mkpath(root + QStringLiteral("/tmp")); + QFile ini(settings + QStringLiteral("/QElectroTech/QElectroTech.ini")); + if (!ini.open(QIODevice::WriteOnly | QIODevice::Text)) + return QString(); + ini.write("[elements-collections]\ncommon-collection-path="); + ini.write(m_collection.toUtf8()); + ini.write("\n"); + ini.close(); + + const QString script = root + QStringLiteral("/add.js"); + QFile js(script); + if (!js.open(QIODevice::WriteOnly)) + return QString(); + js.write("var u = qet.addElement(0, 'common://custom/my_siren.elmt', 100, 100);\n" + "qet.log('RESULT ' + (u ? 'placed' : 'refused'));\n" + "if (u) qet.save('"); + js.write(output.toUtf8()); + js.write("');\n"); + js.close(); + + QProcessEnvironment env = QProcessEnvironment::systemEnvironment(); + env.insert(QStringLiteral("QT_QPA_PLATFORM"), QStringLiteral("offscreen")); + env.insert(QStringLiteral("QET_ENABLE_SCRIPTING"), QStringLiteral("1")); + env.insert(QStringLiteral("HOME"), home); + env.insert(QStringLiteral("XDG_CONFIG_HOME"), home + QStringLiteral("/.config")); + env.insert(QStringLiteral("XDG_DATA_HOME"), home + QStringLiteral("/.local/share")); + env.insert(QStringLiteral("TMPDIR"), root + QStringLiteral("/tmp")); + env.insert(QStringLiteral("QET_SETTINGS_DIR"), settings); + + QProcess proc; + proc.setProcessEnvironment(env); + proc.setProcessChannelMode(QProcess::MergedChannels); + proc.start(QStringLiteral(QET_TEST_BINARY_PATH), {QStringLiteral("--run"), script, project}); + if (!proc.waitForFinished(60000)) + return QString(); + return QString::fromUtf8(proc.readAll()); + } + +private slots: + void initTestCase() + { + QVERIFY(m_dir.isValid()); + QVERIFY(QFile::exists(QStringLiteral(QET_TEST_BINARY_PATH))); + m_collection = m_dir.filePath(QStringLiteral("collection")); + m_symbol = m_collection + QStringLiteral("/custom/my_siren.elmt"); + QVERIFY(QDir().mkpath(m_collection + QStringLiteral("/custom"))); + QVERIFY(QFile::copy(QStringLiteral(QET_ELEMENTS_DIR "/10_electric/10_allpole/380_signaling_operating/12_acoustic_signaling/sirene.elmt"), + m_symbol)); + QVERIFY(QFile::copy(QFINDTESTDATA("fixtures/qet_bug_repro_resaved.qet"), + m_dir.filePath(QStringLiteral("p.qet")))); + } + + void anUnreadableSymbolIsNotACollision() + { + // Placed once while readable: the project now embeds it. + const QString embedded = m_dir.filePath(QStringLiteral("embedded.qet")); + QVERIFY(placeTheSymbol(m_dir.filePath(QStringLiteral("p.qet")), embedded) + .contains(QStringLiteral("RESULT placed"))); + QVERIFY(QFile::exists(embedded)); + + QVERIFY(QFile::setPermissions(m_symbol, QFileDevice::Permissions())); + QFile probe(m_symbol); + if (probe.open(QIODevice::ReadOnly)) + QSKIP("permissions do not stop this user reading the file (root?)"); + + const QString out = placeTheSymbol(embedded, m_dir.filePath(QStringLiteral("again.qet"))); + QFile::setPermissions(m_symbol, QFileDevice::ReadOwner | QFileDevice::WriteOwner); + QVERIFY2(out.contains(QStringLiteral("RESULT refused")), qPrintable(out)); + QVERIFY2(out.contains(QStringLiteral("could not read element")), qPrintable(out)); + QVERIFY2(!out.contains(QStringLiteral("would collide")), qPrintable(out)); + } + + // Readable again, the same symbol is placed a second time as before. + void theSameSymbolIsPlacedAgain() + { + const QString out = placeTheSymbol(m_dir.filePath(QStringLiteral("embedded.qet")), + m_dir.filePath(QStringLiteral("twice.qet"))); + QVERIFY2(out.contains(QStringLiteral("RESULT placed")), qPrintable(out)); + } +}; + +QTEST_APPLESS_MAIN(tst_unreadableelement) + +#include "tst_unreadableelement.moc"