Skip to content

Commit

Permalink
Fix #23802: Backup preferences files get overwritten by bad preferenc…
Browse files Browse the repository at this point in the history
…e 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
  • Loading branch information
taylor.smock committed Aug 12, 2024
1 parent 740ce65 commit 361a240
Showing 1 changed file with 37 additions and 2 deletions.
39 changes: 37 additions & 2 deletions src/org/openstreetmap/josm/data/Preferences.java
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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()));
Expand Down Expand Up @@ -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;
}
}

0 comments on commit 361a240

Please sign in to comment.