fix: cap blob and diff rendering to bound per-request memory
commit
6a867bffix: cap blob and diff rendering to bound per-request memory
blob_page, blob_pane, and commit_page read whole objects and diffs into memory via cat-file/show, so a large object could exhaust the server even though is_binary only sampled the first 8 KB. Reads now go through a capped reader that stops git at 2 MiB and renders a truncation notice past it.
feat: add git_output_capped reading at most a byte cap test: cover capped read truncation, full read, and failure Assisted-by: Claude:claude-opus-4-8
Reviews
No reviews of this commit yet — record a verdict below.
Start a review
crates/git-ents-server/src/web/git.rs
@@ -8,6 +8,7 @@
use gix_date::Time;
use gix_hash::ObjectId;
use gix_object::tree::{Entry, EntryKind, EntryMode};
+use tokio::io::AsyncReadExt as _;
use tokio::process::Command;
use crate::http::{MAX_REPO_DEPTH, is_bare_repo};
@@ -37,6 +38,46 @@
Some(out.stdout)
}
+/// Run `git -C <repo> <args>` capturing at most `cap` bytes of stdout, returning
+/// the captured bytes and whether stdout exceeded `cap`. `None` on a spawn
+/// failure or, for output that fit under the cap, a non-zero exit.
+///
+/// Reading at most `cap + 1` bytes and killing git once the cap is reached
+/// bounds the memory a single request can consume, so an arbitrarily large blob
+/// or diff renders as a truncation notice instead of being slurped whole — the
+/// difference between a capped response and an out-of-memory kill.
+pub(super) async fn git_output_capped(
+ repo: &Path,
+ args: &[&str],
+ cap: usize,
+) -> Option<(Vec<u8>, bool)> {
+ let mut child = Command::new("git")
+ .arg("-C")
+ .arg(repo)
+ .args(args)
+ .stdout(Stdio::piped())
+ .stderr(Stdio::null())
+ .spawn()
+ .ok()?;
+ let mut stdout = child.stdout.take()?;
+ let mut buf = Vec::new();
+ let probe = u64::try_from(cap).unwrap_or(u64::MAX).saturating_add(1);
+ (&mut stdout).take(probe).read_to_end(&mut buf).await.ok()?;
+ if buf.len() > cap {
+ // Over the cap: keep what we have, stop git, and report truncation.
+ buf.truncate(cap);
+ let _killed = child.start_kill();
+ let _reaped = child.wait().await;
+ return Some((buf, true));
+ }
+ let status = child.wait().await.ok()?;
+ if status.success() {
+ Some((buf, false))
+ } else {
+ None
+ }
+}
+
/// The entries of the root tree at `HEAD`, directories first then by name.
pub(super) async fn root_tree(repo: &Path, has_head: bool) -> Vec<Entry> {
if !has_head {
@@ -266,3 +307,76 @@
pub(super) async fn latest_release(repo: &Path) -> Option<Release> {
releases(repo).await.into_iter().next()
}
+
+#[cfg(test)]
+mod tests {
+ #![allow(clippy::unwrap_used, reason = "unit test")]
+
+ use std::process::Command as SyncCommand;
+
+ use super::*;
+
+ fn commit_blob(repo: &Path, name: &str, bytes: usize) {
+ SyncCommand::new("git")
+ .arg("-C")
+ .arg(repo)
+ .args(["init", "-q"])
+ .status()
+ .unwrap();
+ std::fs::write(repo.join(name), vec![b'x'; bytes]).unwrap();
+ for args in [
+ vec!["add", "."],
+ vec![
+ "-c",
+ "user.name=t",
+ "-c",
+ "user.email=t@e",
+ "commit",
+ "-qm",
+ "x",
+ ],
+ ] {
+ SyncCommand::new("git")
+ .arg("-C")
+ .arg(repo)
+ .args(&args)
+ .status()
+ .unwrap();
+ }
+ }
+
+ #[tokio::test]
+ async fn capped_read_flags_oversized_output() {
+ let dir = tempfile::tempdir().unwrap();
+ commit_blob(dir.path(), "big.txt", 4096);
+ let (bytes, truncated) =
+ git_output_capped(dir.path(), &["cat-file", "-p", "HEAD:big.txt"], 1024)
+ .await
+ .unwrap();
+ assert!(truncated);
+ assert_eq!(bytes.len(), 1024);
+ }
+
+ #[tokio::test]
+ async fn capped_read_returns_full_small_output() {
+ let dir = tempfile::tempdir().unwrap();
+ commit_blob(dir.path(), "small.txt", 100);
+ let (bytes, truncated) =
+ git_output_capped(dir.path(), &["cat-file", "-p", "HEAD:small.txt"], 1024)
+ .await
+ .unwrap();
+ assert!(!truncated);
+ assert_eq!(bytes.len(), 100);
+ }
+
+ #[tokio::test]
+ async fn capped_read_reports_failure_as_none() {
+ let dir = tempfile::tempdir().unwrap();
+ commit_blob(dir.path(), "small.txt", 10);
+ assert!(
+ git_output_capped(dir.path(), &["cat-file", "-p", "HEAD:missing"], 1024)
+ .await
+ .is_none()
+ );
+ }
+}
crates/git-ents-server/src/web/pages.rs
@@ -16,13 +16,20 @@
use maud::{Markup, PreEscaped, html};
use super::git::{
- browse_path, git_output, git_output_bytes, languages, latest_release, list_tree, parse_iso,
- releases, root_tree,
+ browse_path, git_output, git_output_bytes, git_output_capped, languages, latest_release,
+ list_tree, parse_iso, releases, root_tree,
};
use super::icons::*;
use super::render::Render;
use super::{RepoMeta, Tab, not_found, repo_shell};
+/// The largest blob or diff rendered in full. Past it a request would read an
+/// unbounded object into memory and highlight it, so the view shows a truncation
+/// notice instead — a cap on what one page can cost. 2 MiB comfortably covers
+/// real source files while ruling out the multi-hundred-MiB objects that would
+/// exhaust the server.
+const MAX_RENDER_BYTES: usize = 2 * 1024 * 1024;
+
/// A single repository's overview: the rendered README beside an aside of
/// clone, about, releases, and language cards.
pub(super) async fn repo_page(repo: &Path, meta: &RepoMeta, host: Option<&str>) -> Markup {
@@ -327,7 +334,12 @@
/// The right-hand pane of the Files view: a file's path, line/size meta, and its
/// syntax-highlighted source (or a binary notice).
async fn blob_pane(repo: &Path, path: &str) -> Markup {
- let Some(bytes) = git_output_bytes(repo, &["cat-file", "-p", &format!("HEAD:{path}")]).await
+ let Some((bytes, truncated)) = git_output_capped(
+ repo,
+ &["cat-file", "-p", &format!("HEAD:{path}")],
+ MAX_RENDER_BYTES,
+ )
+ .await
else {
return html! { div.files-empty { "File not found." } };
};
@@ -336,14 +348,18 @@
div.blob-head {
span { (path) }
span.meta {
- @if !is_binary(&bytes) {
+ @if !truncated && !is_binary(&bytes) {
span { (String::from_utf8_lossy(&bytes).lines().count()) " lines" }
}
- span { (human_size(bytes.len())) }
- button.copy-btn data-copy=(String::from_utf8_lossy(&bytes)) { "Copy" }
+ span { (human_size(bytes.len())) @if truncated { "+" } }
+ @if !truncated {
+ button.copy-btn data-copy=(String::from_utf8_lossy(&bytes)) { "Copy" }
+ }
}
}
- @if is_binary(&bytes) {
+ @if truncated {
+ div.binary { "File too large to display (over " (human_size(MAX_RENDER_BYTES)) ")." }
+ } @else if is_binary(&bytes) {
div.binary { "Binary file (" (human_size(bytes.len())) ") not shown." }
} @else {
(blob_body(name, &String::from_utf8_lossy(&bytes)))
@@ -465,11 +481,15 @@
{
return not_found().into_response();
}
- let Some(bytes) = git_output_bytes(repo, &["cat-file", "-p", &spec]).await else {
+ let Some((bytes, truncated)) =
+ git_output_capped(repo, &["cat-file", "-p", &spec], MAX_RENDER_BYTES).await
+ else {
return not_found().into_response();
};
let name = path.rsplit('/').next().unwrap_or(&path);
- let body = if is_binary(&bytes) {
+ let body = if truncated {
+ html! { div.blob { div.binary { "File too large to display (over " (human_size(MAX_RENDER_BYTES)) ")." } } }
+ } else if is_binary(&bytes) {
html! { div.blob { div.binary { "Binary file (" (human_size(bytes.len())) ") not shown." } } }
} else {
let text = String::from_utf8_lossy(&bytes);
@@ -562,9 +582,14 @@
let subject = parts.next().unwrap_or_default().to_owned();
let body = parts.next().unwrap_or_default().trim_end().to_owned();
let short = short_oid(&oid);
- let patch = git_output(repo, &["show", "--no-color", "--format=", "--patch", sha])
- .await
- .unwrap_or_default();
+ let (patch_bytes, patch_truncated) = git_output_capped(
+ repo,
+ &["show", "--no-color", "--format=", "--patch", sha],
+ MAX_RENDER_BYTES,
+ )
+ .await
+ .unwrap_or_default();
+ let patch = String::from_utf8_lossy(&patch_bytes);
repo_shell(
meta,
@@ -582,6 +607,9 @@
}
}
(diff_view(&patch))
+ @if patch_truncated {
+ div.card { div.binary { "Diff truncated (over " (human_size(MAX_RENDER_BYTES)) ")." } }
+ }
},
)
.into_response()