index, op_store: write temporary files in destination directory

This should be safer, and should be okay since we ignore invalid file names when scanning the store directories. Fixes jj-vcs#5712
yuja · Feb 15, 2025 · 14937d2 · 14937d2
1 parent b1d70bb
commit 14937d2
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 6 deletions.
diff --git a/lib/src/default_index/store.rs b/lib/src/default_index/store.rs
@@ -305,10 +305,11 @@ impl DefaultIndexStore {
         index: &ReadonlyIndexSegment,
         op_id: &OperationId,
     ) -> io::Result<()> {
-        let mut temp_file = NamedTempFile::new_in(&self.dir)?;
+        let dir = self.operations_dir();
+        let mut temp_file = NamedTempFile::new_in(&dir)?;
         let file = temp_file.as_file_mut();
         file.write_all(index.name().as_bytes())?;
-        persist_content_addressed_temp_file(temp_file, self.operations_dir().join(op_id.hex()))?;
+        persist_content_addressed_temp_file(temp_file, dir.join(op_id.hex()))?;
         Ok(())
     }
 }

diff --git a/lib/src/local_backend.rs b/lib/src/local_backend.rs
@@ -195,6 +195,7 @@ impl Backend for LocalBackend {
         _path: &RepoPath,
         contents: &mut (dyn Read + Send),
     ) -> BackendResult<FileId> {
+        // TODO: Write temporary file in the destination directory (#5712)
         let temp_file = NamedTempFile::new_in(&self.path).map_err(to_other_err)?;
         let mut file = temp_file.as_file();
         let mut hasher = Blake2b512::new();
@@ -223,6 +224,7 @@ impl Backend for LocalBackend {
     }
 
     async fn write_symlink(&self, _path: &RepoPath, target: &str) -> BackendResult<SymlinkId> {
+        // TODO: Write temporary file in the destination directory (#5712)
         let mut temp_file = NamedTempFile::new_in(&self.path).map_err(to_other_err)?;
         temp_file
             .write_all(target.as_bytes())
@@ -245,6 +247,7 @@ impl Backend for LocalBackend {
     }
 
     async fn write_tree(&self, _path: &RepoPath, tree: &Tree) -> BackendResult<TreeId> {
+        // TODO: Write temporary file in the destination directory (#5712)
         let temp_file = NamedTempFile::new_in(&self.path).map_err(to_other_err)?;
 
         let proto = tree_to_proto(tree);
@@ -269,6 +272,7 @@ impl Backend for LocalBackend {
     }
 
     fn write_conflict(&self, _path: &RepoPath, conflict: &Conflict) -> BackendResult<ConflictId> {
+        // TODO: Write temporary file in the destination directory (#5712)
         let temp_file = NamedTempFile::new_in(&self.path).map_err(to_other_err)?;
 
         let proto = conflict_to_proto(conflict);
@@ -311,6 +315,7 @@ impl Backend for LocalBackend {
                 "Cannot write a commit with no parents".into(),
             ));
         }
+        // TODO: Write temporary file in the destination directory (#5712)
         let temp_file = NamedTempFile::new_in(&self.path).map_err(to_other_err)?;
 
         let mut proto = commit_to_proto(&commit);

diff --git a/lib/src/simple_op_store.rs b/lib/src/simple_op_store.rs
@@ -159,8 +159,9 @@ impl OpStore for SimpleOpStore {
     }
 
     fn write_view(&self, view: &View) -> OpStoreResult<ViewId> {
+        let dir = self.views_dir();
         let temp_file =
-            NamedTempFile::new_in(&self.path).map_err(|err| io_to_write_error(err, "view"))?;
+            NamedTempFile::new_in(&dir).map_err(|err| io_to_write_error(err, "view"))?;
 
         let proto = view_to_proto(view);
         temp_file
@@ -170,7 +171,7 @@ impl OpStore for SimpleOpStore {
 
         let id = ViewId::new(blake2b_hash(view).to_vec());
 
-        persist_content_addressed_temp_file(temp_file, self.views_dir().join(id.hex()))
+        persist_content_addressed_temp_file(temp_file, dir.join(id.hex()))
             .map_err(|err| io_to_write_error(err, "view"))?;
         Ok(id)
     }
@@ -200,8 +201,9 @@ impl OpStore for SimpleOpStore {
 
     fn write_operation(&self, operation: &Operation) -> OpStoreResult<OperationId> {
         assert!(!operation.parents.is_empty());
+        let dir = self.operations_dir();
         let temp_file =
-            NamedTempFile::new_in(&self.path).map_err(|err| io_to_write_error(err, "operation"))?;
+            NamedTempFile::new_in(&dir).map_err(|err| io_to_write_error(err, "operation"))?;
 
         let proto = operation_to_proto(operation);
         temp_file
@@ -211,7 +213,7 @@ impl OpStore for SimpleOpStore {
 
         let id = OperationId::new(blake2b_hash(operation).to_vec());
 
-        persist_content_addressed_temp_file(temp_file, self.operations_dir().join(id.hex()))
+        persist_content_addressed_temp_file(temp_file, dir.join(id.hex()))
             .map_err(|err| io_to_write_error(err, "operation"))?;
         Ok(id)
     }