> = vec![
+ Ok(bytes::Bytes::from_static(b"ok")),
+ Err(std::io::Error::other("network blip")),
+ ];
+ let s = stream::iter(chunks);
+ let err = accumulate_capped(s, 100).await.unwrap_err();
+ assert!(err.to_string().contains("network blip"));
+ }
+
+ #[test]
+ fn looks_like_image_accepts_jpeg() {
+ // JPEG SOI + APP0 segment.
+ let jpeg = [0xff, 0xd8, 0xff, 0xe0, 0, 0x10, b'J', b'F', b'I', b'F'];
+ assert!(looks_like_image(&jpeg));
+ }
+
+ #[test]
+ fn looks_like_image_accepts_png() {
+ let png = [0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0, 0, 0, 0];
+ assert!(looks_like_image(&png));
+ }
+
+ #[test]
+ fn looks_like_image_rejects_html_disguised_as_image() {
+ let html = b"not an image";
+ assert!(!looks_like_image(html));
+ }
+
+ #[test]
+ fn looks_like_image_rejects_empty() {
+ assert!(!looks_like_image(&[]));
+ }
+
+ #[test]
+ fn looks_like_image_rejects_renderable_but_unsupported_formats() {
+ // BMP, TIFF, ICO, PSD are `infer::MatcherType::Image` but the
+ // /files/*key handler doesn't have Content-Type mappings for
+ // them, so they'd be served as application/octet-stream and
+ // download instead of render. Reject at the crawler so we
+ // never land them in storage.
+ // BMP magic: "BM" + 4-byte size.
+ let bmp = [b'B', b'M', 0, 0, 0, 0, 0, 0, 0, 0, 0, 0];
+ assert!(!looks_like_image(&bmp), "BMP must be rejected (not renderable by /files)");
+
+ // TIFF little-endian magic: "II" + 42.
+ let tiff = [0x49, 0x49, 0x2a, 0x00, 0, 0, 0, 0];
+ assert!(!looks_like_image(&tiff), "TIFF must be rejected");
+
+ // ICO magic: 0x00,0x00,0x01,0x00.
+ let ico = [0x00, 0x00, 0x01, 0x00, 1, 0, 16, 16, 0, 0, 1, 0, 0x18, 0, 0x40, 0, 0, 0, 0x16, 0, 0, 0];
+ assert!(!looks_like_image(&ico), "ICO must be rejected");
+ }
+
+ #[test]
+ fn looks_like_image_accepts_webp_gif_avif() {
+ // Cover the three remaining whitelisted formats so a future
+ // tightening that drops one would fail noisily.
+ let webp = [
+ b'R', b'I', b'F', b'F',
+ 0, 0, 0, 0,
+ b'W', b'E', b'B', b'P',
+ b'V', b'P', b'8', b' ',
+ ];
+ assert!(looks_like_image(&webp));
+
+ let gif = [b'G', b'I', b'F', b'8', b'7', b'a', 0, 0, 0, 0];
+ assert!(looks_like_image(&gif));
+
+ let avif = [
+ 0x00, 0x00, 0x00, 0x18,
+ b'f', b't', b'y', b'p',
+ b'a', b'v', b'i', b'f',
+ 0x00, 0x00, 0x00, 0x00,
+ b'm', b'i', b'f', b'1',
+ b'a', b'v', b'i', b'f',
+ ];
+ assert!(looks_like_image(&avif));
+ }
+}
diff --git a/backend/src/crawler/session.rs b/backend/src/crawler/session.rs
index 321363e..1782c83 100644
--- a/backend/src/crawler/session.rs
+++ b/backend/src/crawler/session.rs
@@ -100,6 +100,54 @@ pub fn classify_probe(html: &str) -> SessionProbe {
}
}
+/// Three-way classification of a chapter page response.
+///
+/// Reader pages don't render `#logo`, so [`classify_probe`] can't be
+/// reused as-is. The chapter-specific marker is `a#pic_container`
+/// (asserted by the reader-page parser at `parse_chapter_pages`).
+///
+/// Order matters: broken-page body wins over selector matches, so a
+/// transient site-wide 5xx that happens to render the avatar widget
+/// elsewhere doesn't falsely reach `Ok`.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum ChapterProbe {
+ /// `a#pic_container` present — reader rendered. Whether
+ /// `#avatar_menu` is also there is informational; if the reader
+ /// loaded the session is by definition still good.
+ Ok,
+ /// Site rendered a "logged out" or "please log in" page (no
+ /// reader, no broken-page body, and no avatar widget either).
+ /// Distinguishes the genuine expired-session case from a
+ /// transient site hiccup.
+ Unauthenticated,
+ /// Broken-page body, or reader didn't render but the user is
+ /// still logged in (avatar widget present). Caller should retry
+ /// rather than blame the session.
+ Transient,
+}
+
+pub fn classify_chapter_probe(html: &str) -> ChapterProbe {
+ if is_broken_page_body(html) {
+ return ChapterProbe::Transient;
+ }
+ let doc = scraper::Html::parse_document(html);
+ let container = scraper::Selector::parse("a#pic_container").unwrap();
+ if doc.select(&container).next().is_some() {
+ return ChapterProbe::Ok;
+ }
+ let avatar = scraper::Selector::parse("#avatar_menu").unwrap();
+ if doc.select(&avatar).next().is_some() {
+ // Logged-in user, but the reader didn't render — most likely
+ // the layout shifted or the site is serving an interstitial.
+ ChapterProbe::Transient
+ } else {
+ // No reader, no avatar, no broken-body marker — site rendered
+ // the "please log in" page, which is the genuine session-
+ // expired signal on this route.
+ ChapterProbe::Unauthenticated
+ }
+}
+
/// In-startup retry budget for the session probe. Small but non-zero —
/// startup hitting a 5-second site hiccup shouldn't fail the operator
/// with "PHPSESSID expired" when the session is actually fine.
@@ -210,6 +258,73 @@ mod tests {
assert_eq!(classify_probe(""), SessionProbe::Transient);
}
+ #[test]
+ fn classify_chapter_probe_ok_when_reader_rendered() {
+ let html = r#"
+
+
+ 
+
+
+ "#;
+ assert_eq!(classify_chapter_probe(html), ChapterProbe::Ok);
+ }
+
+ #[test]
+ fn classify_chapter_probe_unauthenticated_when_no_reader_and_no_avatar() {
+ // What a logged-out hit on a chapter URL renders: a normal
+ // site layout (header etc.) with a "please log in" body, but
+ // no reader and no avatar widget.
+ let html = r#"
+
+
+ Please log in to read this chapter.
+
+ "#;
+ assert_eq!(
+ classify_chapter_probe(html),
+ ChapterProbe::Unauthenticated
+ );
+ }
+
+ #[test]
+ fn classify_chapter_probe_transient_when_logged_in_but_reader_missing() {
+ // Avatar shows the session is still valid; reader didn't
+ // render — site is serving an interstitial or the layout
+ // momentarily shifted. Retry, don't blame the session.
+ let html = r#"
+
+
+ Site maintenance — back in 5 minutes.
+
+ "#;
+ assert_eq!(classify_chapter_probe(html), ChapterProbe::Transient);
+ }
+
+ #[test]
+ fn classify_chapter_probe_transient_on_broken_page_body() {
+ let html =
+ "we're sorry, the request file are not found.
";
+ assert_eq!(classify_chapter_probe(html), ChapterProbe::Transient);
+ }
+
+ #[test]
+ fn classify_chapter_probe_does_not_misfire_on_avatar_alone_without_reader() {
+ // Regression for the original bug: the binary
+ // find_element("#avatar_menu") check treated "no avatar" as
+ // session-expired even when a transient hiccup was the real
+ // cause. classify_chapter_probe must NOT trip on that pattern
+ // when pic_container *is* present.
+ let html = r#"
+
+
+
+
+
+ "#;
+ assert_eq!(classify_chapter_probe(html), ChapterProbe::Ok);
+ }
+
#[test]
fn classify_probe_trusts_broken_body_over_stray_avatar_match() {
// Defensive: if a broken-page body somehow contains an
diff --git a/backend/src/repo/crawler.rs b/backend/src/repo/crawler.rs
index eb83877..eeb5bfd 100644
--- a/backend/src/repo/crawler.rs
+++ b/backend/src/repo/crawler.rs
@@ -274,7 +274,20 @@ async fn sync_tags(
manga_id: Uuid,
tags: &[String],
) -> sqlx::Result<()> {
- sqlx::query("DELETE FROM manga_tags WHERE manga_id = $1")
+ // Only clear crawler-owned attachments (added_by IS NULL). User-
+ // attached tags are owned by the attaching user and must survive
+ // the recurring metadata pass — see manga_tags.added_by in
+ // migration 0009.
+ //
+ // Note on orphans: `manga_tags.added_by` is `ON DELETE SET NULL`,
+ // so an attachment whose user was deleted becomes
+ // indistinguishable from a crawler-owned row and is cleaned up
+ // here. That mirrors how `api::mangas::detach_tag` already treats
+ // orphans ("nobody owns it, refuse to let anyone but admin clear
+ // them") — the crawler now becomes the eventual reaper. Tracked
+ // by `sync_tags_garbage_collects_orphan_user_attachments` in
+ // backend/tests/crawler_sync.rs.
+ sqlx::query("DELETE FROM manga_tags WHERE manga_id = $1 AND added_by IS NULL")
.bind(manga_id)
.execute(&mut **tx)
.await?;
diff --git a/backend/src/repo/manga.rs b/backend/src/repo/manga.rs
index f285940..53d2ace 100644
--- a/backend/src/repo/manga.rs
+++ b/backend/src/repo/manga.rs
@@ -281,3 +281,17 @@ pub async fn exists(pool: &PgPool, id: Uuid) -> AppResult {
.await?;
Ok(exists)
}
+
+/// Returns the uploader's user id for a manga. `None` either when the
+/// manga doesn't exist or when the row predates the `uploaded_by`
+/// column (historical NULL — see migration 0011). Callers must
+/// distinguish "manga missing" via [`exists`] before relying on this
+/// to make an authz decision.
+pub async fn uploaded_by(pool: &PgPool, id: Uuid) -> AppResult> {
+ let row: Option<(Option,)> =
+ sqlx::query_as("SELECT uploaded_by FROM mangas WHERE id = $1")
+ .bind(id)
+ .fetch_optional(pool)
+ .await?;
+ Ok(row.and_then(|(u,)| u))
+}
diff --git a/backend/src/storage/local.rs b/backend/src/storage/local.rs
index 7e6dd23..77a4fa2 100644
--- a/backend/src/storage/local.rs
+++ b/backend/src/storage/local.rs
@@ -16,6 +16,13 @@ impl LocalStorage {
}
fn resolve(&self, key: &str) -> Result {
+ // NUL bytes are rejected by the Linux syscall layer, but the
+ // error surfaces as an opaque IO failure rather than the
+ // explicit `BadKey` the rest of the contract uses. Catch it
+ // here so the error path is consistent.
+ if key.contains('\0') {
+ return Err(StorageError::BadKey);
+ }
let key = key.trim_start_matches('/');
if key.is_empty() {
return Err(StorageError::BadKey);
@@ -114,6 +121,9 @@ mod tests {
assert!(matches!(s.get(".").await, Err(StorageError::BadKey)));
// Empty segment via doubled slash.
assert!(matches!(s.get("a//b").await, Err(StorageError::BadKey)));
+ // NUL byte (rejected explicitly so callers see BadKey rather
+ // than an opaque IO error from the kernel).
+ assert!(matches!(s.put("a\0b", b"x").await, Err(StorageError::BadKey)));
}
#[tokio::test]
diff --git a/backend/tests/api_auth.rs b/backend/tests/api_auth.rs
index 8ded505..827e6c2 100644
--- a/backend/tests/api_auth.rs
+++ b/backend/tests/api_auth.rs
@@ -567,6 +567,91 @@ async fn user_a_cannot_delete_user_b_token(pool: PgPool) {
assert_eq!(resp.status(), StatusCode::NO_CONTENT);
}
+/// Username enumeration via login response time: an attacker probes
+/// for valid usernames by measuring how long /auth/login takes. Before
+/// the equalisation fix, the no-user branch returned 401 in <1 ms
+/// while the wrong-password branch took ~50-100 ms (the argon2 verify
+/// cost). This test asserts the no-user branch now spends at least
+/// some meaningful fraction of the wrong-password branch's time.
+///
+/// Tolerance is intentionally loose so CI variance doesn't flap the
+/// test. The unequalised gap is large enough (~50x) that even a noisy
+/// CI run with a 5x slack still catches it.
+#[sqlx::test(migrations = "./migrations")]
+async fn login_no_user_branch_runs_argon2_for_timing_equalisation(pool: PgPool) {
+ use std::time::Instant;
+
+ let h = common::harness(pool);
+
+ // Register the victim user so the wrong-password branch has a real
+ // argon2 hash to verify against.
+ let _ = h
+ .app
+ .clone()
+ .oneshot(common::post_json(
+ "/api/v1/auth/register",
+ json!({ "username": "victim", "password": "hunter2hunter2" }),
+ ))
+ .await
+ .unwrap();
+
+ // Warm-up: first login of the process initialises the dummy hash
+ // lazily. Skip that cost when measuring.
+ let _ = h
+ .app
+ .clone()
+ .oneshot(common::post_json(
+ "/api/v1/auth/login",
+ json!({ "username": "victim", "password": "wrong" }),
+ ))
+ .await
+ .unwrap();
+ let _ = h
+ .app
+ .clone()
+ .oneshot(common::post_json(
+ "/api/v1/auth/login",
+ json!({ "username": "ghost", "password": "wrong" }),
+ ))
+ .await
+ .unwrap();
+
+ // Median-of-N is more stable than a single sample.
+ async fn sample_min(
+ app: &axum::Router,
+ username: &str,
+ n: u32,
+ ) -> std::time::Duration {
+ let mut samples = Vec::with_capacity(n as usize);
+ for _ in 0..n {
+ let req = common::post_json(
+ "/api/v1/auth/login",
+ json!({ "username": username, "password": "wrong-guess" }),
+ );
+ let t = Instant::now();
+ let resp = app.clone().oneshot(req).await.unwrap();
+ let d = t.elapsed();
+ assert_eq!(resp.status(), StatusCode::UNAUTHORIZED);
+ samples.push(d);
+ }
+ // Use the minimum: it's the floor that argon2 takes, robust
+ // against unrelated stalls (DB connection acquisition, etc.).
+ *samples.iter().min().unwrap()
+ }
+
+ let wrong_pwd = sample_min(&h.app, "victim", 3).await;
+ let no_user = sample_min(&h.app, "ghost", 3).await;
+
+ // 5x slack: argon2 dominates both branches, so they should be
+ // within an order of magnitude. Unequalised, no_user would be
+ // ~50-100x faster. Asserting "no_user >= wrong_pwd / 5" catches
+ // the bug without being flaky in CI.
+ assert!(
+ no_user * 5 >= wrong_pwd,
+ "login timing leaks user existence: no_user={no_user:?}, wrong_pwd={wrong_pwd:?}"
+ );
+}
+
#[sqlx::test(migrations = "./migrations")]
async fn delete_unknown_token_is_404(pool: PgPool) {
let h = common::harness(pool);
@@ -581,3 +666,27 @@ async fn delete_unknown_token_is_404(pool: PgPool) {
.unwrap();
assert_eq!(resp.status(), StatusCode::NOT_FOUND);
}
+
+/// Bot token names are user-supplied free-form strings; a 10 MB name
+/// was accepted before. Cap at 64 chars to match the other free-form
+/// identifier caps (tags, collection names). The response uses
+/// `ValidationFailed` (422 with per-field details) so clients can
+/// render the same shape they already handle for `attach_tag`.
+#[sqlx::test(migrations = "./migrations")]
+async fn create_token_rejects_name_over_64_chars(pool: PgPool) {
+ let h = common::harness(pool);
+ let (_, cookie) = common::register_user(&h.app).await;
+ let resp = h
+ .app
+ .oneshot(common::post_json_with_cookie(
+ "/api/v1/auth/tokens",
+ json!({ "name": "x".repeat(65) }),
+ &cookie,
+ ))
+ .await
+ .unwrap();
+ assert_eq!(resp.status(), StatusCode::UNPROCESSABLE_ENTITY);
+ let body = common::body_json(resp).await;
+ assert_eq!(body["error"]["code"], "validation_failed");
+ assert!(body["error"]["details"]["name"].is_string());
+}
diff --git a/backend/tests/api_mangas_cover.rs b/backend/tests/api_mangas_cover.rs
index 396eacd..398a3f3 100644
--- a/backend/tests/api_mangas_cover.rs
+++ b/backend/tests/api_mangas_cover.rs
@@ -410,3 +410,53 @@ async fn delete_cover_404_on_unknown_id(pool: PgPool) {
.unwrap();
assert_eq!(resp.status(), StatusCode::NOT_FOUND);
}
+
+/// Authz: PUT /mangas/:id/cover must be uploader-only.
+#[sqlx::test(migrations = "./migrations")]
+async fn put_cover_forbidden_for_non_uploader(pool: PgPool) {
+ let h = harness(pool);
+ let (_, owner_cookie) = register_user(&h.app).await;
+ let (_, intruder_cookie) = register_user(&h.app).await;
+
+ let manga =
+ create_manga_with_cover(&h.app, &owner_cookie, "Mine", None).await;
+ let id = id_of(&manga);
+
+ let resp = h
+ .app
+ .oneshot(put_multipart_with_cookie(
+ &format!("/api/v1/mangas/{id}/cover"),
+ cover_form(&fake_png_bytes()),
+ &intruder_cookie,
+ ))
+ .await
+ .unwrap();
+ assert_eq!(resp.status(), StatusCode::FORBIDDEN);
+}
+
+/// Authz: DELETE /mangas/:id/cover must be uploader-only.
+#[sqlx::test(migrations = "./migrations")]
+async fn delete_cover_forbidden_for_non_uploader(pool: PgPool) {
+ let h = harness(pool);
+ let (_, owner_cookie) = register_user(&h.app).await;
+ let (_, intruder_cookie) = register_user(&h.app).await;
+
+ let manga = create_manga_with_cover(
+ &h.app,
+ &owner_cookie,
+ "Mine",
+ Some(("image/jpeg", &fake_jpeg_bytes())),
+ )
+ .await;
+ let id = id_of(&manga);
+
+ let resp = h
+ .app
+ .oneshot(delete_with_cookie(
+ &format!("/api/v1/mangas/{id}/cover"),
+ &intruder_cookie,
+ ))
+ .await
+ .unwrap();
+ assert_eq!(resp.status(), StatusCode::FORBIDDEN);
+}
diff --git a/backend/tests/api_mangas_metadata.rs b/backend/tests/api_mangas_metadata.rs
index 2797d34..107f9d2 100644
--- a/backend/tests/api_mangas_metadata.rs
+++ b/backend/tests/api_mangas_metadata.rs
@@ -566,3 +566,78 @@ async fn patch_requires_authentication(pool: PgPool) {
.unwrap();
assert_eq!(resp.status(), StatusCode::UNAUTHORIZED);
}
+
+/// A signed-in user who didn't upload the manga must not be able to
+/// PATCH it. Without the uploader-gate this returned 200 — see
+/// REVIEW.md "manga PATCH / cover endpoints don't check ownership".
+#[sqlx::test(migrations = "./migrations")]
+async fn patch_forbidden_for_non_uploader(pool: PgPool) {
+ let h = common::harness(pool);
+ let (_, owner_cookie) = common::register_user(&h.app).await;
+ let (_, intruder_cookie) = common::register_user(&h.app).await;
+
+ let created = create_manga(&h.app, &owner_cookie, json!({ "title": "Mine" })).await;
+ let id = id_of(&created);
+
+ let resp = h
+ .app
+ .oneshot(common::patch_json_with_cookie(
+ &format!("/api/v1/mangas/{id}"),
+ json!({ "status": "completed" }),
+ &intruder_cookie,
+ ))
+ .await
+ .unwrap();
+ assert_eq!(resp.status(), StatusCode::FORBIDDEN);
+}
+
+/// Owner can still edit their own manga (regression guard for the
+/// authz fix).
+#[sqlx::test(migrations = "./migrations")]
+async fn patch_allowed_for_uploader(pool: PgPool) {
+ let h = common::harness(pool);
+ let (_, cookie) = common::register_user(&h.app).await;
+ let created = create_manga(&h.app, &cookie, json!({ "title": "Owned" })).await;
+ let id = id_of(&created);
+ let resp = h
+ .app
+ .oneshot(common::patch_json_with_cookie(
+ &format!("/api/v1/mangas/{id}"),
+ json!({ "status": "completed" }),
+ &cookie,
+ ))
+ .await
+ .unwrap();
+ assert_eq!(resp.status(), StatusCode::OK);
+}
+
+/// Legacy rows with `uploaded_by IS NULL` (created before migration
+/// 0011) remain editable by any signed-in user. Without this carve-out
+/// the historical-data note in 0011 would be broken.
+#[sqlx::test(migrations = "./migrations")]
+async fn patch_allowed_on_legacy_null_uploader(pool: PgPool) {
+ let h = common::harness(pool.clone());
+ let (_, cookie) = common::register_user(&h.app).await;
+ let created = create_manga(&h.app, &cookie, json!({ "title": "Legacy" })).await;
+ let id = id_of(&created);
+
+ // Simulate a row uploaded before the column existed: clear
+ // uploaded_by directly via SQL.
+ sqlx::query("UPDATE mangas SET uploaded_by = NULL WHERE id = $1")
+ .bind(id)
+ .execute(&pool)
+ .await
+ .unwrap();
+
+ let (_, other_cookie) = common::register_user(&h.app).await;
+ let resp = h
+ .app
+ .oneshot(common::patch_json_with_cookie(
+ &format!("/api/v1/mangas/{id}"),
+ json!({ "status": "completed" }),
+ &other_cookie,
+ ))
+ .await
+ .unwrap();
+ assert_eq!(resp.status(), StatusCode::OK);
+}
diff --git a/backend/tests/api_tags.rs b/backend/tests/api_tags.rs
index 1ae77b3..c959151 100644
--- a/backend/tests/api_tags.rs
+++ b/backend/tests/api_tags.rs
@@ -59,6 +59,31 @@ async fn reattach_same_tag_is_idempotent_and_returns_200(pool: PgPool) {
assert_eq!(second.status(), StatusCode::OK);
}
+/// Tag names over 64 chars are rejected at the handler boundary. The
+/// repo enforces the same cap, but doing it at the handler keeps the
+/// envelope consistent with the other validation paths
+/// (username, collection name, etc.).
+#[sqlx::test(migrations = "./migrations")]
+async fn attach_rejects_tag_name_over_64_chars(pool: PgPool) {
+ let h = common::harness(pool);
+ let (_, cookie) = common::register_user(&h.app).await;
+ let manga_id = common::seed_manga_via_api(&h.app, &cookie, "Berserk").await;
+
+ let long_name: String = "x".repeat(65);
+ let resp = h
+ .app
+ .oneshot(common::post_json_with_cookie(
+ &format!("/api/v1/mangas/{manga_id}/tags"),
+ json!({ "name": long_name }),
+ &cookie,
+ ))
+ .await
+ .unwrap();
+ assert_eq!(resp.status(), StatusCode::UNPROCESSABLE_ENTITY);
+ let body = common::body_json(resp).await;
+ assert_eq!(body["error"]["code"], "validation_failed");
+}
+
#[sqlx::test(migrations = "./migrations")]
async fn tag_names_dedup_case_insensitively(pool: PgPool) {
let h = common::harness(pool);
diff --git a/backend/tests/crawler_sync.rs b/backend/tests/crawler_sync.rs
index 27f6f3b..d6f7df2 100644
--- a/backend/tests/crawler_sync.rs
+++ b/backend/tests/crawler_sync.rs
@@ -440,6 +440,170 @@ async fn arbitrary_genres_from_source_get_inserted(pool: PgPool) {
assert_eq!(webtoons_count.0, 1, "case-insensitive lookup reuses the existing row");
}
+/// User-attached tags (rows with non-NULL `added_by` in `manga_tags`)
+/// must survive a crawler upsert. The crawler owns source-attached tags
+/// (added_by IS NULL); user attachments are owned by the user who made
+/// them and the recurring metadata pass must not delete them.
+#[sqlx::test(migrations = "./migrations")]
+async fn sync_tags_preserves_user_attached_tags(pool: PgPool) {
+ crawler::ensure_source(&pool, "target", "T", "https://x.example")
+ .await
+ .unwrap();
+ let m = sample_manga("foo", "Foo Manga", "hash-1");
+ let up = crawler::upsert_manga_from_source(&pool, "target", "https://x.example/foo", &m)
+ .await
+ .unwrap();
+
+ // A real user attaches a personal tag.
+ let user = mangalord::repo::user::create(&pool, "alice", "phc-stub")
+ .await
+ .unwrap();
+ let outcome = mangalord::repo::tag::attach_to_manga(&pool, up.manga_id, "personal", user.id)
+ .await
+ .unwrap();
+ assert!(outcome.created_attachment);
+
+ // Second crawler pass. Use a different metadata_hash so the upsert
+ // takes the Updated branch, but the bug also fires on Unchanged
+ // ticks since sync_tags runs unconditionally.
+ let mut m2 = m.clone();
+ m2.metadata_hash = "hash-2".into();
+ m2.tags = vec!["popular".into(), "weekly".into()];
+ let _ = crawler::upsert_manga_from_source(&pool, "target", "https://x.example/foo", &m2)
+ .await
+ .unwrap();
+
+ // The user tag must still be attached.
+ let user_tag_rows: (i64,) = sqlx::query_as(
+ "SELECT COUNT(*) FROM manga_tags mt \
+ JOIN tags t ON t.id = mt.tag_id \
+ WHERE mt.manga_id = $1 AND lower(t.name) = 'personal' \
+ AND mt.added_by = $2",
+ )
+ .bind(up.manga_id)
+ .bind(user.id)
+ .fetch_one(&pool)
+ .await
+ .unwrap();
+ assert_eq!(
+ user_tag_rows.0, 1,
+ "user-attached tag must survive a crawler upsert"
+ );
+
+ // The source's tags should still attach as well, as crawler-owned.
+ let source_tag_rows: (i64,) = sqlx::query_as(
+ "SELECT COUNT(*) FROM manga_tags mt \
+ JOIN tags t ON t.id = mt.tag_id \
+ WHERE mt.manga_id = $1 \
+ AND mt.added_by IS NULL \
+ AND lower(t.name) IN ('popular', 'weekly')",
+ )
+ .bind(up.manga_id)
+ .fetch_one(&pool)
+ .await
+ .unwrap();
+ assert_eq!(source_tag_rows.0, 2, "source tags re-attach on each pass");
+
+ // A subsequent pass where the source drops a previously-seen tag
+ // must clear that crawler-owned attachment (otherwise crawler-tags
+ // would only ever accumulate).
+ let mut m3 = m2.clone();
+ m3.metadata_hash = "hash-3".into();
+ m3.tags = vec!["popular".into()];
+ let _ = crawler::upsert_manga_from_source(&pool, "target", "https://x.example/foo", &m3)
+ .await
+ .unwrap();
+ let weekly_rows: (i64,) = sqlx::query_as(
+ "SELECT COUNT(*) FROM manga_tags mt \
+ JOIN tags t ON t.id = mt.tag_id \
+ WHERE mt.manga_id = $1 AND lower(t.name) = 'weekly'",
+ )
+ .bind(up.manga_id)
+ .fetch_one(&pool)
+ .await
+ .unwrap();
+ assert_eq!(weekly_rows.0, 0, "source-owned tag dropped by source goes away");
+
+ // And the user tag still survives that third pass.
+ let user_tag_rows: (i64,) = sqlx::query_as(
+ "SELECT COUNT(*) FROM manga_tags mt \
+ JOIN tags t ON t.id = mt.tag_id \
+ WHERE mt.manga_id = $1 AND lower(t.name) = 'personal' \
+ AND mt.added_by = $2",
+ )
+ .bind(up.manga_id)
+ .bind(user.id)
+ .fetch_one(&pool)
+ .await
+ .unwrap();
+ assert_eq!(user_tag_rows.0, 1);
+}
+
+/// `manga_tags.added_by` is `ON DELETE SET NULL` on the user FK. When
+/// the attaching user is deleted, their attachments become orphans
+/// indistinguishable from crawler-owned rows — and the crawler should
+/// reap them on the next pass. Pins the semantic so a future change
+/// can't quietly leave orphan rows lying around.
+#[sqlx::test(migrations = "./migrations")]
+async fn sync_tags_garbage_collects_orphan_user_attachments(pool: PgPool) {
+ crawler::ensure_source(&pool, "target", "T", "https://x.example")
+ .await
+ .unwrap();
+ let m = sample_manga("foo", "Foo", "hash-1");
+ let up = crawler::upsert_manga_from_source(&pool, "target", "https://x.example/foo", &m)
+ .await
+ .unwrap();
+
+ // A user attaches "personal", then the user gets deleted. The
+ // attachment row stays (manga_tags.manga_id FK is CASCADE on
+ // mangas only; we never CASCADE-delete user attachments). The FK
+ // on added_by is `ON DELETE SET NULL`, so the row's owner column
+ // goes NULL — same shape as a crawler-owned row.
+ let user = mangalord::repo::user::create(&pool, "bob", "phc-stub")
+ .await
+ .unwrap();
+ let _ = mangalord::repo::tag::attach_to_manga(&pool, up.manga_id, "personal", user.id)
+ .await
+ .unwrap();
+ sqlx::query("DELETE FROM users WHERE id = $1")
+ .bind(user.id)
+ .execute(&pool)
+ .await
+ .unwrap();
+
+ // Sanity: the orphan still exists post-user-delete with added_by NULL.
+ let (orphan_rows,): (i64,) = sqlx::query_as(
+ "SELECT COUNT(*) FROM manga_tags mt \
+ JOIN tags t ON t.id = mt.tag_id \
+ WHERE mt.manga_id = $1 AND lower(t.name) = 'personal' \
+ AND mt.added_by IS NULL",
+ )
+ .bind(up.manga_id)
+ .fetch_one(&pool)
+ .await
+ .unwrap();
+ assert_eq!(orphan_rows, 1);
+
+ // Next crawler pass — orphan should be reaped along with any
+ // other source-owned rows that aren't in the new tag list.
+ let mut m2 = m.clone();
+ m2.metadata_hash = "hash-2".into();
+ m2.tags = vec!["popular".into()];
+ let _ = crawler::upsert_manga_from_source(&pool, "target", "https://x.example/foo", &m2)
+ .await
+ .unwrap();
+ let (orphan_rows,): (i64,) = sqlx::query_as(
+ "SELECT COUNT(*) FROM manga_tags mt \
+ JOIN tags t ON t.id = mt.tag_id \
+ WHERE mt.manga_id = $1 AND lower(t.name) = 'personal'",
+ )
+ .bind(up.manga_id)
+ .fetch_one(&pool)
+ .await
+ .unwrap();
+ assert_eq!(orphan_rows, 0, "orphan user-attached tag should be reaped");
+}
+
#[sqlx::test(migrations = "./migrations")]
async fn re_appearing_manga_clears_dropped_at(pool: PgPool) {
crawler::ensure_source(&pool, "target", "T", "https://x.example")
diff --git a/frontend/package.json b/frontend/package.json
index 159dad9..72a32cd 100644
--- a/frontend/package.json
+++ b/frontend/package.json
@@ -1,6 +1,6 @@
{
"name": "mangalord-frontend",
- "version": "0.34.0",
+ "version": "0.34.1",
"private": true,
"type": "module",
"scripts": {
diff --git a/frontend/src/lib/api/auth.test.ts b/frontend/src/lib/api/auth.test.ts
index 7826151..5281f93 100644
--- a/frontend/src/lib/api/auth.test.ts
+++ b/frontend/src/lib/api/auth.test.ts
@@ -94,6 +94,11 @@ describe('auth api client', () => {
expect(url).toMatch(/\/v1\/auth\/logout$/);
const init = fetchSpy.mock.calls[0][1] as RequestInit;
expect(init.method).toBe('POST');
+ // Consistent content-type for all mutation requests, matching
+ // the rest of the module — axum doesn't require it but the
+ // header keeps the request style uniform.
+ const headers = new Headers(init.headers);
+ expect(headers.get('content-type')).toBe('application/json');
});
it('me returns the user on 200', async () => {
diff --git a/frontend/src/lib/api/auth.ts b/frontend/src/lib/api/auth.ts
index bc9f905..4102325 100644
--- a/frontend/src/lib/api/auth.ts
+++ b/frontend/src/lib/api/auth.ts
@@ -32,7 +32,14 @@ export async function login(creds: Credentials): Promise {
}
export async function logout(): Promise {
- await request('/v1/auth/logout', { method: 'POST' });
+ await request('/v1/auth/logout', {
+ method: 'POST',
+ // Consistent with the other POST/PATCH helpers in this module.
+ // axum doesn't require it (no body), but keeping the header
+ // on every mutation request avoids the false-flag in logs and
+ // matches the project's style.
+ headers: { 'content-type': 'application/json' }
+ });
}
export type ChangePassword = {
diff --git a/frontend/src/routes/manga/[id]/chapter/[chapter_id]/+page.svelte b/frontend/src/routes/manga/[id]/chapter/[chapter_id]/+page.svelte
index e0334b2..ed81173 100644
--- a/frontend/src/routes/manga/[id]/chapter/[chapter_id]/+page.svelte
+++ b/frontend/src/routes/manga/[id]/chapter/[chapter_id]/+page.svelte
@@ -350,54 +350,48 @@
});
/**
- * `fetch()` initiated during `pagehide` / `beforeunload` is
- * cancelled by every browser by default. `sendBeacon` is the
- * supported way to ship a small payload during unload — it's
- * guaranteed to survive even if the tab is closing. Failure here
- * is silent because the API is fire-and-forget.
+ * Flush read-progress as the tab is closing. A plain `fetch()`
+ * during `pagehide` / `beforeunload` is cancelled by every
+ * browser; `fetch(..., { keepalive: true })` is the supported
+ * escape hatch and survives the close.
+ *
+ * `sendBeacon` would be the textbook alternative, but it's
+ * POST-only and `/me/read-progress` takes PUT — so a beacon
+ * always 405s, adds server-log noise, then falls through to this
+ * same keepalive path anyway. The beacon was dropped; the
+ * keepalive fetch is the only path.
*/
- function beaconFinalProgress() {
+ function flushFinalProgress() {
if (!session.user) return;
const body = JSON.stringify({
manga_id: manga.id,
chapter_id: chapter.id,
page: progressPage
});
- const blob = new Blob([body], { type: 'application/json' });
- // sendBeacon only supports POST — the server's PUT route is
- // strict on method. The dedicated POST alias is omitted; in
- // practice the in-app navigation path (back-link, chapter
- // links) already covers the common-case unmount via the
- // onDestroy fetch. Fall through to fetch+keepalive for browser
- // implementations that don't honor sendBeacon for this endpoint.
try {
- const ok = navigator.sendBeacon('/api/v1/me/read-progress', blob);
- if (!ok) throw new Error('sendBeacon rejected');
+ void fetch('/api/v1/me/read-progress', {
+ method: 'PUT',
+ headers: { 'content-type': 'application/json' },
+ body,
+ keepalive: true,
+ credentials: 'include'
+ });
} catch {
- try {
- void fetch('/api/v1/me/read-progress', {
- method: 'PUT',
- headers: { 'content-type': 'application/json' },
- body,
- keepalive: true,
- credentials: 'include'
- });
- } catch {
- // Final fallback failed; the in-app onDestroy flush
- // below catches the SPA-navigation case.
- }
+ // keepalive fetch was rejected (very old Firefox etc.);
+ // the in-app onDestroy flush below catches the SPA-
+ // navigation case, which is the common one anyway.
}
}
onMount(() => {
- window.addEventListener('pagehide', beaconFinalProgress);
+ window.addEventListener('pagehide', flushFinalProgress);
});
onDestroy(() => {
observer?.disconnect();
if (progressTimer) clearTimeout(progressTimer);
if (typeof window !== 'undefined') {
- window.removeEventListener('pagehide', beaconFinalProgress);
+ window.removeEventListener('pagehide', flushFinalProgress);
}
// Don't let the fullscreen flag leak to non-reader pages —
// otherwise the layout header would stay slid-off on /upload