Answer a refused create or delete with a conflict instead of a missing folder

src/control.rs

@@ -21,7 +21,8 @@ pub fn outcome(code: u16) -> Result<(), ErrorCode> {
   match code {
     200..=399 => Ok(()),
     // 530 is a refused login and 550 covers both a missing file and a denied
-    // one; the server does not distinguish them, so neither does this.
+    // one; the server does not distinguish them, so neither does this. Where
+    // the command itself narrows what a 550 can mean, `refusal` does better.
     530 | 532 | 553 => Err(ErrorCode::Denied),
     550 => Err(ErrorCode::NotFound),
     450 | 452 | 552 => Err(ErrorCode::Conflict),
@@ -32,6 +33,40 @@ pub fn outcome(code: u16) -> Result<(), ErrorCode> {
   }
 }
 
+/// What a refusal of a create or a delete means. Those two commands are the
+/// only ones whose 550 can be drawn by something already in the way, so they
+/// are the only ones whose reply text is read. A listing, a stat or a read is
+/// classified by `outcome` alone, because pure-ftpd answers a path inside a
+/// folder it will not open with `550 Can't check for file existence`, which
+/// truly cannot be told apart from absence and must stay `NotFound`.
+///
+/// A server whose wording this does not recognise keeps the plain
+/// classification, so an unfamiliar phrasing degrades to today's answer
+/// rather than to a wrong `Conflict`.
+pub fn refusal(code: u16, text: &str) -> Result<(), ErrorCode> {
+  match outcome(code) {
+    Err(ErrorCode::NotFound) if names_a_conflict(text) => {
+      Err(ErrorCode::Conflict)
+    }
+    verdict => verdict,
+  }
+}
+
+/// Whether a refusal names something already there: a path that exists, or a
+/// directory that still holds children. Matched on whole words, so the
+/// `existence` of "Can't check for file existence" is never the `exists` of
+/// "File exists".
+fn names_a_conflict(text: &str) -> bool {
+  let lowered = text.to_lowercase();
+  let words: Vec<&str> = lowered
+    .split(|letter: char| !letter.is_ascii_alphanumeric())
+    .filter(|word| !word.is_empty())
+    .collect();
+  words.contains(&"exists")
+    || words.contains(&"nonempty")
+    || words.windows(2).any(|pair| matches!(pair, ["not" | "non", "empty"]))
+}
+
 /// What the first reply to a transfer command means. A transfer answers with
 /// a preliminary reply while the data connection is open; its completion
 /// comes only after that connection closes and is read separately.
@@ -176,7 +211,7 @@ fn extended_port(text: &str) -> Option<u16> {
 
 #[cfg(test)]
 mod tests {
-  use super::{extended_port, outcome, preliminary};
+  use super::{extended_port, outcome, preliminary, refusal};
   use memona_plugin_sdk::ErrorCode;
 
   #[test]
@@ -207,6 +242,81 @@ mod tests {
     assert!(matches!(outcome(150), Err(ErrorCode::Unavailable)));
   }
 
+  #[test]
+  fn a_refused_create_or_delete_names_what_is_in_the_way() {
+    // The two refusals a person can see through, in pure-ftpd's wording.
+    assert!(matches!(
+      refusal(550, "550 Can't create directory: File exists"),
+      Err(ErrorCode::Conflict)
+    ));
+    assert!(matches!(
+      refusal(550, "550 Can't remove directory: Directory not empty"),
+      Err(ErrorCode::Conflict)
+    ));
+    // The same two conditions as other servers phrase them.
+    assert!(matches!(
+      refusal(550, "550 Cannot create a file when that file already exists."),
+      Err(ErrorCode::Conflict)
+    ));
+    assert!(matches!(
+      refusal(550, "550 /work/dir: Directory not empty"),
+      Err(ErrorCode::Conflict)
+    ));
+    assert!(matches!(
+      refusal(550, "550 The directory is non-empty."),
+      Err(ErrorCode::Conflict)
+    ));
+  }
+
+  #[test]
+  fn a_refusal_that_cannot_be_read_stays_a_missing_path() {
+    // The genuine ambiguity: a path inside a folder that will not open. The
+    // word is "existence", not "exists", and it must not be mistaken for one.
+    assert!(matches!(
+      refusal(550, "550 Can't check for file existence"),
+      Err(ErrorCode::NotFound)
+    ));
+    // A path that really is absent, and one the account may not touch.
+    assert!(matches!(
+      refusal(550, "550 Can't remove directory: No such file or directory"),
+      Err(ErrorCode::NotFound)
+    ));
+    assert!(matches!(
+      refusal(550, "550 Can't create directory: Permission denied"),
+      Err(ErrorCode::NotFound)
+    ));
+    // A server that says nothing useful keeps the plain classification.
+    assert!(matches!(
+      refusal(550, "550 Create directory operation failed."),
+      Err(ErrorCode::NotFound)
+    ));
+    assert!(matches!(
+      refusal(550, "550 Échec de la création du répertoire"),
+      Err(ErrorCode::NotFound)
+    ));
+  }
+
+  #[test]
+  fn only_a_550_is_reread_for_a_conflict() {
+    assert!(refusal(257, "257 \"/work/dir\" created").is_ok());
+    assert!(matches!(
+      refusal(530, "530 Login incorrect, the account exists"),
+      Err(ErrorCode::Denied)
+    ));
+    assert!(matches!(
+      refusal(553, "553 File name not allowed, it exists"),
+      Err(ErrorCode::Denied)
+    ));
+    assert!(matches!(
+      refusal(452, "452 Not enough space, and the file exists"),
+      Err(ErrorCode::Conflict)
+    ));
+    assert!(matches!(
+      refusal(421, "421 Service not available"),
+      Err(ErrorCode::Unavailable)
+    ));
+  }
+
   #[test]
   fn a_transfer_starts_on_a_preliminary_reply_only() {
     assert!(preliminary(125).is_ok());

src/lib.rs

@@ -4,7 +4,7 @@ mod control;
 mod listing;
 mod transfer;
 
-use control::{Control, outcome, preliminary, transport};
+use control::{Control, outcome, preliminary, refusal, transport};
 use listing::{machine_info, page};
 use memona_plugin_sdk::memona::filesystem::host;
 use memona_plugin_sdk::{
@@ -339,9 +339,14 @@ impl Guest for Ftp {
     }))
   }
 
+  /// A refused create is a conflict when the path is already taken, which is
+  /// the one thing the server says in words rather than in its reply code.
   fn mkdir(target: Locator) -> Result<FileInfo, ErrorCode> {
     let path = route(&target)?;
-    with_control(|control| control.expect(&format!("MKD {path}")))?;
+    with_control(|control| {
+      let (code, text) = control.send(&format!("MKD {path}"))?;
+      refusal(code, &text)
+    })?;
     Ok(FileInfo {
       kind: EntryKind::Directory,
       size: 0,
@@ -369,7 +374,8 @@ impl Guest for Ftp {
   }
 
   /// A file and an empty directory take different commands, and the server
-  /// only says which was wrong after the attempt.
+  /// only says which was wrong after the attempt. A directory that still
+  /// holds children is refused as a conflict, not as an absence.
   fn remove(target: Locator) -> Result<(), ErrorCode> {
     let path = route(&target)?;
     with_control(|control| {
@@ -377,8 +383,8 @@ impl Guest for Ftp {
       if (200..300).contains(&code) {
         return Ok(());
       }
-      let (code, _) = control.send(&format!("RMD {path}"))?;
-      outcome(code)
+      let (code, text) = control.send(&format!("RMD {path}"))?;
+      refusal(code, &text)
     })
   }