From af184ed986350c3ddd476eead7495a3384bb6e04 Mon Sep 17 00:00:00 2001 From: ispyisail Date: Fri, 2 Oct 2026 22:22:18 +1300 Subject: [PATCH 1/2] Read symbol files through QFile so paths past 260 characters work on Windows ElementsLocation::pugiXml() and the qet_directory name lookup opened files with pugixml's load_file(), which on Windows fails once the full path reaches MAX_PATH. QFile handles long paths, so exist() and the import succeeded while uuid(), the name, the informations and the thumbnail of the same symbol came back empty. Read the bytes with QFile and hand them to load_buffer(). Reported on #1178 as a false collision from qet.addElement. Co-Authored-By: Claude Opus 5.5 --- sources/ElementsCollection/elementslocation.cpp | 12 ++++++++++-- .../ElementsCollection/fileelementcollectionitem.cpp | 8 +++++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/sources/ElementsCollection/elementslocation.cpp b/sources/ElementsCollection/elementslocation.cpp index 7c34f923a..0e8a94c7a 100644 --- a/sources/ElementsCollection/elementslocation.cpp +++ b/sources/ElementsCollection/elementslocation.cpp @@ -709,12 +709,20 @@ pugi::xml_document ElementsLocation::pugiXml() const #endif if (!m_project) { + //Read through QFile, not pugi's load_file(): on Windows load_file() + //fails once the full path reaches MAX_PATH (260 characters), while + //QFile handles long paths. + QFile file(m_file_system_path); + if (!file.open(QIODevice::ReadOnly)) { + return docu; + } + const QByteArray data = file.readAll(); #ifndef Q_OS_LINUX - if (docu.load_file(m_file_system_path.toStdWString().c_str())) { + if (docu.load_buffer(data.constData(), data.size())) { docu.save(m_string_stream); } #else - docu.load_file(m_file_system_path.toStdWString().c_str()); + docu.load_buffer(data.constData(), data.size()); #endif } else diff --git a/sources/ElementsCollection/fileelementcollectionitem.cpp b/sources/ElementsCollection/fileelementcollectionitem.cpp index 7b9d3ea98..805c3fc27 100644 --- a/sources/ElementsCollection/fileelementcollectionitem.cpp +++ b/sources/ElementsCollection/fileelementcollectionitem.cpp @@ -24,6 +24,7 @@ #include #include +#include #include #include #include @@ -178,7 +179,12 @@ QString FileElementCollectionItem::localName() bool readable = false; QString str(fileSystemPath() % "/qet_directory"); pugi::xml_document docu; - if (docu.load_file(str.toStdWString().c_str())) + // QFile rather than pugi's load_file(), which fails on + // Windows once the full path reaches 260 characters. + QFile file(str); + const QByteArray data = file.open(QIODevice::ReadOnly) + ? file.readAll() : QByteArray(); + if (!data.isEmpty() && docu.load_buffer(data.constData(), data.size())) { if (QString(docu.document_element().name()) == "qet-directory") From e94c6d82c9e087a99a9e190a6041c6cc749243bb Mon Sep 17 00:00:00 2001 From: ispyisail Date: Fri, 2 Oct 2026 22:22:18 +1300 Subject: [PATCH 2/2] qet.addElement: report an unreadable symbol instead of a collision A symbol file that exists but cannot be read has a null uuid, which the check against the copy already embedded reported as "would collide with a different element". Say that the file could not be read, with its full path and length, since a long path is the usual cause on Windows. tst_unreadableelement makes the file unreadable through its permissions and fails on master with the old message. Co-Authored-By: Claude Opus 5.5 --- sources/scripting/qetscriptapi.cpp | 12 +++ tests/qttest/CMakeLists.txt | 12 +++ tests/qttest/tst_unreadableelement.cpp | 115 +++++++++++++++++++++++++ 3 files changed, 139 insertions(+) create mode 100644 tests/qttest/tst_unreadableelement.cpp 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"