From af334bb0e42ad7e0206f7eb52c1c8bdf42b33d09 Mon Sep 17 00:00:00 2001 From: lorenz Date: Wed, 23 Apr 2025 13:11:58 +0200 Subject: [PATCH] archive: Various Fixes 1. When `git archive` errors, do not require an exit code, but just use the `ExitStatus` we already have. This requires less unwrapping. Also, the previous default of 0 was misleading, since 0 is returned upon successful termination. 2. Use "refname" instead of "refspec" as these are different concepts. Refer to and in particular . 3. Only allow appending ".tar.gz" to commit IDs, not to refnames, because refnames may contain the suffix ".tar.gz". It might look weird, but it is not forbidden to do `git branch foo.tar.gz`. 4. `char::is_ascii` does not help sanitize refnames to file names. In particular we have `'/'.is_ascii() == true`. Replace forward slashes with two underscores. --- radicle-httpd/src/error.rs | 4 +- radicle-httpd/src/raw.rs | 106 ++++++++++++++++++++++++++----------- 2 files changed, 78 insertions(+), 32 deletions(-) diff --git a/radicle-httpd/src/error.rs b/radicle-httpd/src/error.rs index de549ba72..4ed6422cd 100644 --- a/radicle-httpd/src/error.rs +++ b/radicle-httpd/src/error.rs @@ -86,8 +86,8 @@ pub enum RawError { Io(#[from] std::io::Error), /// Archive error. - #[error("git archive error: {0}")] - Archive(String), + #[error("`git archive` exited with status {0}:\n{1}")] + Archive(ExitStatus, String), /// Git error. #[error(transparent)] diff --git a/radicle-httpd/src/raw.rs b/radicle-httpd/src/raw.rs index 0c414137b..f36804787 100644 --- a/radicle-httpd/src/raw.rs +++ b/radicle-httpd/src/raw.rs @@ -22,6 +22,8 @@ use crate::error::RawError as Error; const MAX_BLOB_SIZE: usize = 10_485_760; +const ARCHIVE_SUFFIX: &str = ".tar.gz"; + static MIMES: &[(&str, &str)] = &[ ("3gp", "video/3gpp"), ("7z", "application/x-7z-compressed"), @@ -96,13 +98,39 @@ static MIMES: &[(&str, &str)] = &[ pub fn router(profile: Arc) -> Router { Router::new() + .route("/:rid/:sha", get(commit_handler)) .route("/:rid/:sha/*path", get(file_by_commit_handler)) .route("/:rid/head/*path", get(file_by_canonical_head_handler)) - .route("/:rid/archive/*refspec", get(archive_by_refspec_handler)) + .route("/:rid/archive/*refname", get(archive_by_refname_handler)) .route("/:rid/blobs/:oid", get(file_by_oid_handler)) .with_state(profile) } +async fn commit_handler( + Path((rid, sha)): Path<(RepoId, String)>, + State(profile): State>, +) -> Result<(StatusCode, HeaderMap, Vec), Error> { + let storage = &profile.storage; + let repo = storage.repository(rid)?; + + // Don't allow accessing private repos. + if repo.identity_doc()?.visibility().is_private() { + return Err(Error::NotFound); + } + + if !sha.ends_with(ARCHIVE_SUFFIX) { + return Err(Error::NotFound); + } + + let sha = sha.trim_end_matches(ARCHIVE_SUFFIX); + + if Oid::from_str(sha).is_err() { + return Err(Error::NotFound); + } + + archive_by_refname(rid, sha.to_string(), profile).await +} + async fn file_by_commit_handler( Path((rid, sha, path)): Path<(RepoId, Oid, String)>, State(profile): State>, @@ -121,10 +149,18 @@ async fn file_by_commit_handler( blob_response(blob, path) } -async fn archive_by_refspec_handler( - Path((rid, refspec)): Path<(RepoId, String)>, +async fn archive_by_refname_handler( + Path((rid, refname)): Path<(RepoId, String)>, State(profile): State>, -) -> impl IntoResponse { +) -> Result<(StatusCode, HeaderMap, Vec), Error> { + archive_by_refname(rid, refname, profile).await +} + +async fn archive_by_refname( + rid: RepoId, + refname: String, + profile: Arc, +) -> Result<(StatusCode, HeaderMap, Vec), Error> { let storage = &profile.storage; let repo = storage.repository(rid)?; @@ -137,24 +173,18 @@ async fn archive_by_refspec_handler( let project = doc.project()?; let repo_name = project.name(); - // Remove possible .tar.gz suffix - let refspec = if refspec.ends_with(".tar.gz") { - refspec.trim_end_matches(".tar.gz") - } else { - &refspec + let (oid, via_refname) = match Oid::from_str(&refname) { + Ok(oid) => (oid, false), + Err(_) => ( + repo.backend + .resolve_reference_from_short_name(&refname) + .map(|reference| reference.target())? + .ok_or(Error::NotFound)? + .into(), + true, + ), }; - let oid = match Oid::from_str(&refspec) { - Ok(oid) => oid, - Err(_) => repo - .backend - .resolve_reference_from_short_name(&refspec) - .map(|reference| reference.target())? - .ok_or(Error::NotFound)? - .into(), - }; - - // SAFETY: Git command is available on the system, so we can safely unwrap. let output = Command::new("git") .arg("archive") .arg("--format=tar.gz") @@ -163,22 +193,38 @@ async fn archive_by_refspec_handler( .output()?; if !output.status.success() { - return Err(Error::Archive(format!( - "{}; code={}", - String::from_utf8_lossy(&output.stderr).trim(), - output.status.code().unwrap_or(0) - ))); + return Err(Error::Archive( + output.status, + String::from_utf8_lossy(&output.stderr).to_string(), + )); } + // Build a filename for the archive, which includes the + // refname (if one was given): + // + // Without refname: -.tar.gz + // With refname: ---.tar.gz + let filename = { + let mut build = String::from(repo_name); + build.push('-'); + + if via_refname { + // NOTE: Sanitize refnames according to + // + build.push_str(&refname.replace("/", "__")); + build.push('-'); + } + + build.push_str(oid.to_string().as_str()); + build.push_str(ARCHIVE_SUFFIX); + build + }; + let mut response_headers = HeaderMap::new(); response_headers.insert("Content-Type", HeaderValue::from_str("application/gzip")?); response_headers.insert( "Content-Disposition", - HeaderValue::from_str(&format!( - "attachment; filename={}-{}.tar.gz", - repo_name, - refspec.replace(|c: char| !c.is_ascii(), "_") - ))?, + HeaderValue::from_str(&format!("attachment; filename=\"{filename}\""))?, ); Ok::<_, Error>((StatusCode::OK, response_headers, output.stdout)) }