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 <https://git-scm.com/docs/git-check-ref-format> and in
    particular
    <https://git-scm.com/docs/git-check-ref-format#Documentation/git-check-ref-format.txt---refspec-pattern>.
 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

@@ -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)]

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<Profile>) -> 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<Arc<Profile>>,
+) -> Result<(StatusCode, HeaderMap, Vec<u8>), 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<Arc<Profile>>,
@@ -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<Arc<Profile>>,
-) -> impl IntoResponse {
+) -> Result<(StatusCode, HeaderMap, Vec<u8>), Error> {
+    archive_by_refname(rid, refname, profile).await
+}
+
+async fn archive_by_refname(
+    rid: RepoId,
+    refname: String,
+    profile: Arc<Profile>,
+) -> Result<(StatusCode, HeaderMap, Vec<u8>), 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:   <repo-name>-<oid>.tar.gz
+    // With    refname:   <repo-name>-<refname>--<oid>.tar.gz
+    let filename = {
+        let mut build = String::from(repo_name);
+        build.push('-');
+
+        if via_refname {
+            // NOTE: Sanitize refnames according to
+            // <https://git-scm.com/docs/git-check-ref-format>
+            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))
 }