cjw/Oxicloud
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix(thumbnails): ETag names the blob actually served

Browse Source 
fe9c4f49 keyed the ETag on the SOURCE file's content hash. That is wrong
whenever the response comes from a satellite table, and for attachments
it is wrong in two ways.

Uploading a preview does not change the file's content, so a
source-keyed ETag does not change either — and with `immutable` set,
clients never revalidate and keep the previous render for up to a year.
The exact staleness fe9c4f49 set out to fix, re-entering through the
attachment path.

Worse: a copy inherits the source hash, so an original and a copy have
identical ETags. Give either one a different uploaded preview and they
serve different bytes under one validator, which a shared cache may hand
to either request. That is a collision, not just staleness.

thumbnail_content_id resolves the identity through the same tier
precedence the read path uses: an attached blob's own hash, else a
derived blob's own hash, else the source-keyed form. An ETag naming a
different tier than the one answering is worse than a coarse one, so the
two orders must not drift.

Derived-hash keying is strictly better than source-keying and never
worse. The sidecar and the derived row are written from the same bytes;
where they can diverge — a sidecar re-rendered while the derived row
stays pinned by ON CONFLICT DO NOTHING — source-keying is wrong too,
because the renderer is not part of that key. This is the step 10 change
arriving early, forced by the attachment case; the plan note stands for
the read-order flip itself.

Known gap: a legacy ext-{file_id}.jpg with no file_attached_blobs row
yet falls through to the source-keyed form. No worse than today, and it
resolves when the import backfills.

attached_thumbnail_copy.hurl now asserts ETags, which is why this went
unnoticed: it compared bytes only, and thumbnail_etag_content_keyed
covers content replacement rather than preview upload. A fresh GET
returned the right bytes throughout — the same "healthy locally, broken
for anyone caching" shape as the two bugs before it.
This commit is contained in:
Edouard Vanbelle
2026-08-25 21:28:34 +02:00
parent d71dd973e7
commit 7d9418f63c
4 changed files with 122 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/infrastructure/services/thumbnail_service.rs
+56
View File
@@ -505,6 +505,62 @@ impl ThumbnailService {
/// `blob_hash` is used to locate the file on disk (dedup-aware).
/// If `None`, only the in-memory cache is checked (used for video
/// thumbnails where blob_hash is not yet resolved).
/// Identity of the bytes a thumbnail request will serve — the body of its
/// HTTP ETag.
///
/// Mirrors the tier precedence in [`Self::get_cached_thumbnail`], because
/// an ETag that names a different tier than the one answering is worse
/// than a coarse one: it lets two resources serving different bytes share
/// a validator, and a shared cache may then hand either to either.
///
/// * An **attached** blob wins, and its own hash is the identity. Nothing
/// else works: uploading a preview does not change the file's content,
/// so a source-keyed ETag would not change either — and with
/// `immutable` set, clients would never revalidate. Worse, a copy
/// inherits the source hash, so an original and a copy carrying
/// *different* uploaded previews would collide on one ETag.
/// * Otherwise a **derived** blob's own hash. Strictly better than
/// source-keying, never worse: the pair is written from the same bytes,
/// and where they can diverge — a sidecar re-rendered while the derived
/// row stays pinned by `ON CONFLICT DO NOTHING` — a source-keyed ETag
/// is wrong too, because the renderer is not part of the key.
/// * Otherwise the source-keyed form, which still identifies a render of
/// known content at a known size and format.
///
/// Known gap: a legacy `ext-{file_id}.jpg` with no `file_attached_blobs`
/// row yet falls through to the source-keyed form, so those bytes keep
/// today's coarse validator until the import backfills the row. No worse
/// than current behaviour, and it disappears with the migration.
pub async fn thumbnail_content_id(
&self,
file_id: &str,
blob_hash: &str,
size: ThumbnailSize,
format: ThumbnailFormat,
dedup: Option<&DedupService>,
) -> String {
if let Some(dedup) = dedup {
if let Some(attached) = dedup
.find_attached_blob(file_id, "preview", size.dir_name())
.await
{
return attached.blob_hash;
}
if let Some(derived) = dedup
.find_derived_blob(blob_hash, "thumbnail", size.dir_name())
.await
{
return derived.blob_hash;
}
}
format!(
"thumb-{}-{}-{}",
blob_hash,
size.dir_name(),
format.as_str()
)
}
/// Drain a blob through the dedup stack into memory.
///
/// Shared by the attached and derived tiers — the only difference between
src/interfaces/api/handlers/file_handler.rs
+16 -12
View File
@@ -487,18 +487,22 @@ impl FileHandler {
Ok(h) => h,
Err(err) => return AppError::from(err).into_response(),
};
let etag = {
let (s, f) = (thumb_size.as_str(), format.as_str());
let mut e = String::with_capacity(9 + blob_hash.len() + s.len() + f.len());
e.push_str("\"thumb-");
e.push_str(&blob_hash);
e.push('-');
e.push_str(s);
e.push('-');
e.push_str(f);
e.push('"');
e
};
// The identity of the bytes about to be served, resolved through the
// same tier precedence the read path uses — an uploaded preview's own
// hash, else a derived thumbnail's own hash, else the source-keyed
// form. See `ThumbnailService::thumbnail_content_id`.
let etag = format!(
"\"{}\"",
thumbnail_service
.thumbnail_content_id(
&id,
&blob_hash,
thumb_size.into(),
format,
Some(&state.core.dedup_service),
)
.await
);
if let Some(if_none_match) = headers.get(header::IF_NONE_MATCH)
&& let Ok(val) = if_none_match.to_str()
&& (val == etag || val == "*")
src/interfaces/nextcloud/preview_handler.rs
+17 -10
View File
@@ -166,16 +166,23 @@ pub async fn handle_preview(
.unwrap();
}
};
let etag = {
let s = thumb_size.as_str();
let mut e = String::with_capacity(9 + blob_hash.len() + s.len());
e.push_str("\"thumb-");
e.push_str(&blob_hash);
e.push('-');
e.push_str(s);
e.push('"');
e
};
// Same tier-precedence resolution as the REST endpoint: an uploaded
// preview's own hash, else a derived thumbnail's own hash, else the
// source-keyed form. NC pins JPEG, so that is the format asked for.
let etag = format!(
"\"{}\"",
state
.core
.thumbnail_service
.thumbnail_content_id(
&object_id,
&blob_hash,
thumb_size.into(),
ThumbnailFormat::Jpeg,
Some(&state.core.dedup_service),
)
.await
);
if let Some(inm) = req.headers().get(header::IF_NONE_MATCH)
&& let Ok(client_etag) = inm.to_str()
&& (client_etag == etag || client_etag == "*")
tests/api/attached_thumbnail_copy.hurl
+33
View File
@@ -92,6 +92,7 @@ Authorization: Bearer {{token}}
HTTP 200
[Captures]
rendered_thumb: bytes
rendered_etag: header "ETag"
# ─────────────────────────────────────────────────────────────
@@ -112,8 +113,33 @@ Authorization: Bearer {{token}}
HTTP 200
[Captures]
uploaded_thumb: bytes
uploaded_etag: header "ETag"
[Asserts]
bytes != {{rendered_thumb}}
# The ETag must move with the bytes. It is keyed on the ATTACHED blob's own
# hash, because uploading a preview leaves the file's content — and so a
# source-keyed ETag — unchanged. With `immutable` set, an unchanged
# validator means clients never revalidate and keep the old render for a
# year.
header "ETag" != "{{rendered_etag}}"
# A client holding the pre-upload validator must be told to refetch.
GET {{base_url}}/api/files/{{orig_file_id}}/thumbnail/preview
Authorization: Bearer {{token}}
If-None-Match: {{rendered_etag}}
HTTP 200
[Asserts]
header "ETag" == "{{uploaded_etag}}"
# ...and the new one revalidates.
GET {{base_url}}/api/files/{{orig_file_id}}/thumbnail/preview
Authorization: Bearer {{token}}
If-None-Match: {{uploaded_etag}}
HTTP 304
# ─────────────────────────────────────────────────────────────
@@ -141,6 +167,12 @@ HTTP 200
[Asserts]
bytes == {{uploaded_thumb}}
bytes != {{rendered_thumb}}
# Same bytes, so the same validator — the copy's attachment row points at
# the same blob. This is also what stops the collision a source-keyed ETag
# would allow: the copy inherits the source hash, so if either side later
# gets a DIFFERENT preview the two would serve different bytes under one
# ETag, and a shared cache could hand either to either.
header "ETag" == "{{uploaded_etag}}"
# ─────────────────────────────────────────────────────────────
@@ -184,6 +216,7 @@ HTTP 200
[Asserts]
bytes == {{uploaded_thumb}}
bytes != {{rendered_thumb}}
header "ETag" == "{{uploaded_etag}}"
# ─────────────────────────────────────────────────────────────