From b340f4a17c05b6ca61876a17a85901d0a5cd922c Mon Sep 17 00:00:00 2001
From: zzz <zzz@mail.i2p>
Date: Fri, 12 May 2017 17:16:25 +0000
Subject: [PATCH] i2psnark: Better handling of read-only i2psnark dir (ticket
#1990) Prevent add/create/remove/delete More handling of RuntimeException via
Snark.fatal() in SnarkManager
---
.../java/src/org/klomp/snark/Snark.java | 7 +++-
.../src/org/klomp/snark/SnarkManager.java | 41 ++++++++++++-------
.../org/klomp/snark/web/I2PSnarkServlet.java | 41 +++++++++++++++----
3 files changed, 66 insertions(+), 23 deletions(-)
diff --git a/apps/i2psnark/java/src/org/klomp/snark/Snark.java b/apps/i2psnark/java/src/org/klomp/snark/Snark.java
index 203f601957..2a43b6cebb 100644
--- a/apps/i2psnark/java/src/org/klomp/snark/Snark.java
+++ b/apps/i2psnark/java/src/org/klomp/snark/Snark.java
@@ -1243,8 +1243,13 @@ public class Snark
} catch (IOException ioe) {
if (storage != null) {
try { storage.close(); } catch (IOException ioee) {}
+ // clear storage, we have a mess if we have non-null storage and null metainfo,
+ // as on restart, Storage.reopen() will throw an ioe
+ storage = null;
}
- fatal("Could not check or create storage", ioe);
+ // TODO we're still in an inconsistent state, won't work if restarted
+ // (PeerState "disconnecting seed that connects to seeds"
+ fatal("Could not create data files", ioe);
}
}
diff --git a/apps/i2psnark/java/src/org/klomp/snark/SnarkManager.java b/apps/i2psnark/java/src/org/klomp/snark/SnarkManager.java
index cc379357e1..77964cb6b4 100644
--- a/apps/i2psnark/java/src/org/klomp/snark/SnarkManager.java
+++ b/apps/i2psnark/java/src/org/klomp/snark/SnarkManager.java
@@ -731,7 +731,7 @@ public class SnarkManager implements CompleteListener, ClientApp {
return new SHA1Hash(ih);
}
- /** null to set initial defaults */
+ /** @param filename null to set initial defaults */
public void loadConfig(String filename) {
synchronized(_configLock) {
locked_loadConfig(filename);
@@ -896,7 +896,14 @@ public class SnarkManager implements CompleteListener, ClientApp {
_util.setRatingsEnabled(Boolean.parseBoolean(_config.getProperty(PROP_RATINGS, "true")));
_util.setCommentsEnabled(Boolean.parseBoolean(_config.getProperty(PROP_COMMENTS, "true")));
_util.setCommentsName(_config.getProperty(PROP_COMMENTS_NAME, ""));
- getDataDir().mkdirs();
+ File dd = getDataDir();
+ if (dd.isDirectory()) {
+ if (!dd.canWrite())
+ addMessage(_t("No write permissions for data directory") + ": " + dd);
+ } else {
+ if (!dd.mkdirs())
+ addMessage(_t("Data directory cannot be created") + ": " + dd);
+ }
initTrackerMap();
}
@@ -1031,6 +1038,8 @@ public class SnarkManager implements CompleteListener, ClientApp {
} else if (!dd.canRead()) {
addMessage(_t("Unreadable") + ": " + dataDir);
} else {
+ if (!dd.canWrite())
+ addMessage(_t("No write permissions for data directory") + ": " + dataDir);
changed = true;
interruptMonitor = true;
_config.setProperty(PROP_DIR, dataDir);
@@ -2640,6 +2649,7 @@ public class SnarkManager implements CompleteListener, ClientApp {
/**
* If not connected, thread it, otherwise inline
+ * @throws RuntimeException via Snark.fatal()
* @since 0.9.1
*/
public void startTorrent(byte[] infoHash) {
@@ -2654,6 +2664,7 @@ public class SnarkManager implements CompleteListener, ClientApp {
/**
* If not connected, thread it, otherwise inline
+ * @throws RuntimeException via Snark.fatal()
* @since 0.9.23
*/
public void startTorrent(Snark snark) {
@@ -2701,17 +2712,14 @@ public class SnarkManager implements CompleteListener, ClientApp {
private final Snark snark;
public ThreadedStarter(Snark s) { snark = s; }
public void run() {
- try {
- run2();
- } catch (RuntimeException e) {
- _log.error("Error starting", e);
- }
- }
-
- private void run2() {
if (snark != null) {
- if (snark.isStopped())
- snark.startTorrent();
+ if (snark.isStopped()) {
+ try {
+ snark.startTorrent();
+ } catch (RuntimeException re) {
+ // Snark.fatal() will log and call fatal() here for user message before throwing
+ }
+ }
} else {
startAll();
}
@@ -2724,8 +2732,13 @@ public class SnarkManager implements CompleteListener, ClientApp {
*/
private void startAll() {
for (Snark snark : _snarks.values()) {
- if (snark.isStopped())
- snark.startTorrent();
+ if (snark.isStopped()) {
+ try {
+ snark.startTorrent();
+ } catch (RuntimeException re) {
+ // Snark.fatal() will log and call fatal() here for user message before throwing
+ }
+ }
}
}
diff --git a/apps/i2psnark/java/src/org/klomp/snark/web/I2PSnarkServlet.java b/apps/i2psnark/java/src/org/klomp/snark/web/I2PSnarkServlet.java
index 4a73cb27bf..1612ff2fab 100644
--- a/apps/i2psnark/java/src/org/klomp/snark/web/I2PSnarkServlet.java
+++ b/apps/i2psnark/java/src/org/klomp/snark/web/I2PSnarkServlet.java
@@ -987,6 +987,11 @@ public class I2PSnarkServlet extends BasicServlet {
}
}
}
+ File dd = _manager.getDataDir();
+ if (!dd.canWrite()) {
+ _manager.addMessage(_t("No write permissions for data directory") + ": " + dd);
+ return;
+ }
if (newURL.startsWith("http://")) {
FetchAndAdd fetch = new FetchAndAdd(_context, _manager, newURL, dir);
_manager.addDownloader(fetch);
@@ -1045,12 +1050,18 @@ public class I2PSnarkServlet extends BasicServlet {
_manager.addMessage(_t("Magnet deleted: {0}", name));
return;
}
- _manager.stopTorrent(snark, true);
- // should we delete the torrent file?
- // yeah, need to, otherwise it'll get autoadded again (at the moment
File f = new File(name);
- f.delete();
- _manager.addMessage(_t("Torrent file deleted: {0}", f.getAbsolutePath()));
+ File dd = _manager.getDataDir();
+ boolean canDelete = dd.canWrite() || !f.exists();
+ _manager.stopTorrent(snark, canDelete);
+ // TODO race here with the DirMonitor, could get re-added
+ if (f.delete()) {
+ _manager.addMessage(_t("Torrent file deleted: {0}", f.getAbsolutePath()));
+ } else if (f.exists()) {
+ if (!canDelete)
+ _manager.addMessage(_t("No write permissions for data directory") + ": " + dd);
+ _manager.addMessage(_t("Torrent file could not be deleted: {0}", f.getAbsolutePath()));
+ }
break;
}
}
@@ -1074,10 +1085,19 @@ public class I2PSnarkServlet extends BasicServlet {
_manager.addMessage(_t("Magnet deleted: {0}", name));
return;
}
- _manager.stopTorrent(snark, true);
File f = new File(name);
- f.delete();
- _manager.addMessage(_t("Torrent file deleted: {0}", f.getAbsolutePath()));
+ File dd = _manager.getDataDir();
+ boolean canDelete = dd.canWrite() || !f.exists();
+ _manager.stopTorrent(snark, canDelete);
+ // TODO race here with the DirMonitor, could get re-added
+ if (f.delete()) {
+ _manager.addMessage(_t("Torrent file deleted: {0}", f.getAbsolutePath()));
+ } else if (f.exists()) {
+ if (!canDelete)
+ _manager.addMessage(_t("No write permissions for data directory") + ": " + dd);
+ _manager.addMessage(_t("Torrent file could not be deleted: {0}", f.getAbsolutePath()));
+ return;
+ }
Storage storage = snark.getStorage();
if (storage == null)
break;
@@ -1178,6 +1198,11 @@ public class I2PSnarkServlet extends BasicServlet {
// announceURL = announceURLOther;
if (baseFile.exists()) {
+ File dd = _manager.getDataDir();
+ if (!dd.canWrite()) {
+ _manager.addMessage(_t("No write permissions for data directory") + ": " + dd);
+ return;
+ }
String torrentName = baseFile.getName();
if (torrentName.toLowerCase(Locale.US).endsWith(".torrent")) {
_manager.addMessage(_t("Cannot add a torrent ending in \".torrent\": {0}", baseFile.getAbsolutePath()));
--
GitLab