fix: memory issues when importing giant mrpack files (#6278)

* feat: dont load mrpacks into memory if they are local imports

* fix: frontend

packages/app-lib/src/api/pack/install_from.rs

@@ -8,10 +8,9 @@ use crate::state::{
     SideType,
 };
 use crate::util::fetch::{
-    DownloadMeta, DownloadReason, fetch, fetch_advanced, write_cached_icon,
+    DownloadMeta, DownloadReason, fetch, fetch_advanced, sha1_file_async,
+    write_cached_icon,
 };
-use crate::util::io;
-
 use path_util::SafeRelativeUtf8UnixPathBuf;
 use reqwest::Method;
 use serde::{Deserialize, Serialize};
@@ -134,12 +133,22 @@ impl Default for CreatePackProfile {
     }
 }
 
+#[derive(Clone)]
+pub enum CreatePackFile {
+    Bytes(bytes::Bytes),
+    // Local packs can be larger than available memory, so keep them file-backed.
+    Path(PathBuf),
+}
+
 #[derive(Clone)]
 pub struct CreatePack {
-    pub file: bytes::Bytes,
+    pub file: CreatePackFile,
     pub description: CreatePackDescription,
 }
 
+// The hash lookup only gates the unknown-pack warning, so avoid a long blocking scan for huge local packs.
+const MAX_LOCAL_FILE_HASH_LOOKUP_SIZE: u64 = 1024 * 1024 * 1024;
+
 #[derive(Clone, Debug)]
 pub struct CreatePackDescription {
     pub icon: Option<PathBuf>,
@@ -176,28 +185,31 @@ pub async fn get_profile_from_pack(
                 .to_string_lossy()
                 .to_string();
 
-            let state = State::get().await?;
-            let file_bytes = io::read(&path).await?;
-            let hash =
-                crate::util::fetch::sha1_async(bytes::Bytes::from(file_bytes))
-                    .await?;
-            let is_known_file = match CachedEntry::get_file_many(
-                &[&hash],
-                Some(CacheBehaviour::StaleWhileRevalidateSkipOffline),
-                &state.pool,
-                &state.api_semaphore,
-            )
-            .await
+            let is_known_file = if tokio::fs::metadata(&path).await?.len()
+                <= MAX_LOCAL_FILE_HASH_LOOKUP_SIZE
             {
-                Ok(files) => !files.is_empty(),
-                Err(err) => {
-                    tracing::warn!(
-                        "Failed to check Modrinth file hash for {}: {}",
-                        path.display(),
-                        err
-                    );
-                    false
+                let state = State::get().await?;
+                let (_, hash) = sha1_file_async(&path).await?;
+                match CachedEntry::get_file_many(
+                    &[&hash],
+                    Some(CacheBehaviour::StaleWhileRevalidateSkipOffline),
+                    &state.pool,
+                    &state.api_semaphore,
+                )
+                .await
+                {
+                    Ok(files) => !files.is_empty(),
+                    Err(err) => {
+                        tracing::warn!(
+                            "Failed to check Modrinth file hash for {}: {}",
+                            path.display(),
+                            err
+                        );
+                        false
+                    }
                 }
+            } else {
+                false
             };
 
             Ok(CreatePackProfile {
@@ -380,7 +392,7 @@ pub async fn generate_pack_from_version_id(
     }
 
     Ok(CreatePack {
-        file,
+        file: CreatePackFile::Bytes(file),
         description: CreatePackDescription {
             icon,
             override_title: Some(title),
@@ -398,9 +410,8 @@ pub async fn generate_pack_from_file(
     path: PathBuf,
     profile_path: String,
 ) -> crate::Result<CreatePack> {
-    let file = io::read(&path).await?;
     Ok(CreatePack {
-        file: bytes::Bytes::from(file),
+        file: CreatePackFile::Path(path),
         description: CreatePackDescription {
             icon: None,
             override_title: None,

packages/app-lib/src/api/pack/install_mrpack.rs

@@ -9,21 +9,212 @@ use crate::state::{
     CacheBehaviour, CachedEntry, Profile, ProfileInstallStage, SideType,
     cache_file_hash,
 };
-use crate::util::fetch::{
-    DownloadMeta, DownloadReason, fetch_mirrors, sha1_async, write,
-};
+use crate::util::fetch::{DownloadMeta, DownloadReason, fetch_mirrors, write};
 use crate::util::io;
 use crate::{State, profile};
-use async_zip::base::read::seek::ZipFileReader;
+use async_zip::base::read::seek::ZipFileReader as SeekZipFileReader;
+use async_zip::base::read::{WithEntry, ZipEntryReader};
+use async_zip::tokio::read::fs::ZipFileReader as FsZipFileReader;
 use futures::StreamExt;
 use path_util::SafeRelativeUtf8UnixPathBuf;
 
 use super::install_from::{
-    CreatePack, CreatePackLocation, PackFormat, generate_pack_from_file,
-    generate_pack_from_version_id,
+    CreatePack, CreatePackFile, CreatePackLocation, PackFormat,
+    generate_pack_from_file, generate_pack_from_version_id,
 };
 use crate::data::ProjectType;
 use std::io::{Cursor, ErrorKind};
+use std::path::Path;
+use tokio::io::AsyncWriteExt;
+
+enum MrpackZipReader {
+    Memory(async_zip::tokio::read::seek::ZipFileReader<Cursor<bytes::Bytes>>),
+    // Local imports stay on disk so large .mrpacks do not have to fit in memory.
+    File(FsZipFileReader),
+}
+
+impl MrpackZipReader {
+    async fn new(file: &CreatePackFile) -> crate::Result<Self> {
+        match file {
+            CreatePackFile::Bytes(file) => Ok(Self::Memory(
+                SeekZipFileReader::with_tokio(Cursor::new(file.clone()))
+                    .await
+                    .map_err(|_| {
+                        crate::Error::from(crate::ErrorKind::InputError(
+                            "Failed to read input modpack zip".to_string(),
+                        ))
+                    })?,
+            )),
+            CreatePackFile::Path(path) => Ok(Self::File(
+                FsZipFileReader::new(path).await.map_err(|_| {
+                    crate::Error::from(crate::ErrorKind::InputError(
+                        "Failed to read input modpack zip".to_string(),
+                    ))
+                })?,
+            )),
+        }
+    }
+
+    fn file(&self) -> &async_zip::ZipFile {
+        match self {
+            Self::Memory(reader) => reader.file(),
+            Self::File(reader) => reader.file(),
+        }
+    }
+
+    async fn read_entry_to_string(
+        &mut self,
+        index: usize,
+    ) -> crate::Result<String> {
+        let mut value = String::new();
+        match self {
+            Self::Memory(reader) => {
+                let mut reader = reader.reader_with_entry(index).await?;
+                reader.read_to_string_checked(&mut value).await?;
+            }
+            Self::File(reader) => {
+                let mut reader = reader.reader_with_entry(index).await?;
+                reader.read_to_string_checked(&mut value).await?;
+            }
+        }
+
+        Ok(value)
+    }
+
+    async fn hash_entry(
+        &mut self,
+        index: usize,
+    ) -> crate::Result<(u64, String)> {
+        match self {
+            Self::Memory(reader) => {
+                hash_zip_entry(reader.reader_with_entry(index).await?).await
+            }
+            Self::File(reader) => {
+                hash_zip_entry(reader.reader_with_entry(index).await?).await
+            }
+        }
+    }
+
+    async fn extract_entry(
+        &mut self,
+        index: usize,
+        path: &Path,
+        semaphore: &crate::util::fetch::IoSemaphore,
+    ) -> crate::Result<(u64, String)> {
+        match self {
+            Self::Memory(reader) => {
+                extract_zip_entry(
+                    reader.reader_with_entry(index).await?,
+                    path,
+                    semaphore,
+                )
+                .await
+            }
+            Self::File(reader) => {
+                extract_zip_entry(
+                    reader.reader_with_entry(index).await?,
+                    path,
+                    semaphore,
+                )
+                .await
+            }
+        }
+    }
+}
+
+async fn hash_zip_entry<R>(
+    mut reader: ZipEntryReader<'_, R, WithEntry<'_>>,
+) -> crate::Result<(u64, String)>
+where
+    R: futures_lite::io::AsyncBufRead + Unpin,
+{
+    let expected_crc32 = reader.entry().crc32();
+    let mut hasher = sha1_smol::Sha1::new();
+    let mut size = 0;
+    let mut buffer = vec![0; 262144];
+
+    loop {
+        let bytes_read =
+            futures_lite::io::AsyncReadExt::read(&mut reader, &mut buffer)
+                .await?;
+        if bytes_read == 0 {
+            break;
+        }
+
+        hasher.update(&buffer[..bytes_read]);
+        size += bytes_read as u64;
+    }
+
+    if reader.compute_hash() != expected_crc32 {
+        return Err(async_zip::error::ZipError::CRC32CheckError.into());
+    }
+
+    Ok((size, hasher.digest().to_string()))
+}
+
+async fn extract_zip_entry<R>(
+    mut reader: ZipEntryReader<'_, R, WithEntry<'_>>,
+    path: &Path,
+    semaphore: &crate::util::fetch::IoSemaphore,
+) -> crate::Result<(u64, String)>
+where
+    R: futures_lite::io::AsyncBufRead + Unpin,
+{
+    let _permit = semaphore.0.acquire().await?;
+
+    if let Some(parent) = path.parent() {
+        io::create_dir_all(parent).await?;
+    }
+
+    let parent = path.parent().ok_or_else(|| {
+        io::IOError::from(std::io::Error::other(
+            "could not get parent directory for temporary file",
+        ))
+    })?;
+    let temp_path = tempfile::NamedTempFile::new_in(parent)
+        .map_err(|e| io::IOError::with_path(e, parent))?
+        .into_temp_path();
+
+    // Only replace the profile file after the ZIP entry has passed its CRC check.
+    let expected_crc32 = reader.entry().crc32();
+    let mut file = tokio::fs::File::create(&temp_path)
+        .await
+        .map_err(|e| io::IOError::with_path(e, &temp_path))?;
+    let mut hasher = sha1_smol::Sha1::new();
+    let mut size = 0;
+    let mut buffer = vec![0; 262144];
+
+    loop {
+        let bytes_read =
+            futures_lite::io::AsyncReadExt::read(&mut reader, &mut buffer)
+                .await?;
+        if bytes_read == 0 {
+            break;
+        }
+
+        file.write_all(&buffer[..bytes_read])
+            .await
+            .map_err(|e| io::IOError::with_path(e, &temp_path))?;
+        hasher.update(&buffer[..bytes_read]);
+        size += bytes_read as u64;
+    }
+
+    file.flush()
+        .await
+        .map_err(|e| io::IOError::with_path(e, &temp_path))?;
+    drop(file);
+
+    if reader.compute_hash() != expected_crc32 {
+        return Err(async_zip::error::ZipError::CRC32CheckError.into());
+    }
+
+    temp_path.persist(path).map_err(|e| {
+        let tempfile::PathPersistError { error, .. } = e;
+        io::IOError::with_path(error, path)
+    })?;
+
+    Ok((size, hasher.digest().to_string()))
+}
 
 /// Install a pack
 /// Wrapper around install_pack_files that generates a pack creation description, and
@@ -93,15 +284,7 @@ pub async fn install_zipped_mrpack_files(
     let profile_path = create_pack.description.profile_path;
     let icon_exists = icon.is_some();
 
-    let reader: Cursor<&bytes::Bytes> = Cursor::new(&file);
-
-    // Create zip reader around file
-    let mut zip_reader =
-        ZipFileReader::with_tokio(reader).await.map_err(|_| {
-            crate::Error::from(crate::ErrorKind::InputError(
-                "Failed to read input modpack zip".to_string(),
-            ))
-        })?;
+    let mut zip_reader = MrpackZipReader::new(&file).await?;
 
     // Extract index of modrinth.index.json
     let Some(manifest_idx) = zip_reader.file().entries().iter().position(|f| {
@@ -113,8 +296,7 @@ pub async fn install_zipped_mrpack_files(
     };
 
     let mut manifest = String::new();
-    let mut reader = zip_reader.reader_with_entry(manifest_idx).await?;
-    reader.read_to_string_checked(&mut manifest).await?;
+    manifest.push_str(&zip_reader.read_entry_to_string(manifest_idx).await?);
 
     let pack: PackFormat = serde_json::from_str(&manifest)?;
 
@@ -151,11 +333,7 @@ pub async fn install_zipped_mrpack_files(
             .collect();
 
         for index in override_entries {
-            let mut file_bytes = Vec::new();
-            let mut entry_reader = zip_reader.reader_with_entry(index).await?;
-            entry_reader.read_to_end_checked(&mut file_bytes).await?;
-
-            let hash = sha1_async(bytes::Bytes::from(file_bytes)).await?;
+            let (_, hash) = zip_reader.hash_entry(index).await?;
             file_hashes.push(hash);
         }
 
@@ -320,17 +498,18 @@ pub async fn install_zipped_mrpack_files(
                 ))
             })?;
 
-        let mut file_bytes = vec![];
-        let mut reader = zip_reader.reader_with_entry(index).await?;
-        reader.read_to_end_checked(&mut file_bytes).await?;
+        let path = profile::get_full_path(&profile_path)
+            .await?
+            .join(relative_override_file_path.as_str());
+        let (size, hash) = zip_reader
+            .extract_entry(index, &path, &state.io_semaphore)
+            .await?;
 
-        let file_bytes = bytes::Bytes::from(file_bytes);
-
-        cache_file_hash(
-            file_bytes.clone(),
+        crate::state::cache_file_hash_metadata(
             &profile_path,
             relative_override_file_path.as_str(),
-            None,
+            size,
+            hash,
             ProjectType::get_from_parent_folder(
                 relative_override_file_path.as_str(),
             ),
@@ -338,15 +517,6 @@ pub async fn install_zipped_mrpack_files(
         )
         .await?;
 
-        write(
-            &profile::get_full_path(&profile_path)
-                .await?
-                .join(relative_override_file_path.as_str()),
-            &file_bytes,
-            &state.io_semaphore,
-        )
-        .await?;
-
         emit_loading(
             &loading_bar,
             30.0 / override_file_entries_count as f64,
@@ -382,17 +552,10 @@ pub async fn install_zipped_mrpack_files(
 
 pub async fn remove_all_related_files(
     profile_path: String,
-    mrpack_file: bytes::Bytes,
+    mrpack_file: CreatePackFile,
 ) -> crate::Result<()> {
-    let reader: Cursor<&bytes::Bytes> = Cursor::new(&mrpack_file);
-
-    // Create zip reader around file
-    let mut zip_reader =
-        ZipFileReader::with_tokio(reader).await.map_err(|_| {
-            crate::Error::from(crate::ErrorKind::InputError(
-                "Failed to read input modpack zip".to_string(),
-            ))
-        })?;
+    // Updates can remove files from a locally imported or downloaded pack, so share the same reader path.
+    let mut zip_reader = MrpackZipReader::new(&mrpack_file).await?;
 
     // Extract index of modrinth.index.json
     let Some(manifest_idx) = zip_reader.file().entries().iter().position(|f| {
@@ -403,10 +566,7 @@ pub async fn remove_all_related_files(
         )));
     };
 
-    let mut manifest = String::new();
-
-    let mut reader = zip_reader.reader_with_entry(manifest_idx).await?;
-    reader.read_to_string_checked(&mut manifest).await?;
+    let manifest = zip_reader.read_entry_to_string(manifest_idx).await?;
 
     let pack: PackFormat = serde_json::from_str(&manifest)?;