Move read_blob to ExposureCtrl.

- To avoid the instantiation cost whenever possible.
- Also move pmrapp to use the helper instead.

Author: metatoaster

pmrapp/src/exposure/api.rs

@@ -211,7 +211,7 @@ pub async fn read_blob(
     path: String,
     efvid: i64,
     key: String,
-) -> Result<Box<[u8]>, AppError> {
+) -> Result<Vec<u8>, AppError> {
     session().await?
         .enforcer(format!("/exposure/{id}/"), "").await?;
     let platform = platform().await?;
@@ -245,8 +245,7 @@ pub async fn read_safe_index_html(
 
     let blob = read_blob(id, path.clone(), efvid, "index.html".to_string())
         .await
-        .map_err(|_| AppError::InternalServerError)?
-        .into_vec();
+        .map_err(|_| AppError::InternalServerError)?;
     Ok(Builder::new()
         .url_relative(UrlRelative::Custom(Box::new(evaluate)))
         .clean(&String::from_utf8_lossy(&blob))

pmrapp/src/server/exposure.rs

@@ -113,25 +113,11 @@ pub async fn exposure_file_data(
     session: Extension<AuthSession<ACPlatform>>,
     Path((e_id, ef_id, view_key, path)): Path<(i64, i64, String, String)>,
 ) -> Result<Vec<u8>, AppError> {
-    use std::path::{Component, Path};
-
     Session::from(session)
         .enforcer(format!("/exposure/{e_id}/"), "").await?;
     let ec = platform.get_exposure(e_id).await
         .map_err(|_| AppError::InternalServerError)?;
-    // TODO should really have the ExposureCtrl offer the base function
-    // of providing the blob to bypass the more work involving getting
-    // the other Ctrl types instantiated.
-    let mut target = ec.data_root();
-    target.push(ef_id.to_string());
-    target.push(view_key);
-    target.push("work");
-    Path::new(&path).components()
-	.for_each(|p| if let Component::Normal(s) = p {
-	    target.push(s)
-	});
-
-    Ok(tokio::fs::read(target).await
-        .map_err(|_| AppError::InternalServerError)?)
+    ec.read_blob(ef_id, &view_key, &path).await
+        .map_err(|_| AppError::NotFound)
 
 }

pmrapp/src/view/cellml_codegen.rs

@@ -60,7 +60,7 @@ pub fn CellMLCodegen() -> impl IntoView {
                     }.to_string();
                     match code.await {
                         Some(Ok(code)) => {
-                            let code = String::from_utf8(code.into_vec())
+                            let code = String::from_utf8(code)
                                 .map_err(|_| AppError::InternalServerError)?;
                             Ok(view! {
                                 <div><a href="../cellml_codegen">Back</a></div>

pmrctrl/src/error.rs

@@ -41,6 +41,9 @@ pub enum CtrlError {
     /// Path that isn't known under the associated resource
     #[error("unknown path: {0}")]
     UnknownPath(String),
+    /// Path that isn't known under the associated resource
+    #[error("invalid view_key: {0}")]
+    InvalidViewKey(String),
     /// Path is valid, but is not associated with a ExposureFileCtrl
     #[error("exposure file not found: {0}")]
     EFCNotFound(String),

pmrctrl/src/handle/exposure/impls.rs

@@ -20,7 +20,11 @@ use pmrrepo::handle::GitHandle;
 use std::{
     collections::HashMap,
     ops::Deref,
-    path::PathBuf,
+    path::{
+        Component,
+        Path,
+        PathBuf,
+    },
     sync::{
         Arc,
         OnceLock,
@@ -294,6 +298,24 @@ impl<'p> ExposureCtrl<'p> {
         result
     }
 
+    pub async fn read_blob(&self, file_id: i64, view_key: &str, path: &str) -> Result<Vec<u8>, CtrlError> {
+        let mut target = self.data_root();
+        target.push(file_id.to_string());
+        // FIXME should check against `/` in view_key
+        if view_key == "." || view_key == ".." {
+            return Err(CtrlError::InvalidViewKey(view_key.to_string()))
+        }
+        target.push(view_key);
+        target.push("work");
+        Path::new(path).components()
+            .for_each(|p| if let Component::Normal(s) = p {
+                target.push(<&str>::try_from(s)
+                    .expect("this started as a valid str"))
+            });
+        tokio::fs::read(target).await
+            .map_err(|_| CtrlError::EFVCBlobNotFound(path.to_string()))
+    }
+
     /// This ensures there is filesystem level access to the underlying
     /// files for this exposure (backed by the relevant workspace at the
     /// specified commit_id).

pmrctrl/src/handle/exposure_file/impls.rs

@@ -263,4 +263,8 @@ impl<'p> ExposureFileCtrl<'p> {
         data_root.push(self.0.exposure_file.id().to_string());
         data_root
     }
+
+    pub async fn read_blob(&self, view_key: &str, path: &str) -> Result<Vec<u8>, CtrlError> {
+        self.0.exposure.read_blob(self.0.exposure_file.id(), view_key, path).await
+    }
 }

pmrctrl/src/handle/exposure_file_view/impls.rs

@@ -14,11 +14,7 @@ use pmrcore::{
 };
 use std::{
     fmt,
-    path::{
-        Component,
-        Path,
-        PathBuf,
-    },
+    path::PathBuf,
 };
 
 use super::ExposureFileViewCtrl;
@@ -94,16 +90,11 @@ impl<'p> ExposureFileViewCtrl<'p> {
         Ok((efv_id, task.id))
     }
 
-    pub async fn read_blob(&self, path: &str) -> Result<Box<[u8]>, CtrlError> {
-        let mut target = self.working_dir()?;
-        Path::new(path).components()
-            .for_each(|p| if let Component::Normal(s) = p {
-                target.push(<&str>::try_from(s)
-                    .expect("this started as a valid str"))
-            });
-        tokio::fs::read(target).await
-            .map(Vec::into_boxed_slice)
-            .map_err(|_| CtrlError::EFVCBlobNotFound(path.to_string()))
+    pub async fn read_blob(&self, path: &str) -> Result<Vec<u8>, CtrlError> {
+        let view_key = self.exposure_file_view
+            .view_key()
+            .ok_or(CtrlError::EFVCIncomplete)?;
+        self.exposure_file.read_blob(view_key, path).await
     }
 
     pub fn exposure_file_ctrl(&self) -> &ExposureFileCtrl<'p> {

pmrctrl/src/platform/impls/task.rs

@@ -53,6 +53,7 @@ impl<'p> Platform {
                     false
                 }
                 None => {
+                    // TODO we've somehow triggered this with sqlite.
                     log::warn!("Task:{task_id} ran but it failed to produce results?");
                     false
                 }

pmrctrl/tests/pmrctrl_test.rs

@@ -1690,11 +1690,22 @@ async fn test_resolve_exposure_file_view_read_blob() -> anyhow::Result<()> {
         .join("size");
     assert!(target.exists());
 
+    // TODO should check against `/` in view_key
+    let err = exposure.read_blob(1, "..", "1/iorw/data/size").await;
+    assert_eq!(err, Err(CtrlError::InvalidViewKey("..".to_string())));
+    assert_eq!(
+        exposure.read_blob(1, "iorw", "../work/size").await,
+        Err(CtrlError::EFVCBlobNotFound("../work/size".to_string())),
+    );
+
+    let e_size = exposure.read_blob(1, "iorw", "size").await?;
+    assert_eq!(e_size, "13".as_bytes().to_vec());
+
     let size = efvc.read_blob("size").await?;
-    assert_eq!(size, "13".as_bytes().into());
+    assert_eq!(size, "13".as_bytes().to_vec());
     // pardir tokens and absolute paths should all be ignored
-    assert_eq!(efvc.read_blob("../../size").await?, "13".as_bytes().into());
-    assert_eq!(efvc.read_blob("/../../size").await?, "13".as_bytes().into());
+    assert_eq!(efvc.read_blob("../../size").await?, "13".as_bytes().to_vec());
+    assert_eq!(efvc.read_blob("/../../size").await?, "13".as_bytes().to_vec());
 
     assert_eq!(
         efvc.read_blob("../1/size").await,