Merge pull request #1243 from ispyisail/fix/element-long-paths

Fix symbols in deep folders on Windows (paths of 260+ characters)

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

sources/ElementsCollection/fileelementcollectionitem.cpp

@@ -24,6 +24,7 @@
 
 #include <QApplication>
 #include <QDir>
+#include <QFile>
 #include <QPainter>
 #include <QPixmap>
 #include <QStyle>
@@ -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")

sources/scripting/qetscriptapi.cpp

@@ -86,6 +86,7 @@
 #include <QSqlError>
 #include <QSqlQuery>
 #include <QSqlRecord>
+#include <QDir>
 #include <QDomDocument>
 #include <QFileInfo>
 #include <QFont>
@@ -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 "

tests/qttest/CMakeLists.txt

@@ -436,6 +436,18 @@ if(QET_HAS_SCRIPTING)
     "QET_TEST_BINARY_PATH=\"$<TARGET_FILE:qelectrotech>\""
     "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=\"$<TARGET_FILE:qelectrotech>\""
+    "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.

tests/qttest/tst_unreadableelement.cpp

@@ -0,0 +1,115 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+#include <QtTest>
+
+#include <QDir>
+#include <QFile>
+#include <QProcess>
+#include <QProcessEnvironment>
+#include <QTemporaryDir>
+
+// 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"