Skip to content

Commit 48d0261

Browse files
jrauberjrauber
authored
Refactors Dolphin quick access integration for robustness
* Refactors Dolphin quick access integration for robustness Refactor the Dolphin quick access integration to use DOM manipulation for adding and removing bookmarks in the XBEL file. This change aims to improve the reliability and correctness of bookmark management by handling stale entries. Changes - Replaces string-based manipulation of the XBEL file with DOM manipulation using org.w3c.dom. - Introduces methods for loading, validating, and writing the XBEL document. - Implements bookmark creation and removal using XPath queries and DOM operations. - Adds more test cases to DolphinPlacesTest to cover adding and removing bookmarks under different scenarios.
1 parent daef0f2 commit 48d0261

8 files changed

Lines changed: 479 additions & 64 deletions

File tree

‎src/main/java/module-info.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
requires org.purejava.appindicator;
1919
requires org.purejava.kwallet;
2020
requires de.swiesend.secretservice;
21+
requires java.xml;
2122

2223
provides AutoStartProvider with FreedesktopAutoStartService;
2324
provides KeychainAccessProvider with GnomeKeyringKeychainAccess, KDEWalletKeychainAccess;

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

Lines changed: 177 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,40 @@
66
import org.cryptomator.integrations.common.Priority;
77
import org.cryptomator.integrations.quickaccess.QuickAccessService;
88
import org.cryptomator.integrations.quickaccess.QuickAccessServiceException;
9+
import org.slf4j.Logger;
10+
import org.slf4j.LoggerFactory;
11+
import org.w3c.dom.DOMException;
12+
import org.w3c.dom.Document;
13+
import org.w3c.dom.Node;
14+
import org.w3c.dom.NodeList;
15+
import org.xml.sax.InputSource;
916
import org.xml.sax.SAXException;
1017

1118
import javax.xml.XMLConstants;
19+
import javax.xml.namespace.QName;
20+
import javax.xml.parsers.DocumentBuilder;
21+
import javax.xml.parsers.DocumentBuilderFactory;
22+
import javax.xml.parsers.ParserConfigurationException;
23+
import javax.xml.transform.OutputKeys;
1224
import javax.xml.transform.Source;
25+
import javax.xml.transform.Transformer;
26+
import javax.xml.transform.TransformerException;
27+
import javax.xml.transform.TransformerFactory;
28+
import javax.xml.transform.dom.DOMSource;
29+
import javax.xml.transform.stream.StreamResult;
1330
import javax.xml.transform.stream.StreamSource;
1431
import javax.xml.validation.SchemaFactory;
1532
import javax.xml.validation.Validator;
33+
import javax.xml.xpath.XPathConstants;
34+
import javax.xml.xpath.XPathExpressionException;
35+
import javax.xml.xpath.XPathFactory;
36+
import java.io.ByteArrayInputStream;
1637
import java.io.IOException;
1738
import java.io.StringReader;
39+
import java.io.StringWriter;
40+
import java.nio.charset.StandardCharsets;
1841
import java.nio.file.Files;
1942
import java.nio.file.Path;
20-
import java.util.List;
2143
import java.util.UUID;
2244

2345
/**
@@ -29,20 +51,11 @@
2951
@Priority(90)
3052
public class DolphinPlaces extends FileConfiguredQuickAccess implements QuickAccessService {
3153

54+
private static final Logger LOG = LoggerFactory.getLogger(DolphinPlaces.class);
55+
56+
private static final String XBEL_NAMESPACE = "http://www.freedesktop.org/standards/desktop-bookmarks";
3257
private static final int MAX_FILE_SIZE = 1 << 20; //1MiB, xml is quite verbose
3358
private static final Path PLACES_FILE = Path.of(System.getProperty("user.home"), ".local/share/user-places.xbel");
34-
private static final String ENTRY_TEMPLATE = """
35-
<bookmark href=\"%s\">
36-
<title>%s</title>
37-
<info>
38-
<metadata owner=\"http://freedesktop.org\">
39-
<bookmark:icon name="drive-harddisk-encrypted"/>
40-
</metadata>
41-
<metadata owner=\"https://cryptomator.org\">
42-
<id>%s</id>
43-
</metadata>
44-
</info>
45-
</bookmark>""";
4659

4760
private static final Validator XML_VALIDATOR;
4861

@@ -61,29 +74,157 @@ public DolphinPlaces() {
6174
super(PLACES_FILE, MAX_FILE_SIZE);
6275
}
6376

77+
public DolphinPlaces(Path configFilePath) {
78+
super(configFilePath, MAX_FILE_SIZE);
79+
}
80+
6481
@Override
6582
EntryAndConfig addEntryToConfig(String config, Path target, String displayName) throws QuickAccessServiceException {
6683
try {
67-
String id = UUID.randomUUID().toString();
68-
//validate
84+
var id = UUID.randomUUID().toString();
85+
LOG.trace("Adding bookmark for target: '{}', displayName: '{}', id: '{}'", target, displayName, id);
6986
XML_VALIDATOR.validate(new StreamSource(new StringReader(config)));
70-
// modify
71-
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
72-
var adjustedConfig = config.substring(0, insertIndex) //
73-
+ "\n" //
74-
+ ENTRY_TEMPLATE.formatted(target.toUri(), escapeXML(displayName), id).indent(1) //
75-
+ "\n" //
76-
+ config.substring(insertIndex);
77-
return new EntryAndConfig(new DolphinPlacesEntry(id), adjustedConfig);
78-
} catch (SAXException | IOException e) {
79-
throw new QuickAccessServiceException("Adding entry to KDE places file failed.", e);
87+
var xmlDocument = loadXmlDocument(config);
88+
var nodeList = extractBookmarksByPath(target, xmlDocument);
89+
removeStaleBookmarks(nodeList);
90+
createBookmark(target, displayName, id, xmlDocument);
91+
var changedConfig = documentToString(xmlDocument);
92+
XML_VALIDATOR.validate(new StreamSource(new StringReader(changedConfig)));
93+
return new EntryAndConfig(new DolphinPlacesEntry(id), changedConfig);
94+
} catch (SAXException e) {
95+
throw new QuickAccessServiceException("Invalid structure in xbel bookmark file", e);
96+
} catch (IOException e) {
97+
throw new QuickAccessServiceException("Failed reading/writing the xbel bookmark file", e);
8098
}
8199
}
82100

83-
private String escapeXML(String s) {
84-
return s.replace("&","&amp;") //
85-
.replace("<","&lt;") //
86-
.replace(">","&gt;");
101+
private void removeStaleBookmarks(NodeList nodeList) {
102+
for (int i = nodeList.getLength() - 1; i >= 0; i--) {
103+
Node node = nodeList.item(i);
104+
node.getParentNode().removeChild(node);
105+
}
106+
}
107+
108+
private NodeList extractBookmarksByPath(Path target, Document xmlDocument) throws QuickAccessServiceException {
109+
try {
110+
var xpathFactory = XPathFactory.newInstance();
111+
var xpath = xpathFactory.newXPath();
112+
xpath.setXPathVariableResolver(v -> {
113+
if (v.equals(new QName("uri"))) {
114+
return target.toUri().toString();
115+
}
116+
throw new IllegalArgumentException();
117+
});
118+
var expression = "/xbel/bookmark[info/metadata[@owner='https://cryptomator.org']][@href=$uri]";
119+
return (NodeList) xpath.compile(expression).evaluate(xmlDocument, XPathConstants.NODESET);
120+
} catch (XPathExpressionException xee) {
121+
throw new QuickAccessServiceException("Invalid XPath expression", xee);
122+
}
123+
}
124+
125+
private NodeList extractBookmarksById(String id, Document xmlDocument) throws QuickAccessServiceException {
126+
try {
127+
var xpathFactory = XPathFactory.newInstance();
128+
var xpath = xpathFactory.newXPath();
129+
xpath.setXPathVariableResolver(v -> {
130+
if (v.equals(new QName("id"))) {
131+
return id;
132+
}
133+
throw new IllegalArgumentException();
134+
});
135+
var expression = "/xbel/bookmark[info/metadata[@owner='https://cryptomator.org']][info/metadata/id[text()=$id]]";
136+
return (NodeList) xpath.compile(expression).evaluate(xmlDocument, XPathConstants.NODESET);
137+
} catch (XPathExpressionException xee) {
138+
throw new QuickAccessServiceException("Invalid XPath expression", xee);
139+
}
140+
}
141+
142+
private Document loadXmlDocument(String config) throws QuickAccessServiceException {
143+
try {
144+
var builderFactory = DocumentBuilderFactory.newInstance();
145+
builderFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
146+
builderFactory.setXIncludeAware(false);
147+
builderFactory.setExpandEntityReferences(false);
148+
builderFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
149+
builderFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");
150+
builderFactory.setNamespaceAware(true);
151+
DocumentBuilder builder = builderFactory.newDocumentBuilder();
152+
// Prevent external entities from being resolved
153+
builder.setEntityResolver((publicId, systemId) -> new InputSource(new StringReader("")));
154+
return builder.parse(new ByteArrayInputStream(config.getBytes(StandardCharsets.UTF_8)));
155+
} catch (IOException | SAXException | ParserConfigurationException e) {
156+
throw new QuickAccessServiceException("Error while loading xml file", e);
157+
}
158+
}
159+
160+
private String documentToString(Document xmlDocument) throws QuickAccessServiceException {
161+
try {
162+
var buf = new StringWriter();
163+
Transformer transformer = TransformerFactory.newInstance().newTransformer();
164+
transformer.setOutputProperty(OutputKeys.DOCTYPE_PUBLIC, "");
165+
transformer.setOutputProperty(OutputKeys.DOCTYPE_SYSTEM, "");
166+
transformer.setOutputProperty(OutputKeys.OMIT_XML_DECLARATION, "no");
167+
transformer.setOutputProperty(OutputKeys.INDENT, "yes");
168+
transformer.setOutputProperty(OutputKeys.ENCODING, StandardCharsets.UTF_8.name());
169+
transformer.transform(new DOMSource(xmlDocument), new StreamResult(buf));
170+
var content = buf.toString();
171+
content = content.replaceFirst("\\s*standalone=\"(yes|no)\"", "");
172+
content = content.replaceFirst("<!DOCTYPE xbel PUBLIC \"\" \"\">","<!DOCTYPE xbel>");
173+
return content;
174+
} catch (TransformerException e) {
175+
throw new QuickAccessServiceException("Error while serializing document to string", e);
176+
}
177+
}
178+
179+
/**
180+
*
181+
* Adds a xml bookmark element to the specified xml document
182+
*
183+
* <pre>{@code
184+
* <bookmark href="file:///home/someuser/folder1/">
185+
* <title>integrations-linux</title>
186+
* <info>
187+
* <metadata owner="http://freedesktop.org">
188+
* <bookmark:icon name="drive-harddisk-encrypted"/>
189+
* </metadata>
190+
* <metadata owner="https://cryptomator.org">
191+
* <id>sldkf-sadf-sadf-sadf</id>
192+
* </metadata>
193+
* </info>
194+
* </bookmark>
195+
* }</pre>
196+
*
197+
* @param target The mount point of the vault
198+
* @param displayName Caption of the vault link in dolphin
199+
* @param xmlDocument The xbel document to which the bookmark should be added
200+
*
201+
* @throws QuickAccessServiceException if the bookmark could not be created
202+
*/
203+
private void createBookmark(Path target, String displayName, String id, Document xmlDocument) throws QuickAccessServiceException {
204+
try {
205+
var bookmark = xmlDocument.createElement("bookmark");
206+
var title = xmlDocument.createElement("title");
207+
var info = xmlDocument.createElement("info");
208+
var metadataBookmark = xmlDocument.createElement("metadata");
209+
var metadataOwner = xmlDocument.createElement("metadata");
210+
var bookmarkIcon = xmlDocument.createElementNS(XBEL_NAMESPACE, "bookmark:icon");
211+
var idElem = xmlDocument.createElement("id");
212+
bookmark.setAttribute("href", target.toUri().toString());
213+
title.setTextContent(displayName);
214+
bookmark.appendChild(title);
215+
bookmark.appendChild(info);
216+
info.appendChild(metadataBookmark);
217+
info.appendChild(metadataOwner);
218+
metadataBookmark.appendChild(bookmarkIcon);
219+
metadataOwner.appendChild(idElem);
220+
metadataBookmark.setAttribute("owner", "http://freedesktop.org");
221+
bookmarkIcon.setAttribute("name","drive-harddisk-encrypted");
222+
metadataOwner.setAttribute("owner", "https://cryptomator.org");
223+
idElem.setTextContent(id);
224+
xmlDocument.getDocumentElement().appendChild(bookmark);
225+
} catch (DOMException | IllegalArgumentException e) {
226+
throw new QuickAccessServiceException("Error while creating bookmark for target: " + target, e);
227+
}
87228
}
88229

89230
private class DolphinPlacesEntry extends FileConfiguredQuickAccessEntry implements QuickAccessEntry {
@@ -97,46 +238,20 @@ private class DolphinPlacesEntry extends FileConfiguredQuickAccessEntry implemen
97238
@Override
98239
public String removeEntryFromConfig(String config) throws QuickAccessServiceException {
99240
try {
100-
int idIndex = config.lastIndexOf(id);
101-
if (idIndex == -1) {
102-
return config; //assume someone has removed our entry, nothing to do
103-
}
104-
//validate
105-
XML_VALIDATOR.validate(new StreamSource(new StringReader(config)));
106-
//modify
107-
int openingTagIndex = indexOfEntryOpeningTag(config, idIndex);
108-
var contentToWrite1 = config.substring(0, openingTagIndex).stripTrailing();
109-
110-
int closingTagEndIndex = config.indexOf('>', config.indexOf("</bookmark", idIndex));
111-
var part2Tmp = config.substring(closingTagEndIndex + 1).split("\\A\\v+", 2); //removing leading vertical whitespaces, but no indentation
112-
var contentToWrite2 = part2Tmp[part2Tmp.length - 1];
113-
114-
return contentToWrite1 + "\n" + contentToWrite2;
241+
var xmlDocument = loadXmlDocument(config);
242+
var nodeList = extractBookmarksById(id, xmlDocument);
243+
removeStaleBookmarks(nodeList);
244+
var changedConfig = documentToString(xmlDocument);
245+
XML_VALIDATOR.validate(new StreamSource(new StringReader(changedConfig)));
246+
return changedConfig;
115247
} catch (IOException | SAXException | IllegalStateException e) {
116248
throw new QuickAccessServiceException("Removing entry from KDE places file failed.", e);
117249
}
118250
}
119-
120-
/**
121-
* Returns the start index (inclusive) of the {@link DolphinPlaces#ENTRY_TEMPLATE} entry
122-
* @param placesContent the content of the XBEL places file
123-
* @param idIndex start index (inclusive) of the entrys id tag value
124-
* @return start index of the first bookmark tag, searching backwards from idIndex
125-
*/
126-
private int indexOfEntryOpeningTag(String placesContent, int idIndex) {
127-
var xmlWhitespaceChars = List.of(' ', '\t', '\n');
128-
for (char c : xmlWhitespaceChars) {
129-
int idx = placesContent.lastIndexOf("<bookmark" + c, idIndex); //with the whitespace we ensure, that no tags starting with "bookmark" (e.g. bookmarkz) are selected
130-
if (idx != -1) {
131-
return idx;
132-
}
133-
}
134-
throw new IllegalStateException("Found entry id " + id + " in " + PLACES_FILE + ", but it is not a child of <bookmark> tag.");
135-
}
136251
}
137252

138253
@CheckAvailability
139254
public static boolean isSupported() {
140255
return Files.exists(PLACES_FILE);
141256
}
142-
}
257+
}

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,20 @@ abstract class FileConfiguredQuickAccess implements QuickAccessService {
3131
Runtime.getRuntime().addShutdownHook(new Thread(this::cleanup));
3232
}
3333

34+
/**
35+
*
36+
* Adds the vault path to the quick-access config file
37+
*
38+
* @param target The mount point of the vault
39+
* @param displayName Caption of the vault link
40+
* @return A cleanup reference for vault link removal
41+
* @throws QuickAccessServiceException If the entry could not be added to the quick-access config file
42+
*/
3443
@Override
3544
public QuickAccessEntry add(Path target, String displayName) throws QuickAccessServiceException {
3645
try {
3746
modifyLock.lock();
47+
checkFileSize();
3848
var entryAndConfig = addEntryToConfig(readConfig(), target, displayName);
3949
persistConfig(entryAndConfig.config());
4050
return entryAndConfig.entry();

0 commit comments

Comments
 (0)