From: Simon Tatham <anakin@pobox.com>
Date: Fri, 5 Jan 2024 23:35:28 +0000 (+0000)
Subject: Write configuration files atomically.
X-Git-Url: http://www.chiark.greenend.org.uk/ucgi/~ian/git?a=commitdiff_plain;h=e0c887f3c04c21a2717dc9d85a9dc2d6de7aa0aa;p=mastodonochrome.git

Write configuration files atomically.

This is generally good practice, and clears away a prerequisite for
constantly updating an LDB.
---

diff --git a/Cargo.toml b/Cargo.toml
index 6f1af20..05b1d6f 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -19,6 +19,7 @@ serde = { version = "1.0.193", features = ["derive"] }
 serde_json = "1.0.108"
 strum = { version = "0.25.0", features = ["derive"] }
 sys-locale = "0.3.1"
+tempfile = "3.9.0"
 unicode-width = "0.1.5"
 
 [target.'cfg(unix)'.dependencies]
diff --git a/TODO.md b/TODO.md
index 7cf8d7e..6e511e2 100644
--- a/TODO.md
+++ b/TODO.md
@@ -38,10 +38,6 @@ view the first thing you haven't yet read in that feed. This means
 tracking what _is_ read during run time, and saving it to a file in
 the config directory.
 
-(Prerequisite: a means of atomically replacing that file with the new
-version each time you overwrite it, to prevent data loss. I think
-[`tempfile::NamedTempFile::persist`](https://docs.rs/tempfile/latest/tempfile/struct.NamedTempFile.html#method.persist) is probably the tool for that job.)
-
 Similarly, when the client automatically moves you into the Read
 Mentions field because a new mention has arrived, it should
 automatically jump the position to the first unread one.
@@ -242,8 +238,10 @@ dev instance in a VM, which is a common activity during development!)
 
 But I'm not sure. The docs for that method say you're required to
 provide a user token. But how did you get one without already logging
-in as an existing user? I don't quite understand what the flow is for
-this.
+in as an existing user? Perhaps I've misunderstood, and this API
+request is for _administrators_ to use, to create an account for a
+user who asked for one, if for some reason they don't want to use web
+registration?
 
 ### General search
 
diff --git a/src/config.rs b/src/config.rs
index 61895af..5c6cc29 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -1,10 +1,6 @@
-use std::path::PathBuf;
 use std::io::Write;
-
-use std::fs::OpenOptions;
-
-#[cfg(unix)]
-use std::os::unix::fs::OpenOptionsExt;
+use std::path::PathBuf;
+use tempfile::NamedTempFile;
 
 #[cfg(windows)]
 use std::str::FromStr;
@@ -98,20 +94,14 @@ impl ConfigLocation {
     {
         std::fs::create_dir_all(&self.dir)?;
 
-        // FIXME: we could do better here, by creating a file under a
-        // separate name and then atomically renaming it
-
-        let mut opts = OpenOptions::new();
-        opts.write(true);
-        opts.create_new(true);
-
-        // On Windows files are not-world-readable by default, but on
-        // Unix we must be careful about this
-        #[cfg(unix)]
-        opts.mode(0o600);
+        // NamedTempFile creates the file with restricted permissions,
+        // so we don't need to specify to be careful about that. (I
+        // think if we wanted to make the file _public_ we'd have to
+        // fchmod it or similar.)
 
-        let path = self.get_path(leaf);
-        opts.open(path)?
-            .write_all(contents.as_bytes())
+        let mut file = NamedTempFile::new_in(&self.dir)?;
+        file.write_all(contents.as_bytes())?;
+        file.persist(self.get_path(leaf))?;
+        Ok(())
     }
 }