Fix extracting password protected archives

Closes: #1157

The fix splits the "canceled" and "failed" states for OperationError. It
also preserves that state because some functions overwrote the state by
rewrapping the error.

Author: joshuamegnauth54

i18n/en/cosmic_files.ftl

@@ -185,6 +185,7 @@ no-history = No items in history.
 pending = Pending
 progress = {$percent}%
 progress-cancelled = {$percent}%, cancelled
+progress-failed = {$percent}%, failed
 progress-paused = {$percent}%, paused
 failed = Failed
 complete = Complete

src/archive.rs

@@ -47,53 +47,58 @@ pub fn extract(
     controller: &Controller,
 ) -> Result<(), OperationError> {
     let mime = mime_for_path(path, None, false);
-    let controller = controller.clone();
     let password = password.clone();
     match mime.essence_str() {
-        "application/gzip" | "application/x-compressed-tar" => OpReader::new(path, controller)
-            .map(io::BufReader::new)
-            .map(flate2::read::GzDecoder::new)
-            .map(tar::Archive::new)
-            .and_then(|mut archive| archive.unpack(&new_dir))
-            .map_err(OperationError::from_str)?,
-        "application/x-tar" => OpReader::new(path, controller)
+        "application/gzip" | "application/x-compressed-tar" => {
+            OpReader::new(path, controller.clone())
+                .map(io::BufReader::new)
+                .map(flate2::read::GzDecoder::new)
+                .map(tar::Archive::new)
+                .and_then(|mut archive| archive.unpack(new_dir))
+                .map_err(|e| OperationError::from_err(e, controller))?
+        }
+        "application/x-tar" => OpReader::new(path, controller.clone())
             .map(io::BufReader::new)
             .map(tar::Archive::new)
-            .and_then(|mut archive| archive.unpack(&new_dir))
-            .map_err(OperationError::from_str)?,
+            .and_then(|mut archive| archive.unpack(new_dir))
+            .map_err(|e| OperationError::from_err(e, controller))?,
         "application/zip" => fs::File::open(path)
             .map(io::BufReader::new)
             .map(zip::ZipArchive::new)
-            .map_err(OperationError::from_str)?
-            .and_then(move |mut archive| zip_extract(&mut archive, &new_dir, password, controller))
+            .map_err(|e| OperationError::from_err(e, controller))?
+            .and_then(move |mut archive| {
+                zip_extract(&mut archive, new_dir, password, controller.clone())
+            })
             .map_err(|e| match e {
                 ZipError::UnsupportedArchive(ZipError::PASSWORD_REQUIRED)
-                | ZipError::InvalidPassword => OperationError {
-                    kind: OperationErrorType::PasswordRequired,
-                },
-                _ => OperationError::from_str(e),
+                | ZipError::InvalidPassword => {
+                    OperationError::from_kind(OperationErrorType::PasswordRequired, controller)
+                }
+                _ => OperationError::from_err(e, controller),
             })?,
         #[cfg(feature = "bzip2")]
         "application/x-bzip"
         | "application/x-bzip-compressed-tar"
         | "application/x-bzip2"
-        | "application/x-bzip2-compressed-tar" => OpReader::new(path, controller)
+        | "application/x-bzip2-compressed-tar" => OpReader::new(path, controller.clone())
             .map(io::BufReader::new)
             .map(bzip2::read::BzDecoder::new)
             .map(tar::Archive::new)
-            .and_then(|mut archive| archive.unpack(&new_dir))
-            .map_err(OperationError::from_str)?,
+            .and_then(|mut archive| archive.unpack(new_dir))
+            .map_err(|e| OperationError::from_err(e, controller))?,
         #[cfg(feature = "xz2")]
-        "application/x-xz" | "application/x-xz-compressed-tar" => OpReader::new(path, controller)
-            .map(io::BufReader::new)
-            .map(xz2::read::XzDecoder::new)
-            .map(tar::Archive::new)
-            .and_then(|mut archive| archive.unpack(&new_dir))
-            .map_err(OperationError::from_str)?,
-        _ => Err(OperationError::from_str(format!(
-            "unsupported mime type {:?}",
-            mime
-        )))?,
+        "application/x-xz" | "application/x-xz-compressed-tar" => {
+            OpReader::new(path, controller.clone())
+                .map(io::BufReader::new)
+                .map(xz2::read::XzDecoder::new)
+                .map(tar::Archive::new)
+                .and_then(|mut archive| archive.unpack(new_dir))
+                .map_err(|e| OperationError::from_err(e, controller))?
+        }
+        _ => Err(OperationError::from_err(
+            format!("unsupported mime type {:?}", mime),
+            controller,
+        ))?,
     }
     Ok(())
 }
@@ -135,19 +140,18 @@ fn zip_extract<R: io::Read + io::Seek, P: AsRef<Path>>(
             controller
                 .check()
                 .await
-                .map_err(|err| io::Error::new(io::ErrorKind::Other, err))
+                .map_err(|s| io::Error::other(OperationError::from_state(s, &controller)))
         })?;
 
         controller.set_progress((i as f32) / total_files as f32);
 
         let mut file = match &password {
             None => archive.by_index(i),
             Some(pwd) => archive.by_index_decrypt(i, pwd.as_bytes()),
-        }
-        .map_err(|e| e)?;
+        }?;
         let filepath = file
             .enclosed_name()
-            .ok_or(ZipError::InvalidArchive("Invalid file path".into()))?;
+            .ok_or(ZipError::InvalidArchive("Invalid file path"))?;
 
         let outpath = directory.as_ref().join(filepath);
 
@@ -206,8 +210,7 @@ fn zip_extract<R: io::Read + io::Seek, P: AsRef<Path>>(
         let mut file = match &password {
             None => archive.by_index(i),
             Some(pwd) => archive.by_index_decrypt(i, pwd.as_bytes()),
-        }
-        .map_err(|e| e)?;
+        }?;
 
         // create all pending dirs
         while let Some(pending_dir) = pending_directory_creates.pop_front() {
@@ -226,7 +229,7 @@ fn zip_extract<R: io::Read + io::Seek, P: AsRef<Path>>(
                 controller
                     .check()
                     .await
-                    .map_err(|err| io::Error::new(io::ErrorKind::Other, err))
+                    .map_err(|s| io::Error::other(OperationError::from_state(s, &controller)))
             })?;
 
             let count = file.read(&mut buffer)?;

src/operation/controller.rs

@@ -1,11 +1,10 @@
-use crate::fl;
-
 use std::sync::{Arc, Mutex};
 use tokio::sync::Notify;
 
-#[derive(Clone, Copy, Debug)]
+#[derive(Clone, Copy, Debug, PartialEq)]
 pub enum ControllerState {
     Cancelled,
+    Failed,
     Paused,
     Running,
 }
@@ -37,10 +36,11 @@ impl Default for Controller {
 }
 
 impl Controller {
-    pub async fn check(&self) -> Result<(), String> {
+    pub async fn check(&self) -> Result<(), ControllerState> {
         loop {
             match self.state() {
-                ControllerState::Cancelled => return Err(fl!("cancelled")),
+                ControllerState::Cancelled => return Err(ControllerState::Cancelled),
+                ControllerState::Failed => return Err(ControllerState::Failed),
                 ControllerState::Paused => (),
                 ControllerState::Running => return Ok(()),
             }
@@ -74,6 +74,10 @@ impl Controller {
         self.set_state(ControllerState::Cancelled);
     }
 
+    pub fn is_failed(&self) -> bool {
+        matches!(self.state(), ControllerState::Failed)
+    }
+
     pub fn is_paused(&self) -> bool {
         matches!(self.state(), ControllerState::Paused)
     }
@@ -83,7 +87,7 @@ impl Controller {
     }
 
     pub fn unpause(&self) {
-        if !self.is_cancelled() {
+        if !self.is_cancelled() | !self.is_failed() {
             self.set_state(ControllerState::Running);
         }
     }
@@ -100,8 +104,8 @@ impl Clone for Controller {
 
 impl Drop for Controller {
     fn drop(&mut self) {
-        // Cancel operations if primary controller is dropped
-        if self.primary {
+        // Cancel operations if primary controller is dropped and controller is still running
+        if self.primary && self.state() != ControllerState::Failed {
             self.cancel();
         }
     }