From 85b7ec34c2337bc0baf6d49fbfead99db8f39d45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jelmer=20Vernoo=C4=B3?= Date: Wed, 22 Apr 2026 00:46:51 +0100 Subject: [PATCH] download: send Last-Modified based on the served revision The tip-based last_modified middleware set Last-Modified on every response to the branch tip's commit time, which is wrong for downloads of files/tarballs from older revisions. Emit the header from the download handlers using the served revision's timestamp, and let the middleware skip responses that already carry one. Fixes: https://bugs.launchpad.net/loggerhead/+bug/503144 --- NEWS | 8 ++++++++ src/app.rs | 6 ++---- src/controllers/download.rs | 36 ++++++++++++++++++++++-------------- src/util/fmt.rs | 17 +++++++++++++++++ 4 files changed, 49 insertions(+), 18 deletions(-) diff --git a/NEWS b/NEWS index f6bc9c74..93a694de 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,14 @@ What's changed in loggerhead? ============================= +UNRELEASED +---------- + + - Send ``Last-Modified`` based on the revision being served for file + downloads and tarballs, rather than the branch tip. The tip-based + middleware no longer overwrites a header already set by the + handler. (Jelmer Vernooij, #503144) + 3.0.0 [22Apr2026] ----------------- diff --git a/src/app.rs b/src/app.rs index 4335b01c..b1eacffd 100644 --- a/src/app.rs +++ b/src/app.rs @@ -5,7 +5,6 @@ use axum::http::{header, HeaderValue, StatusCode}; use axum::middleware::Next; use axum::response::{IntoResponse, Redirect, Response}; use axum::{routing::get, Router}; -use chrono::{DateTime, Utc}; use moka::sync::Cache; use tower_http::services::ServeDir; use tower_http::trace::TraceLayer; @@ -225,10 +224,9 @@ async fn last_modified_layer( } let mut resp = next.run(req).await; - if resp.status().is_success() { + if resp.status().is_success() && !resp.headers().contains_key(header::LAST_MODIFIED) { if let Some(ts) = last_ts { - if let Some(dt) = DateTime::::from_timestamp(ts as i64, 0) { - let rfc = dt.format("%a, %d %b %Y %H:%M:%S GMT").to_string(); + if let Some(rfc) = crate::util::fmt::http_date(ts) { if let Ok(v) = HeaderValue::from_str(&rfc) { resp.headers_mut().insert(header::LAST_MODIFIED, v); } diff --git a/src/controllers/download.rs b/src/controllers/download.rs index 7df7758f..de0bebb6 100644 --- a/src/controllers/download.rs +++ b/src/controllers/download.rs @@ -16,6 +16,7 @@ use crate::app::AppState; use crate::breezy::open_branch; use crate::history::History; use crate::util::errors::{AppError, AppResult}; +use crate::util::fmt::http_date; /// GET /download (no args) — permanent redirect to `/changes`. Matches /// Python's DownloadUI, which redirects when fewer than two args are given. @@ -41,7 +42,7 @@ pub async fn show_file( .unwrap_or_else(|| path.clone()); let path_for_task = path.clone(); - let content = tokio::task::spawn_blocking(move || -> AppResult> { + let (content, timestamp) = tokio::task::spawn_blocking(move || -> AppResult<(Vec, f64)> { let branch = open_branch(&state.root)?; let _lock = branch.lock_read()?; let whole = state.load_whole_history(&branch)?; @@ -52,7 +53,11 @@ pub async fn show_file( let repo = branch.repository(); let tree = repo.revision_tree(&revid)?; let p = PathBuf::from(&path_for_task); - Ok(tree.get_file_text(&p)?) + let bytes = tree.get_file_text(&p)?; + // `Last-Modified` for the download: timestamp of the revision + // being served, not the branch tip. See Launchpad bug #503144. + let timestamp = repo.get_revision(&revid)?.timestamp; + Ok((bytes, timestamp)) }) .await??; @@ -60,16 +65,17 @@ pub async fn show_file( .first_or_octet_stream() .to_string(); let encoded = utf8_percent_encode(&filename, NON_ALPHANUMERIC).to_string(); - Ok(Response::builder() + let mut builder = Response::builder() .status(StatusCode::OK) .header(header::CONTENT_TYPE, mime) .header( header::CONTENT_DISPOSITION, format!("attachment; filename*=utf-8''{encoded}"), - ) - .body(Body::from(content)) - .unwrap() - .into_response()) + ); + if let Some(d) = http_date(timestamp) { + builder = builder.header(header::LAST_MODIFIED, d); + } + Ok(builder.body(Body::from(content)).unwrap().into_response()) } /// GET /tarball/:revid — stream a tgz of the tree at the given @@ -89,7 +95,7 @@ pub async fn tarball( // iterator across the async boundary while still holding the GIL is // awkward; for the sizes most loggerhead deployments see this is a fair // tradeoff, and we can revisit with a bounded mpsc channel if needed. - let (bytes, filename) = tokio::task::spawn_blocking(move || -> AppResult<_> { + let (bytes, filename, timestamp) = tokio::task::spawn_blocking(move || -> AppResult<_> { let branch = open_branch(&state.root)?; let _lock = branch.lock_read()?; let whole = state.load_whole_history(&branch)?; @@ -108,23 +114,25 @@ pub async fn tarball( // be a dotted revno. let revno_part = history.whole.get_revno(&revid); let filename = format!("{nick}-r{revno_part}.tgz"); + let timestamp = repo.get_revision(&revid)?.timestamp; let mut out = Vec::new(); for chunk in archive(&tree, ArchiveFormat::Tgz, &filename, None, Some(&nick))? { out.extend_from_slice(&chunk?); } - Ok::<_, AppError>((out, filename)) + Ok::<_, AppError>((out, filename, timestamp)) }) .await??; let encoded = utf8_percent_encode(&filename, NON_ALPHANUMERIC).to_string(); - Ok(Response::builder() + let mut builder = Response::builder() .status(StatusCode::OK) .header(header::CONTENT_TYPE, "application/octet-stream") .header( header::CONTENT_DISPOSITION, format!("attachment; filename*=utf-8''{encoded}"), - ) - .body(Body::from(bytes)) - .unwrap() - .into_response()) + ); + if let Some(d) = http_date(timestamp) { + builder = builder.header(header::LAST_MODIFIED, d); + } + Ok(builder.body(Body::from(bytes)).unwrap().into_response()) } diff --git a/src/util/fmt.rs b/src/util/fmt.rs index 046b99cb..2ff51eb8 100644 --- a/src/util/fmt.rs +++ b/src/util/fmt.rs @@ -12,6 +12,14 @@ pub fn hide_email(author: &str) -> String { } } +/// Render a Breezy (unix) timestamp as an RFC 7231 HTTP-date, suitable +/// for the `Last-Modified` header. Returns `None` if the timestamp is +/// outside the representable range. +pub fn http_date(timestamp: f64) -> Option { + DateTime::::from_timestamp(timestamp as i64, 0) + .map(|dt| dt.format("%a, %d %b %Y %H:%M:%S GMT").to_string()) +} + /// Render a Breezy timestamp+timezone as the "2026-04-21 17:09:44 UTC" form /// Python loggerhead uses as the `` tooltip. pub fn utc_iso(timestamp: f64, timezone: i32) -> String { @@ -80,4 +88,13 @@ mod tests { let t = (Utc::now() - chrono::Duration::hours(3)).timestamp() as f64; assert_eq!(approximate_date(t), "3 hours ago"); } + + #[test] + fn http_date_formats_rfc7231() { + // 2024-01-02T03:04:05Z + assert_eq!( + http_date(1_704_164_645.0).as_deref(), + Some("Tue, 02 Jan 2024 03:04:05 GMT"), + ); + } }