Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Use source mtime for Last-Modified and unify modified-since semantics #1326

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lovasoa wants to merge 1 commit into main
base: main
Choose a base branch
Loading
from fix-last-modified-header-handling
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 73 additions & 1 deletion src/filesystem.rs
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,32 @@ impl FileSystem {
}
}

pub async fn last_modified(
&self,
app_state: &AppState,
path: &Path,
priviledged: bool,
) -> anyhow::Result<DateTime<Utc>> {
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::<Utc>::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,
Expand Down Expand Up @@ -258,6 +284,7 @@ pub struct DbFsQueries {
was_modified: AnyStatement<'static>,
read_file: AnyStatement<'static>,
exists: AnyStatement<'static>,
last_modified: AnyStatement<'static>,
}

impl DbFsQueries {
Expand Down Expand Up @@ -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?,
})
}

Expand All @@ -300,7 +328,7 @@ impl DbFsQueries {

async fn make_was_modified_query(db: &Database) -> anyhow::Result<AnyStatement<'static>> {
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)
);
Expand Down Expand Up @@ -331,6 +359,39 @@ impl DbFsQueries {
db.prepare_with(&exists_query, param_types).await
}

async fn make_last_modified_query(db: &Database) -> anyhow::Result<AnyStatement<'static>> {
let last_modified_query = format!(
"SELECT last_modified from sqlpage_files WHERE path = {}",
make_placeholder(db.info.kind, 1),
);
let param_types: &[AnyTypeInfo; 1] = &[<str as Type<Postgres>>::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<DateTime<Utc>> {
self.last_modified
.query_as::<(DateTime<Utc>,)>()

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Handle nullable database modification timestamps

The documented and generated sqlpage_files schemas allow last_modified to be NULL for several databases, so a row inserted with an explicit NULL was previously still readable and servable. Decoding it as DateTime<Utc> rejects NULL, and serve_file now calls this method before every static-file read, causing requests for such DB-backed assets to return a 500 instead of their contents. Decode an optional timestamp and provide an appropriate fallback or omit the header for NULL values.

Useful? React with 👍 / 👎.

lovasoa reacted with thumbs up emoji

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@codex fix

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have reached your Codex usage limits. You can see your limits in the Codex usage dashboard.

.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,
Expand Down Expand Up @@ -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(())
}
28 changes: 14 additions & 14 deletions src/webserver/http.rs
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -441,17 +439,19 @@ async fn serve_file(
if_modified_since: Option<IfModifiedSince>,
) -> actix_web::Result<HttpResponse> {
let path = strip_site_prefix(path, state);
if let Some(IfModifiedSince(date)) = if_modified_since {
let since = DateTime::<Utc>::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
Expand All @@ -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)
})
}
Expand Down
40 changes: 40 additions & 0 deletions tests/requests/mod.rs
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
Loading

AltStyle によって変換されたページ (->オリジナル) /