Correct device renaming, save config via temp file (fixes #1059)

app/src/main/java/com/nutomic/syncthingandroid/service/Constants.java

@@ -46,6 +46,15 @@ public class Constants {
         return new File(context.getFilesDir(), CONFIG_FILE);
     }
 
+    /**
+     * File in the config folder we write to temporarily before renaming to CONFIG_FILE.
+     */
+    static final String CONFIG_TEMP_FILE = "config.xml.tmp";
+
+    public static File getConfigTempFile(Context context) {
+        return new File(context.getFilesDir(), CONFIG_TEMP_FILE);
+    }
+
     /**
      * Name of the public key file in the data directory.
      */

app/src/main/java/com/nutomic/syncthingandroid/service/SyncthingRunnable.java

@@ -55,6 +55,7 @@ public class SyncthingRunnable implements Runnable {
     @Inject NotificationHandler mNotificationHandler;
 
     public enum Command {
+        deviceid,           // Output the device ID to the command line.
         generate,           // Generate keys, a config file and immediately exit.
         main,               // Run the main Syncthing application.
         resetdatabase,      // Reset Syncthing's database
@@ -75,6 +76,9 @@ public class SyncthingRunnable implements Runnable {
         // Get preferences relevant to starting syncthing core.
         mUseRoot = mPreferences.getBoolean(Constants.PREF_USE_ROOT, false) && Shell.SU.available();
         switch (command) {
+            case deviceid:
+                mCommand = new String[]{ mSyncthingBinary.getPath(), "-home", mContext.getFilesDir().toString(), "--device-id" };
+                break;
             case generate:
                 mCommand = new String[]{ mSyncthingBinary.getPath(), "-generate", mContext.getFilesDir().toString(), "-logflags=0" };
                 break;
@@ -94,8 +98,13 @@ public class SyncthingRunnable implements Runnable {
 
     @Override
     public void run() {
+        run(false);
+    }
+
+    public String run(boolean returnStdOut) {
         trimLogFile();
         int ret;
+        String capturedStdOut = "";
         // Make sure Syncthing is executable
         try {
             ProcessBuilder pb = new ProcessBuilder("chmod", "500", mSyncthingBinary.getPath());
@@ -120,15 +129,36 @@ public class SyncthingRunnable implements Runnable {
 
             mSyncthing.set(process);
 
-            Thread lInfo = log(process.getInputStream(), Log.INFO, true);
+            Thread lInfo = null;
-            Thread lWarn = log(process.getErrorStream(), Log.WARN, true);
+            Thread lWarn = null;
+            if (returnStdOut) {
+                BufferedReader br = null;
+                try {
+                    br = new BufferedReader(new InputStreamReader(process.getInputStream(), Charsets.UTF_8));
+                    String line;
+                    while ((line = br.readLine()) != null) {
+                        Log.println(Log.INFO, TAG_NATIVE, line);
+                        capturedStdOut = capturedStdOut + line + "\n";
+                    }
+                } catch (IOException e) {
+                    Log.w(TAG, "Failed to read Syncthing's command line output", e);
+                } finally {
+                    if (br != null)
+                        br.close();
+                }
+            } else {
+                lInfo = log(process.getInputStream(), Log.INFO, true);
+                lWarn = log(process.getErrorStream(), Log.WARN, true);
+            }
 
             niceSyncthing();
 
             ret = process.waitFor();
             Log.i(TAG, "Syncthing exited with code " + ret);
             mSyncthing.set(null);
+            if (lInfo != null)
                 lInfo.join();
+            if (lWarn != null)
                 lWarn.join();
 
             switch (ret) {
@@ -157,6 +187,7 @@ public class SyncthingRunnable implements Runnable {
             if (process != null)
                 process.destroy();
         }
+        return capturedStdOut;
     }
 
     private void putCustomEnvironmentVariables(Map<String, String> environment, SharedPreferences sp) {
@@ -307,9 +338,9 @@ public class SyncthingRunnable implements Runnable {
      */
     private Thread log(final InputStream is, final int priority, final boolean saveLog) {
         Thread t = new Thread(() -> {
+            BufferedReader br = null;
             try {
-                InputStreamReader isr = new InputStreamReader(is, Charsets.UTF_8);
+                br = new BufferedReader(new InputStreamReader(is, Charsets.UTF_8));
-                BufferedReader br = new BufferedReader(isr);
                 String line;
                 while ((line = br.readLine()) != null) {
                     Log.println(priority, TAG_NATIVE, line);
@@ -321,6 +352,13 @@ public class SyncthingRunnable implements Runnable {
             } catch (IOException e) {
                 Log.w(TAG, "Failed to read Syncthing's command line output", e);
             }
+            if (br != null) {
+                try {
+                    br.close();
+                } catch (IOException e) {
+                    Log.w(TAG, "log: Failed to close bufferedReader", e);
+                }
+            }
         });
         t.start();
         return t;

app/src/main/java/com/nutomic/syncthingandroid/util/ConfigXml.java

@@ -19,11 +19,15 @@ import org.w3c.dom.Node;
 import org.w3c.dom.NodeList;
 import org.xml.sax.SAXException;
 
+import java.io.BufferedReader;
 import java.io.File;
+import java.io.InputStreamReader;
 import java.io.IOException;
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.util.Locale;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import javax.inject.Inject;
 import javax.xml.parsers.DocumentBuilder;
@@ -60,7 +64,7 @@ public class ConfigXml {
         boolean isFirstStart = !mConfigFile.exists();
         if (isFirstStart) {
             Log.i(TAG, "App started for the first time. Generating keys and config.");
-            generateKeysConfig(context);
+            new SyncthingRunnable(context, SyncthingRunnable.Command.generate).run();
         }
 
         readConfig();
@@ -68,10 +72,13 @@ public class ConfigXml {
         if (isFirstStart) {
             boolean changed = false;
 
-            /* Synthing devices */
+            Log.i(TAG, "Starting syncthing to retrieve local device id.");
-            changeLocalDeviceName();
+            String logOutput = new SyncthingRunnable(context, SyncthingRunnable.Command.deviceid).run(true);
-
+            String localDeviceID = logOutput.replace("\n", "");
-            /* Syncthing folders */
+            // Verify local device ID is correctly formatted.
+            if (localDeviceID.matches("^([A-Z0-9]{7}-){7}[A-Z0-9]{7}$")) {
+                changed = changeLocalDeviceName(localDeviceID) || changed;
+            }
             changed = changeDefaultFolder() || changed;
 
             // Save changes if we made any.
@@ -96,10 +103,6 @@ public class ConfigXml {
         Log.i(TAG, "Loaded Syncthing config file");
     }
 
-    private void generateKeysConfig(Context context) {
-        new SyncthingRunnable(context, SyncthingRunnable.Command.generate).run();
-    }
-
     public URL getWebGuiUrl() {
         try {
             return new URL("https://" + getGuiElement().getElementsByTagName("address").item(0).getTextContent());
@@ -271,17 +274,23 @@ public class ConfigXml {
      * Set model name as device name for Syncthing.
      *
      * We need to iterate through XML nodes manually, as mConfig.getDocumentElement() will also
-     * return nested elements inside folder element.
+     * return nested elements inside folder element. We have to check that we only rename the
+     * device corresponding to the local device ID.
+     * Returns if changes to the config have been made.
      */
-    private void changeLocalDeviceName() {
+    private boolean changeLocalDeviceName(String localDeviceID) {
         NodeList childNodes = mConfig.getDocumentElement().getChildNodes();
         for (int i = 0; i < childNodes.getLength(); i++) {
             Node node = childNodes.item(i);
             if (node.getNodeName().equals("device")) {
+                if (((Element) node).getAttribute("id").equals(localDeviceID)) {
+                    Log.i(TAG, "changeLocalDeviceName: Rename device ID " + localDeviceID + " to " + Build.MODEL);
                     ((Element) node).setAttribute("name", Build.MODEL);
+                    return true;
                 }
             }
-        saveChanges();
+        }
+        return false;
     }
 
     /**
@@ -313,15 +322,23 @@ public class ConfigXml {
             Log.w(TAG, "Failed to save updated config. Cannot change the owner of the config file.");
             return;
         }
+
+        Log.i(TAG, "Writing updated config file");
+        File mConfigTempFile = Constants.getConfigTempFile(mContext);
         try {
-            Log.i(TAG, "Writing updated config back to file");
             TransformerFactory transformerFactory = TransformerFactory.newInstance();
             Transformer transformer = transformerFactory.newTransformer();
             DOMSource domSource = new DOMSource(mConfig);
-            StreamResult streamResult = new StreamResult(mConfigFile);
+            StreamResult streamResult = new StreamResult(mConfigTempFile);
             transformer.transform(domSource, streamResult);
         } catch (TransformerException e) {
-            Log.w(TAG, "Failed to save updated config", e);
+            Log.w(TAG, "Failed to save temporary config file", e);
+            return;
+        }
+        try {
+            mConfigTempFile.renameTo(mConfigFile);
+        } catch (Exception e) {
+            Log.w(TAG, "Failed to rename temporary config file to original file");
         }
     }
 }