From ac22aeeb3678cdcb08dd6add76aeb8d28a940148 Mon Sep 17 00:00:00 2001 From: Ophir LOJKINE Date: Fri, 12 Jun 2026 14:52:55 +0200 Subject: [PATCH] Fix static file Last-Modified headers --- src/filesystem.rs | 74 ++++++++++++++++++++++++++++++++++++++++++- src/webserver/http.rs | 28 ++++++++-------- tests/requests/mod.rs | 40 +++++++++++++++++++++++ 3 files changed, 127 insertions(+), 15 deletions(-) diff --git a/src/filesystem.rs b/src/filesystem.rs index e081d2fc..e20cb47c 100644 --- a/src/filesystem.rs +++ b/src/filesystem.rs @@ -36,6 +36,32 @@ impl FileSystem { } } + pub async fn last_modified( + &self, + app_state: &AppState, + path: &Path, + priviledged: bool, + ) -> anyhow::Result> { + let local_path = self.safe_local_path(app_state, path, priviledged)?; + let local_result = tokio::fs::metadata(&local_path) + .await + .and_then(|metadata| metadata.modified()) + .map(DateTime::::from); + match (local_result, &self.db_fs_queries) { + (Ok(last_modified), _) => Ok(last_modified), + (Err(e), Some(db_fs)) if is_path_missing_error(&e) => { + db_fs.last_modified_in_db(app_state, path).await + } + (Err(e), _) => { + let status = io_error_status(&e) + .unwrap_or(actix_web::http::StatusCode::INTERNAL_SERVER_ERROR); + Err(e).with_status(status).with_context(|| { + format!("Unable to read local file metadata for {}", path.display()) + }) + } + } + } + pub async fn modified_since( &self, app_state: &AppState, @@ -258,6 +284,7 @@ pub struct DbFsQueries { was_modified: AnyStatement<'static>, read_file: AnyStatement<'static>, exists: AnyStatement<'static>, + last_modified: AnyStatement<'static>, } impl DbFsQueries { @@ -286,6 +313,7 @@ impl DbFsQueries { was_modified: Self::make_was_modified_query(db).await?, read_file: Self::make_read_file_query(db).await?, exists: Self::make_exists_query(db).await?, + last_modified: Self::make_last_modified_query(db).await?, }) } @@ -300,7 +328,7 @@ impl DbFsQueries { async fn make_was_modified_query(db: &Database) -> anyhow::Result> { let was_modified_query = format!( - "SELECT 1 from sqlpage_files WHERE last_modified >= {} AND path = {}", + "SELECT 1 from sqlpage_files WHERE last_modified > {} AND path = {}", make_placeholder(db.info.kind, 1), make_placeholder(db.info.kind, 2) ); @@ -331,6 +359,39 @@ impl DbFsQueries { db.prepare_with(&exists_query, param_types).await } + async fn make_last_modified_query(db: &Database) -> anyhow::Result> { + let last_modified_query = format!( + "SELECT last_modified from sqlpage_files WHERE path = {}", + make_placeholder(db.info.kind, 1), + ); + let param_types: &[AnyTypeInfo; 1] = &[>::type_info().into()]; + db.prepare_with(&last_modified_query, param_types).await + } + + async fn last_modified_in_db( + &self, + app_state: &AppState, + path: &Path, + ) -> anyhow::Result> { + self.last_modified + .query_as::<(DateTime,)>() + .bind(path.display().to_string()) + .fetch_optional(&app_state.db.connection) + .await + .map_err(anyhow::Error::from) + .and_then(|last_modified| { + last_modified + .map(|(last_modified,)| last_modified) + .ok_or_else(|| { + ErrorWithStatus { + status: actix_web::http::StatusCode::NOT_FOUND, + } + .into() + }) + }) + .with_context(|| format!("Unable to get the modification time of {}", path.display())) + } + async fn file_modified_since_in_db( &self, app_state: &AppState, @@ -475,5 +536,16 @@ async fn test_sql_file_read_utf8() -> anyhow::Result<()> { "File should not be modified since one hour in the future" ); + let last_modified = fs + .last_modified(&state, "unit test file.txt".as_ref(), false) + .await?; + let was_modified = fs + .modified_since(&state, "unit test file.txt".as_ref(), last_modified, false) + .await?; + assert!( + !was_modified, + "A file should not be considered modified since its exact modification time" + ); + Ok(()) } diff --git a/src/webserver/http.rs b/src/webserver/http.rs index 242d9613..0706a782 100644 --- a/src/webserver/http.rs +++ b/src/webserver/http.rs @@ -33,14 +33,12 @@ use crate::webserver::routing::RoutingAction::{ use crate::webserver::routing::{AppFileStore, calculate_route}; use actix_web::body::MessageBody; use anyhow::{Context, bail}; -use chrono::{DateTime, Utc}; use futures_util::StreamExt; use futures_util::stream::Stream; use std::borrow::Cow; use std::path::PathBuf; use std::pin::Pin; use std::sync::Arc; -use std::time::SystemTime; use tokio::sync::mpsc; #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] @@ -441,17 +439,19 @@ async fn serve_file( if_modified_since: Option, ) -> actix_web::Result { let path = strip_site_prefix(path, state); - if let Some(IfModifiedSince(date)) = if_modified_since { - let since = DateTime::::from(SystemTime::from(date)); - let modified = state - .file_system - .modified_since(state, path.as_ref(), since, false) - .await - .with_context(|| format!("Unable to get modification time of file {path:?}")) - .map_err(|e| anyhow_err_to_actix(e, state))?; - if !modified { - return Ok(HttpResponse::NotModified().finish()); - } + let last_modified = state + .file_system + .last_modified(state, path.as_ref(), false) + .await + .with_context(|| format!("Unable to get modification time of file {path:?}")) + .map_err(|e| anyhow_err_to_actix(e, state))?; + let last_modified = HttpDate::from(last_modified.into()); + if let Some(IfModifiedSince(date)) = if_modified_since + && last_modified <= date + { + return Ok(HttpResponse::NotModified() + .insert_header(LastModified(last_modified)) + .finish()); } state .file_system @@ -466,7 +466,7 @@ async fn serve_file( .first() .map_or_else(ContentType::octet_stream, ContentType), ) - .insert_header(LastModified(HttpDate::from(SystemTime::now()))) + .insert_header(LastModified(last_modified)) .body(b) }) } diff --git a/tests/requests/mod.rs b/tests/requests/mod.rs index d2b8fe75..7e77af57 100644 --- a/tests/requests/mod.rs +++ b/tests/requests/mod.rs @@ -248,3 +248,43 @@ async fn test_missing_multipart_content_disposition_returns_bad_request() -> act } mod webhook_hmac; + +#[actix_web::test] +async fn static_file_uses_source_last_modified_for_revalidation() -> anyhow::Result<()> { + use actix_web::http::header; + use actix_web::http::header::HttpDate; + use std::time::{SystemTime, UNIX_EPOCH}; + + let unique = SystemTime::now().duration_since(UNIX_EPOCH)?.as_nanos(); + let web_root = std::env::temp_dir().join(format!("sqlpage-last-modified-{unique}")); + std::fs::create_dir_all(&web_root)?; + let asset_path = web_root.join("asset.txt"); + std::fs::write(&asset_path, "hello static world\n")?; + + let expected = HttpDate::from(std::fs::metadata(&asset_path)?.modified()?); + let mut config = crate::common::test_config(); + config.web_root = web_root.clone(); + let app_data = crate::common::make_app_data_from_config(config).await; + + let req = crate::common::get_request_to_with_data("/asset.txt", app_data.clone()) + .await? + .to_srv_request(); + let response = main_handler(req).await?; + assert_eq!(response.status(), StatusCode::OK); + let last_modified = response + .headers() + .get(header::LAST_MODIFIED) + .expect("static files should have a Last-Modified header") + .to_str()?; + assert_eq!(last_modified, expected.to_string()); + + let req = crate::common::get_request_to_with_data("/asset.txt", app_data) + .await? + .insert_header((header::IF_MODIFIED_SINCE, last_modified)) + .to_srv_request(); + let response = main_handler(req).await?; + assert_eq!(response.status(), StatusCode::NOT_MODIFIED); + + std::fs::remove_dir_all(web_root)?; + Ok(()) +}