From 361a24004095d65f0dca8218e71eab34c309f794 Mon Sep 17 00:00:00 2001
From: "taylor.smock" <taylor.smock@0c6e7542-c601-0410-84e7-c038aed88b3b>
Date: Mon, 12 Aug 2024 15:04:53 +0000
Subject: [PATCH] Fix #23802: Backup preferences files get overwritten by bad
 preference files

This validates a preferences file before copying/moving it to another file.
This should avoid cases where a file is partially written and then copied or
moved over a "good" preferences file.

git-svn-id: https://josm.openstreetmap.de/svn/trunk@19179 0c6e7542-c601-0410-84e7-c038aed88b3b
---
 .../openstreetmap/josm/data/Preferences.java  | 39 ++++++++++++++++++-
 1 file changed, 37 insertions(+), 2 deletions(-)

diff --git a/src/org/openstreetmap/josm/data/Preferences.java b/src/org/openstreetmap/josm/data/Preferences.java
index fcf37a9ea0f..888070f2e21 100644
--- a/src/org/openstreetmap/josm/data/Preferences.java
+++ b/src/org/openstreetmap/josm/data/Preferences.java
@@ -439,7 +439,7 @@ protected void save(File prefFile, Stream<Entry<String, Setting<?>>> settings, b
 
         // Backup old preferences if there are old preferences
         if (initSuccessful && prefFile.exists() && prefFile.length() > 0) {
-            Utils.copyFile(prefFile, backupFile);
+            checkFileValidity(prefFile, f -> Utils.copyFile(f, backupFile));
         }
 
         try (PreferencesWriter writer = new PreferencesWriter(
@@ -450,12 +450,33 @@ protected void save(File prefFile, Stream<Entry<String, Setting<?>>> settings, b
         }
 
         File tmpFile = new File(prefFile + "_tmp");
-        Files.move(tmpFile.toPath(), prefFile.toPath(), StandardCopyOption.REPLACE_EXISTING);
+        // Only replace the pref file if the _tmp file is valid
+        checkFileValidity(tmpFile, f -> Files.move(f.toPath(), prefFile.toPath(), StandardCopyOption.REPLACE_EXISTING));
 
         setCorrectPermissions(prefFile);
         setCorrectPermissions(backupFile);
     }
 
+    /**
+     * Ensure that a preferences file is "ok" before copying/moving it over another preferences file
+     * @param file The file to check
+     * @param consumer The consumer that will perform the copy/move action
+     * @throws IOException If there is an issue reading/writing the file
+     */
+    private static void checkFileValidity(File file, ThrowingConsumer<File, IOException> consumer) throws IOException {
+        try {
+            // But don't back up if the current preferences are invalid.
+            // The validations are expensive (~2/3 CPU, ~1/3 memory), but this isn't a "hot" method
+            PreferencesReader.validateXML(file);
+            PreferencesReader reader = new PreferencesReader(file, false);
+            reader.parse();
+            consumer.accept(file);
+        } catch (SAXException | XMLStreamException e) {
+            Logging.trace(e);
+            Logging.debug("Invalid preferences file (" + file + ") due to: " + e.getMessage());
+        }
+    }
+
     private static void setCorrectPermissions(File file) {
         if (!file.setReadable(false, false) && Logging.isTraceEnabled()) {
             Logging.trace(tr("Unable to set file non-readable {0}", file.getAbsolutePath()));
@@ -973,4 +994,18 @@ public final void enableSaveOnPut(boolean enable) {
             saveOnPut = enable;
         }
     }
+
+    /**
+     * A consumer that can throw an exception
+     * @param <T> The object type to accept
+     * @param <E> The throwable type
+     */
+    private interface ThrowingConsumer<T, E extends Throwable> {
+        /**
+         * Accept an object
+         * @param object The object to accept
+         * @throws E The exception that can be thrown
+         */
+        void accept(T object) throws E;
+    }
 }