From 59dba8a2e11e012d75f8cef99c43d702ea613589 Mon Sep 17 00:00:00 2001
From: Olivier 'reivilibre <olivier@librepush.net>
Date: Mon, 7 Feb 2022 23:21:20 +0000
Subject: [PATCH] Add support for unlink

---
 olivefs/src/filesystem.rs          | 41 ++++++++++++++++++++++++------
 olivefs/src/requester.rs           | 10 ++++++++
 olivefs_common/src/messages.rs     |  4 +++
 olivefsd/src/server/connections.rs | 12 ++++++++-
 olivefsd/src/server/file_access.rs | 16 ++++++++++++
 5 files changed, 74 insertions(+), 9 deletions(-)

diff --git a/olivefs/src/filesystem.rs b/olivefs/src/filesystem.rs
index 3e2c4c4..80d5491 100644
--- a/olivefs/src/filesystem.rs
+++ b/olivefs/src/filesystem.rs
@@ -281,10 +281,6 @@ impl Filesystem for OliveFilesystem {
 
                 match requester.mkdir(parent_vnode, name).await? {
                     DataResponse::Success(metadata) => {
-                        // Use the direct I/O flag so that we can be in control of when EOF happens
-                        // rather than letting the kernel assume the getattr size is valid
-                        // and caching up to that point.
-                        // We might wind up wanting to do our own buffering...
                         let file_attr: FileAttr = metadata.into();
                         // TODO generation should be generated client-side
                         reply.entry(&Duration::from_secs(5), &file_attr, 42);
@@ -303,11 +299,40 @@ impl Filesystem for OliveFilesystem {
 
     /// Remove a file.
     fn unlink(&mut self, _req: &Request<'_>, parent: u64, name: &OsStr, reply: ReplyEmpty) {
-        debug!(
-            "[Not Implemented] unlink(parent: {:#x?}, name: {:?})",
-            parent, name,
+        let name = if let Some(name) = name.to_str() {
+            name
+        } else {
+            // If we can't decode the filename, pretend it doesn't exist.
+            error!("unlink: Filename not convertible.");
+            reply.error(ENOENT);
+            return;
+        }
+            .to_owned();
+
+        let requester = self.requester.clone();
+
+        self.spawn_with_error_handler(
+            async move {
+                let parent_vnode = VnodeId(
+                    parent
+                        .try_into()
+                        .context("Converting u64 inode to u32 VnodeId.")?,
+                );
+
+                match requester.unlink(parent_vnode, name).await? {
+                    DataResponse::Success(()) => {
+                        reply.ok();
+                    }
+                    DataResponse::Error { code, message } => {
+                        warn!("unlink(parent: {:#x?}) failed: {:?}", parent_vnode, message);
+                        reply.error(code as c_int);
+                    }
+                }
+
+                Ok(())
+            },
+            "unlink",
         );
-        reply.error(ENOSYS);
     }
 
     /// Remove a directory.
diff --git a/olivefs/src/requester.rs b/olivefs/src/requester.rs
index f6cc257..00c747b 100644
--- a/olivefs/src/requester.rs
+++ b/olivefs/src/requester.rs
@@ -222,6 +222,16 @@ impl Requester {
             .await
     }
 
+    pub async fn unlink(
+        &self,
+        dir_vnode: VnodeId,
+        name: String,
+    ) -> anyhow::Result<DataResponse<()>> {
+        self.internal
+            .command(&DataCommand::Unlink { dir_vnode, name })
+            .await
+    }
+
     pub async fn open(&self, vnode: VnodeId, mode: OpenMode) -> anyhow::Result<DataResponse<u32>> {
         self.internal
             .command(&DataCommand::OpenFile { vnode, mode })
diff --git a/olivefs_common/src/messages.rs b/olivefs_common/src/messages.rs
index fe50bfd..8419324 100644
--- a/olivefs_common/src/messages.rs
+++ b/olivefs_common/src/messages.rs
@@ -95,6 +95,10 @@ pub enum DataCommand {
         dir_vnode: VnodeId,
         name: String,
     },
+    Unlink {
+        dir_vnode: VnodeId,
+        name: String,
+    },
 }
 
 #[derive(Serialize, Deserialize, Debug, Clone)]
diff --git a/olivefsd/src/server/connections.rs b/olivefsd/src/server/connections.rs
index ae7209b..cbe1f38 100644
--- a/olivefsd/src/server/connections.rs
+++ b/olivefsd/src/server/connections.rs
@@ -144,7 +144,17 @@ pub async fn handle_command_stream(
                         .await
                         .unwrap_or_else(error_to_response),
                 )
-                .await?;
+                    .await?;
+            }
+            DataCommand::Unlink { dir_vnode, name } => {
+                send_bare_message(
+                    &mut tx,
+                    &file_access
+                        .unlink(dir_vnode, name)
+                        .await
+                        .unwrap_or_else(error_to_response),
+                )
+                    .await?;
             }
         }
     }
diff --git a/olivefsd/src/server/file_access.rs b/olivefsd/src/server/file_access.rs
index eaf7060..9d1a325 100644
--- a/olivefsd/src/server/file_access.rs
+++ b/olivefsd/src/server/file_access.rs
@@ -762,4 +762,20 @@ impl FileAccess {
         let metadata = self.read_metadata(&target_path, vnode).await?;
         Ok(DataResponse::Success(metadata))
     }
+
+    /// Symlink safety:
+    ///   Makes an attempt to safely resolve symlinks, but there is a possibility of a TOCTOU race.
+    ///   TODO Revisit later for safety.
+    pub async fn unlink(
+        &self,
+        dir_vnode: VnodeId,
+        name: String,
+    ) -> anyhow::Result<DataResponse<()>> {
+        let parent_dir_resolved = self
+            .resolve_vnode_including_follow_symlinks_safely_best_effort(dir_vnode)
+            .await?;
+        let target_path = parent_dir_resolved.join(name);
+        tokio::fs::remove_file(&target_path).await?;
+        Ok(DataResponse::Success(()))
+    }
 }