Use less risky code for /tmp handling in I2P initscript and systemd unit file

[zzz]

​https://labs.riseup.net/code/issues/10890

[zzz]

I'm not sure why we specify the temp dir at all. It defaults to /tmp, and I2PAppContext creates a 700 dir i2p-XXXXXXXX.tmp under that. All users of the temp dir inside i2p code should use I2PAppContext.getTempDir(), which for non-service instances is /tmp/i2p-XXXXXXXX.tmp/, and for the service instances, is /tmp/i2p-daemon/i2p-XXXXXXXX.tmp/ because of the override in the init scripts.

If any files were placed directly in /tmp/i2p-daemon/ by our code, that would be more of a problem.

We'll need to research the mtn history for why there's a temp dir specified in the systemd and non-systemd scripts, but it seems like the easiest fix would be to remove it, and bring everything "up" a level.

[zzz]

@mhatta would this fix it?

--- debian/i2p.init	ea8bda5cf970d5b3bf38839447ae1e5a542e7f53
+++ debian/i2p.init	218a1a105579ec730a06e914688011c58cc4bf78
@@ -25,7 +25,7 @@ I2P="/usr/share/i2p"
 PIDFILE="$RUN/$NAME.pid"
 JVMPIDFILE="$RUN/routerjvm.pid"
 I2P="/usr/share/i2p"
-I2PTEMP="/tmp/${NAME}-daemon"
+I2PTEMP=`mktmp -d -p ${NAME}-daemon.XXXXXXXXXX`
 WRAPPERLOG="/var/log/i2p/wrapper.log"
 
 # Don't touch these, edit /etc/default/i2p

[Masayuki Hatta]

I'm not sure why I2PTEMP is specified separately (can it simply use the same /tmp/i2p-XXXXXXXX.tmp?), but anyway the use of mktemp should be a good thing.

[zzz]

oops. several problems with the patch in comment 4, which I obviously didn't test:

• it's mktemp, not mktmp
• it must be —tmpdir not -p (the last arg is the template, not the arg for -p)
• the whole thing won't work, we can't make a new directory every time you run the init script (for example when the arg is stop or status)

So that won't work at all.

Comment 3 above has the main issues. The i2p.dir.temp system property is not i2p's temp dir, but its PARENT. I2PAppContext creates a unique temp dir inside it. The only reason for i2p.init to create, specify, and delete a temp dir is if it must put its own files in there.

A review of the checkin history shows that it did used to put its own files there, but does not now. Back in 2011, kytv did have a "anchor file" in there (see ​https://wrapper.tanukisoftware.com/doc/english/prop-anchorfile.html ) but deleted it within a couple weeks, as it wasn't being used. Similarly, the pid files used to be in $I2PTEMP, but in late 2011 he moved them to /var/run/i2p .

So an inspection of i2p.init today reveals no files placed inside there by i2p.init. The sole content is the directory created there by I2PAppContext. Therefore, we can remove all $I2PTEMP references in i2p.init. Additionally, The wrapper additional params should be consecutive (not 1 10 11 12).

So that leads me to this patch, which I haven't tested either:

#
# old_revision [effa099b40ff593227c59707a33922176dc775b1]
#
# patch "debian/i2p.init"
#  from [ea8bda5cf970d5b3bf38839447ae1e5a542e7f53]
#    to [0294d721997794ea80e5aee7d55001056fe70a1f]
#
============================================================
--- debian/i2p.init	ea8bda5cf970d5b3bf38839447ae1e5a542e7f53
+++ debian/i2p.init	0294d721997794ea80e5aee7d55001056fe70a1f
@@ -25,7 +25,6 @@ I2P="/usr/share/i2p"
 PIDFILE="$RUN/$NAME.pid"
 JVMPIDFILE="$RUN/routerjvm.pid"
 I2P="/usr/share/i2p"
-I2PTEMP="/tmp/${NAME}-daemon"
 WRAPPERLOG="/var/log/i2p/wrapper.log"
 
 # Don't touch these, edit /etc/default/i2p
@@ -35,9 +34,8 @@ I2P_ARGS="/etc/i2p/wrapper.config \
 
 I2P_ARGS="/etc/i2p/wrapper.config \
  wrapper.java.additional.1=-DloggerFilenameOverride=/var/log/i2p/log-router-@.txt \
- wrapper.java.additional.10=-Dwrapper.logfile=$WRAPPERLOG \
- wrapper.java.additional.11=-Di2p.dir.pid=$RUN \
- wrapper.java.additional.12=-Di2p.dir.temp=$I2PTEMP \
+ wrapper.java.additional.2=-Dwrapper.logfile=$WRAPPERLOG \
+ wrapper.java.additional.3=-Di2p.dir.pid=$RUN \
  wrapper.logfile=$WRAPPERLOG \
  wrapper.pidfile=$PIDFILE \
  wrapper.java.pidfile=$JVMPIDFILE \
@@ -95,7 +93,6 @@ do_start()
     start-stop-daemon --start --quiet --pidfile $PIDFILE --exec $DAEMON --test > /dev/null 2>&1 \
         || return 1
     [ -d $RUN ] || mkdir $RUN > /dev/null 2>&1
-    [ -d $I2PTEMP ] || mkdir $I2PTEMP > /dev/null 2>&1
     if [ -r $PIDFILE ]; then
         PID="$(cat ${PIDFILE})"
         if ! kill -0 $PID > /dev/null 2>&1; then
@@ -106,7 +103,7 @@ do_start()
                 return 1
         fi
     fi
-    chown -Rf $I2PUSER:$I2PUSER  $I2PTEMP $RUN > /dev/null 2>&1
+    chown -Rf $I2PUSER:$I2PUSER  $RUN > /dev/null 2>&1
     chown -f -R $I2PUSER:i2psvc /var/log/$NAME > /dev/null 2>&1
     if [ "$USE_AA" = "yes" ] && \
        [ -x /usr/sbin/aa-status ] && \
@@ -141,7 +138,6 @@ do_stop()
 
     start-stop-daemon --stop --quiet --oknodo --retry=0/60/KILL/20 --exec $DAEMON
 
-    rm -rf "$I2PTEMP" > /dev/null 2>&1
     [ -d "$RUN" ] && rm -f "$RUN/*" > /dev/null 2>&1
     [ -d "$RUN" ] && rmdir "$RUN" > /dev/null 2>&1
 }

[zzz]

Milestone set to 39 as requested by mhatta.
I still haven't tested the above patch.
mhatta please test and report results.