Refactors Dolphin quick access integration for robustness - Fixes #3883

src/main/java/module-info.java

@@ -18,6 +18,7 @@
 	requires org.purejava.appindicator;
 	requires org.purejava.kwallet;
 	requires de.swiesend.secretservice;
+	requires java.xml;
 
 	provides AutoStartProvider with FreedesktopAutoStartService;
 	provides KeychainAccessProvider with GnomeKeyringKeychainAccess, KDEWalletKeychainAccess;

src/main/java/org/cryptomator/linux/quickaccess/DolphinPlaces.java

@@ -6,18 +6,40 @@
 import org.cryptomator.integrations.common.Priority;
 import org.cryptomator.integrations.quickaccess.QuickAccessService;
 import org.cryptomator.integrations.quickaccess.QuickAccessServiceException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.w3c.dom.DOMException;
+import org.w3c.dom.Document;
+import org.w3c.dom.Node;
+import org.w3c.dom.NodeList;
+import org.xml.sax.InputSource;
 import org.xml.sax.SAXException;
 
 import javax.xml.XMLConstants;
+import javax.xml.namespace.QName;
+import javax.xml.parsers.DocumentBuilder;
+import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.transform.OutputKeys;
 import javax.xml.transform.Source;
+import javax.xml.transform.Transformer;
+import javax.xml.transform.TransformerException;
+import javax.xml.transform.TransformerFactory;
+import javax.xml.transform.dom.DOMSource;
+import javax.xml.transform.stream.StreamResult;
 import javax.xml.transform.stream.StreamSource;
 import javax.xml.validation.SchemaFactory;
 import javax.xml.validation.Validator;
+import javax.xml.xpath.XPathConstants;
+import javax.xml.xpath.XPathExpressionException;
+import javax.xml.xpath.XPathFactory;
+import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.StringReader;
+import java.io.StringWriter;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.util.List;
 import java.util.UUID;
 
 /**
@@ -29,20 +51,11 @@
 @Priority(90)
 public class DolphinPlaces extends FileConfiguredQuickAccess implements QuickAccessService {
 
+	private static final Logger LOG = LoggerFactory.getLogger(DolphinPlaces.class);
+
+	private static final String XBEL_NAMESPACE = "http://www.freedesktop.org/standards/desktop-bookmarks";
 	private static final int MAX_FILE_SIZE = 1 << 20; //1MiB, xml is quite verbose
 	private static final Path PLACES_FILE = Path.of(System.getProperty("user.home"), ".local/share/user-places.xbel");
-	private static final String ENTRY_TEMPLATE = """
-			<bookmark href=\"%s\">
-			 <title>%s</title>
-			 <info>
-			  <metadata owner=\"http://freedesktop.org\">
-			   <bookmark:icon name="drive-harddisk-encrypted"/>
-			  </metadata>
-			  <metadata owner=\"https://cryptomator.org\">
-			  	<id>%s</id>
-			  </metadata>
-			 </info>
-			</bookmark>""";
 
 	private static final Validator XML_VALIDATOR;
 
@@ -61,29 +74,157 @@ public DolphinPlaces() {
 		super(PLACES_FILE, MAX_FILE_SIZE);
 	}
 
+	public DolphinPlaces(Path configFilePath) {
+		super(configFilePath, MAX_FILE_SIZE);
+	}
+
 	@Override
 	EntryAndConfig addEntryToConfig(String config, Path target, String displayName) throws QuickAccessServiceException {
 		try {
-			String id = UUID.randomUUID().toString();
-			//validate
+			var id = UUID.randomUUID().toString();
+			LOG.trace("Adding bookmark for target: '{}', displayName: '{}', id: '{}'", target, displayName, id);
 			XML_VALIDATOR.validate(new StreamSource(new StringReader(config)));
-			// modify
-			int insertIndex = config.lastIndexOf("</xbel"); //cannot be -1 due to validation; we do not match the whole end tag, since between tag name and closing bracket can be whitespaces
-			var adjustedConfig = config.substring(0, insertIndex) //
-					+ "\n" //
-					+ ENTRY_TEMPLATE.formatted(target.toUri(), escapeXML(displayName), id).indent(1) //
-					+ "\n" //
-					+ config.substring(insertIndex);
-			return new EntryAndConfig(new DolphinPlacesEntry(id), adjustedConfig);
-		} catch (SAXException | IOException e) {
-			throw new QuickAccessServiceException("Adding entry to KDE places file failed.", e);
+			var xmlDocument = loadXmlDocument(config);
+			var nodeList = extractBookmarksByPath(target, xmlDocument);
+			removeStaleBookmarks(nodeList);
+			createBookmark(target, displayName, id, xmlDocument);
+			var changedConfig = documentToString(xmlDocument);
+			XML_VALIDATOR.validate(new StreamSource(new StringReader(changedConfig)));
+			return new EntryAndConfig(new DolphinPlacesEntry(id), changedConfig);
+		} catch (SAXException e) {
+			throw new QuickAccessServiceException("Invalid structure in xbel bookmark file", e);
+		} catch (IOException e) {
+			throw new QuickAccessServiceException("Failed reading/writing the xbel bookmark file", e);
 		}
 	}
 
-	private String escapeXML(String s) {
-		return s.replace("&","&amp;") //
-				.replace("<","&lt;") //
-				.replace(">","&gt;");
+	private void removeStaleBookmarks(NodeList nodeList) {
+		for (int i = nodeList.getLength() - 1; i >= 0; i--) {
+			Node node = nodeList.item(i);
+			node.getParentNode().removeChild(node);
+		}
+	}
+
+	private NodeList extractBookmarksByPath(Path target, Document xmlDocument) throws QuickAccessServiceException {
+		try {
+			var xpathFactory = XPathFactory.newInstance();
+			var xpath = xpathFactory.newXPath();
+			xpath.setXPathVariableResolver(v -> {
+				if (v.equals(new QName("uri"))) {
+					return target.toUri().toString();
+				}
+				throw new IllegalArgumentException();
+			});
+			var expression = "/xbel/bookmark[info/metadata[@owner='https://cryptomator.org']][@href=$uri]";
+			return (NodeList) xpath.compile(expression).evaluate(xmlDocument, XPathConstants.NODESET);
+		} catch (XPathExpressionException xee) {
+			throw new QuickAccessServiceException("Invalid XPath expression", xee);
+		}
+	}
+
+	private NodeList extractBookmarksById(String id, Document xmlDocument) throws QuickAccessServiceException {
+		try {
+			var xpathFactory = XPathFactory.newInstance();
+			var xpath = xpathFactory.newXPath();
+			xpath.setXPathVariableResolver(v -> {
+				if (v.equals(new QName("id"))) {
+					return id;
+				}
+				throw new IllegalArgumentException();
+			});
+			var expression = "/xbel/bookmark[info/metadata[@owner='https://cryptomator.org']][info/metadata/id[text()=$id]]";
+			return (NodeList) xpath.compile(expression).evaluate(xmlDocument, XPathConstants.NODESET);
+		} catch (XPathExpressionException xee) {
+			throw new QuickAccessServiceException("Invalid XPath expression", xee);
+		}
+	}
+
+	private Document loadXmlDocument(String config) throws QuickAccessServiceException {
+		try {
+			var builderFactory = DocumentBuilderFactory.newInstance();
+			builderFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+			builderFactory.setXIncludeAware(false);
+			builderFactory.setExpandEntityReferences(false);
+			builderFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
+			builderFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");
+			builderFactory.setNamespaceAware(true);
+			DocumentBuilder builder = builderFactory.newDocumentBuilder();
+			// Prevent external entities from being resolved
+			builder.setEntityResolver((publicId, systemId) -> new InputSource(new StringReader("")));
+			return builder.parse(new ByteArrayInputStream(config.getBytes(StandardCharsets.UTF_8)));
+		} catch (IOException | SAXException | ParserConfigurationException e) {
+			throw new QuickAccessServiceException("Error while loading xml file", e);
+		}
+	}
+
+	private String documentToString(Document xmlDocument) throws QuickAccessServiceException {
+		try {
+			var buf = new StringWriter();
+			Transformer transformer = TransformerFactory.newInstance().newTransformer();
+			transformer.setOutputProperty(OutputKeys.DOCTYPE_PUBLIC, "");
+			transformer.setOutputProperty(OutputKeys.DOCTYPE_SYSTEM, "");
+			transformer.setOutputProperty(OutputKeys.OMIT_XML_DECLARATION, "no");
+			transformer.setOutputProperty(OutputKeys.INDENT, "yes");
+			transformer.setOutputProperty(OutputKeys.ENCODING, StandardCharsets.UTF_8.name());
+			transformer.transform(new DOMSource(xmlDocument), new StreamResult(buf));
+			var content = buf.toString();
+			content = content.replaceFirst("\\s*standalone=\"(yes|no)\"", "");
+			content = content.replaceFirst("<!DOCTYPE xbel PUBLIC \"\" \"\">","<!DOCTYPE xbel>");
+			return content;
+		} catch (TransformerException e) {
+			throw new QuickAccessServiceException("Error while serializing document to string", e);
+		}
+	}
+
+	/**
+	 *
+	 * Adds a xml bookmark element to the specified xml document
+	 *
+	 * <pre>{@code
+	 * <bookmark href="file:///home/someuser/folder1/">
+	 *   <title>integrations-linux</title>
+	 *   <info>
+	 *     <metadata owner="http://freedesktop.org">
+	 *       <bookmark:icon name="drive-harddisk-encrypted"/>
+	 *     </metadata>
+	 *     <metadata owner="https://cryptomator.org">
+	 *       <id>sldkf-sadf-sadf-sadf</id>
+	 *     </metadata>
+	 *   </info>
+	 * </bookmark>
+	 * }</pre>
+	 *
+	 * @param target The mount point of the vault
+	 * @param displayName Caption of the vault link in dolphin
+	 * @param xmlDocument The xbel document to which the bookmark should be added
+	 *
+	 * @throws QuickAccessServiceException if the bookmark could not be created
+	 */
+	private void createBookmark(Path target, String displayName, String id, Document xmlDocument) throws QuickAccessServiceException {
+		try {
+			var bookmark = xmlDocument.createElement("bookmark");
+			var title = xmlDocument.createElement("title");
+			var info = xmlDocument.createElement("info");
+			var metadataBookmark = xmlDocument.createElement("metadata");
+			var metadataOwner = xmlDocument.createElement("metadata");
+			var bookmarkIcon = xmlDocument.createElementNS(XBEL_NAMESPACE, "bookmark:icon");
+			var idElem = xmlDocument.createElement("id");
+			bookmark.setAttribute("href", target.toUri().toString());
+			title.setTextContent(displayName);
+			bookmark.appendChild(title);
+			bookmark.appendChild(info);
+			info.appendChild(metadataBookmark);
+			info.appendChild(metadataOwner);
+			metadataBookmark.appendChild(bookmarkIcon);
+			metadataOwner.appendChild(idElem);
+			metadataBookmark.setAttribute("owner", "http://freedesktop.org");
+			bookmarkIcon.setAttribute("name","drive-harddisk-encrypted");
+			metadataOwner.setAttribute("owner", "https://cryptomator.org");
+			idElem.setTextContent(id);
+			xmlDocument.getDocumentElement().appendChild(bookmark);
+		} catch (DOMException | IllegalArgumentException e) {
+			throw new QuickAccessServiceException("Error while creating bookmark for target: " + target, e);
+		}
 	}
 
 	private class DolphinPlacesEntry extends FileConfiguredQuickAccessEntry implements QuickAccessEntry {
@@ -97,46 +238,20 @@ private class DolphinPlacesEntry extends FileConfiguredQuickAccessEntry implemen
 		@Override
 		public String removeEntryFromConfig(String config) throws QuickAccessServiceException {
 			try {
-				int idIndex = config.lastIndexOf(id);
-				if (idIndex == -1) {
-					return config; //assume someone has removed our entry, nothing to do
-				}
-				//validate
-				XML_VALIDATOR.validate(new StreamSource(new StringReader(config)));
-				//modify
-				int openingTagIndex = indexOfEntryOpeningTag(config, idIndex);
-				var contentToWrite1 = config.substring(0, openingTagIndex).stripTrailing();
-
-				int closingTagEndIndex = config.indexOf('>', config.indexOf("</bookmark", idIndex));
-				var part2Tmp = config.substring(closingTagEndIndex + 1).split("\\A\\v+", 2); //removing leading vertical whitespaces, but no indentation
-				var contentToWrite2 = part2Tmp[part2Tmp.length - 1];
-
-				return contentToWrite1 + "\n" + contentToWrite2;
+				var xmlDocument = loadXmlDocument(config);
+				var nodeList = extractBookmarksById(id, xmlDocument);
+				removeStaleBookmarks(nodeList);
+				var changedConfig = documentToString(xmlDocument);
+				XML_VALIDATOR.validate(new StreamSource(new StringReader(changedConfig)));
+				return changedConfig;
 			} catch (IOException | SAXException | IllegalStateException e) {
 				throw new QuickAccessServiceException("Removing entry from KDE places file failed.", e);
 			}
 		}
-
-		/**
-		 * Returns the start index (inclusive)  of the {@link DolphinPlaces#ENTRY_TEMPLATE} entry
-		 * @param placesContent the content of the XBEL places file
-		 * @param idIndex start index (inclusive) of the entrys id tag value
-		 * @return start index of the first bookmark tag, searching backwards from idIndex
-		 */
-		private int indexOfEntryOpeningTag(String placesContent, int idIndex) {
-			var xmlWhitespaceChars = List.of(' ', '\t', '\n');
-			for (char c : xmlWhitespaceChars) {
-				int idx = placesContent.lastIndexOf("<bookmark" + c, idIndex); //with the whitespace we ensure, that no tags starting with "bookmark" (e.g. bookmarkz) are selected
-				if (idx != -1) {
-					return idx;
-				}
-			}
-			throw new IllegalStateException("Found entry id " + id + " in " + PLACES_FILE + ", but it is not a child of <bookmark> tag.");
-		}
 	}
 
 	@CheckAvailability
 	public static boolean isSupported() {
 		return Files.exists(PLACES_FILE);
 	}
-}
+}

src/main/java/org/cryptomator/linux/quickaccess/FileConfiguredQuickAccess.java

@@ -31,10 +31,20 @@ abstract class FileConfiguredQuickAccess implements QuickAccessService {
 		Runtime.getRuntime().addShutdownHook(new Thread(this::cleanup));
 	}
 
+	/**
+	 *
+	 * Adds the vault path to the quick-access config file
+	 *
+	 * @param target The mount point of the vault
+	 * @param displayName Caption of the vault link
+	 * @return A cleanup reference for vault link removal
+	 * @throws QuickAccessServiceException If the entry could not be added to the quick-access config file
+	 */
 	@Override
 	public QuickAccessEntry add(Path target, String displayName) throws QuickAccessServiceException {
 		try {
 			modifyLock.lock();
+			checkFileSize();
 			var entryAndConfig = addEntryToConfig(readConfig(), target, displayName);
 			persistConfig(entryAndConfig.config());
 			return entryAndConfig.entry();