Compare commits
12 Commits
fix/reject
...
fix/video-
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
402215d405 | ||
|
|
2b313e67e0 | ||
|
|
2551c25436 | ||
|
|
d51c6b8c4b | ||
|
|
3bcb7c6a76 | ||
|
|
08d92b7531 | ||
|
|
06bc9ddcb3 | ||
|
|
5f702f2b40 | ||
|
|
31faccfdf8 | ||
|
|
06ade4e158 | ||
|
|
b601c062bd | ||
|
|
0c0eed885a |
12
.env.example
12
.env.example
@@ -12,6 +12,14 @@ APP_ENV=production
|
||||
# ── Database ──────────────────────────────────────────────────────────────────
|
||||
# Set a strong password and keep it in sync between DATABASE_URL and
|
||||
# POSTGRES_PASSWORD. Generate one with: openssl rand -hex 24
|
||||
#
|
||||
# SET THIS BEFORE THE FIRST `docker compose up -d`. Postgres reads POSTGRES_PASSWORD
|
||||
# only when it initialises its data directory, on that very first boot. Change it
|
||||
# afterwards and the app authenticates with the new password against a volume still
|
||||
# holding the old one — a permanent restart loop ("password authentication failed").
|
||||
# The only ways out are restoring the old password or `docker compose down -v`, which
|
||||
# deletes the database, the media and the exports. In production the app refuses to
|
||||
# boot while this is still the placeholder below, so it cannot be missed by accident.
|
||||
DATABASE_URL=postgres://eventsnap:CHANGE_ME_use_a_strong_password@db:5432/eventsnap
|
||||
POSTGRES_USER=eventsnap
|
||||
POSTGRES_PASSWORD=CHANGE_ME_use_a_strong_password
|
||||
@@ -30,7 +38,9 @@ JWT_SECRET=change_me_to_a_random_64_byte_hex_string
|
||||
SESSION_EXPIRY_DAYS=30
|
||||
|
||||
# Admin dashboard password (bcrypt hash).
|
||||
# Generate with: htpasswd -bnBC 12 "" yourpassword | tr -d ':\n'
|
||||
# Generate with an image the stack already pulls (htpasswd needs apache2-utils, which
|
||||
# a stock VPS does not have):
|
||||
# docker run --rm caddy:2-alpine caddy hash-password --plaintext 'yourpassword'
|
||||
# IMPORTANT: keep the SINGLE QUOTES. A bcrypt hash is full of `$` (e.g. $2b$12$…$…),
|
||||
# and both Docker Compose's env_file interpolation and dotenvy's variable substitution
|
||||
# would otherwise eat the `$…` segments (reading them as unset vars) and corrupt the
|
||||
|
||||
12
.gitignore
vendored
12
.gitignore
vendored
@@ -13,8 +13,16 @@ frontend/build/
|
||||
frontend/export-viewer/node_modules/
|
||||
frontend/export-viewer/.svelte-kit/
|
||||
|
||||
# Media uploads (mounted volume in production)
|
||||
media/
|
||||
# Media uploads. In production these live in the `media_data` DOCKER VOLUME, never in the
|
||||
# working tree — so this pattern is anchored to the repo root and exists only for a local
|
||||
# bind-mount experiment.
|
||||
#
|
||||
# It used to read `media/`, unanchored, which matches a directory of that name at ANY depth.
|
||||
# The only one in the repo is `e2e/fixtures/media/`, so the rule's entire practical effect was
|
||||
# to keep every E2E fixture untracked: a fresh clone got the specs and none of the images or
|
||||
# videos they read. `.github/workflows/e2e.yml` does a plain checkout and generates nothing, so
|
||||
# the committed CI job could not have run the upload, video or export suites at all.
|
||||
/media/
|
||||
|
||||
# Playwright E2E suite — runtime artifacts (the suite itself is committed)
|
||||
e2e/node_modules/
|
||||
|
||||
53
README.md
53
README.md
@@ -97,17 +97,51 @@ eventsnap/
|
||||
git clone https://git.mc02.dev/fabi/EventSnap.git eventsnap
|
||||
cd eventsnap
|
||||
|
||||
# 2. Configure environment
|
||||
# 2. Configure environment — set EVERY secret NOW, before step 3.
|
||||
cp .env.example .env
|
||||
nano .env # set DOMAIN, JWT_SECRET, ADMIN_PASSWORD_HASH, EVENT_NAME, etc.
|
||||
nano .env # DOMAIN, EVENT_NAME, EVENT_SLUG,
|
||||
# JWT_SECRET, ADMIN_PASSWORD_HASH,
|
||||
# POSTGRES_PASSWORD *and* the same password inside DATABASE_URL
|
||||
# (see "Generate required secrets" below)
|
||||
|
||||
# 3. Start the stack
|
||||
docker compose up -d
|
||||
```
|
||||
|
||||
> **Set every secret before step 3 — `POSTGRES_PASSWORD` especially.** Postgres reads it **only
|
||||
> when it initialises its data directory**, which happens on the very first `docker compose up -d`.
|
||||
> Changing it in `.env` afterwards does not change the stored password: the app then authenticates
|
||||
> with the new one against a volume holding the old one, and you get a permanent restart loop with
|
||||
> `password authentication failed for user "eventsnap"`. The only fixes are restoring the old
|
||||
> password or `docker compose down -v`, which **deletes the database, the media and the exports**.
|
||||
> Getting it right once, up front, costs nothing; getting it wrong costs the volume.
|
||||
|
||||
Caddy automatically obtains a Let's Encrypt certificate on first start. The app is live at `https://DOMAIN` within ~30 seconds.
|
||||
|
||||
> **If the site never comes up:** with `APP_ENV=production` the backend **refuses to boot** while `JWT_SECRET`/`ADMIN_PASSWORD_HASH` still hold the `.env.example` placeholders (this is deliberate — a publicly-known signing key is worse than downtime). Caddy then waits on the unhealthy `app` container and never serves. Check `docker compose logs app` — a "Refusing to start … placeholder …" line means you skipped step 2. Rotate the secrets (see below) and restart.
|
||||
> **If the site never comes up:** with `APP_ENV=production` the backend **refuses to boot** while
|
||||
> `JWT_SECRET`, `ADMIN_PASSWORD_HASH` or the password inside `DATABASE_URL` still hold the
|
||||
> `.env.example` placeholders (this is deliberate — a publicly-known signing key or database
|
||||
> password is worse than downtime). Caddy then waits on the unhealthy `app` container and never
|
||||
> serves. Check `docker compose logs app` — a "Refusing to start … placeholder …" line lists
|
||||
> **every** unset secret at once, so one edit fixes them all.
|
||||
>
|
||||
> **If it comes up but keeps restarting with `password authentication failed for user
|
||||
> "eventsnap"`:** `POSTGRES_PASSWORD` was changed after the database volume was created. Postgres
|
||||
> applies that variable only at initialisation, so `.env` and the stored password have drifted
|
||||
> apart permanently. `docker compose logs app` spells this out. Before the event, with nothing
|
||||
> worth keeping:
|
||||
>
|
||||
> ```bash
|
||||
> docker compose down -v && docker compose up -d # -v DELETES db + media + exports. No undo.
|
||||
> ```
|
||||
>
|
||||
> **Once the event has real data, never do that.** Put the original password back into
|
||||
> `DATABASE_URL`, or change the stored one instead:
|
||||
>
|
||||
> ```bash
|
||||
> docker compose exec db psql -U "$POSTGRES_USER" -c \
|
||||
> "ALTER ROLE eventsnap WITH PASSWORD 'the-password-now-in-your-.env';"
|
||||
> ```
|
||||
|
||||
> **Production note:** `docker compose up -d` does **not** expose the database — Postgres is reachable only on the internal Docker network. For local development where you need host access to Postgres, opt into the dev overlay explicitly:
|
||||
> ```bash
|
||||
@@ -179,10 +213,19 @@ TLS certificate and all data volumes survive.
|
||||
# JWT secret (64 random bytes)
|
||||
openssl rand -hex 64
|
||||
|
||||
# Admin password hash (bcrypt, cost 12)
|
||||
htpasswd -bnBC 12 "" yourpassword | tr -d ':\n'
|
||||
# Database password (goes in BOTH DATABASE_URL and POSTGRES_PASSWORD)
|
||||
openssl rand -hex 24
|
||||
|
||||
# Admin password hash (bcrypt). Uses an image the stack already pulls, so it needs
|
||||
# nothing installed on the host — `htpasswd` lives in apache2-utils, which a stock
|
||||
# VPS does not have. Emits cost 14 rather than 12; that is fine (admin login is
|
||||
# rate-limited and hashed off the async runtime), and any $2a/$2b/$2y hash verifies.
|
||||
docker run --rm caddy:2-alpine caddy hash-password --plaintext 'yourpassword'
|
||||
```
|
||||
|
||||
Wrap the resulting hash in **single quotes** in `.env` — see the note there; a bcrypt
|
||||
hash is full of `$`, and both Compose and dotenvy would otherwise eat those segments.
|
||||
|
||||
### Environment Variables
|
||||
|
||||
See [.env.example](.env.example) for the full list with descriptions and defaults. Key variables:
|
||||
|
||||
@@ -20,21 +20,53 @@ fn looks_placeholder(s: &str) -> bool {
|
||||
/// Enforce secret hygiene. In production every guard is hard-fail: a booting app
|
||||
/// with a publicly-known signing key is worse than one that refuses to start.
|
||||
/// Outside production the dev sentinel is tolerated (warned) so local dev is frictionless.
|
||||
fn validate_secrets(is_prod: bool, jwt_secret: &str, admin_password_hash: &str) -> Result<()> {
|
||||
///
|
||||
/// EVERY failure is collected and reported together. Returning on the first one made fixing two
|
||||
/// secrets cost two boot cycles — the operator rotates JWT_SECRET, restarts, and only then learns
|
||||
/// about ADMIN_PASSWORD_HASH. Restarting this stack is not free (Caddy waits on the unhealthy app),
|
||||
/// and each avoidable cycle is another chance to reach for `down -v`.
|
||||
fn validate_secrets(
|
||||
is_prod: bool,
|
||||
jwt_secret: &str,
|
||||
admin_password_hash: &str,
|
||||
database_url: &str,
|
||||
) -> Result<()> {
|
||||
if is_prod {
|
||||
let mut problems: Vec<&str> = Vec::new();
|
||||
if looks_placeholder(jwt_secret) {
|
||||
return Err(anyhow!(
|
||||
"Refusing to start in production with a placeholder JWT_SECRET — \
|
||||
rotate it (openssl rand -hex 64)."
|
||||
));
|
||||
}
|
||||
if jwt_secret.len() < 32 {
|
||||
return Err(anyhow!("JWT_SECRET must be at least 32 characters."));
|
||||
problems.push(
|
||||
"JWT_SECRET is still the .env.example placeholder — rotate it \
|
||||
(openssl rand -hex 64).",
|
||||
);
|
||||
} else if jwt_secret.len() < 32 {
|
||||
problems.push("JWT_SECRET must be at least 32 characters.");
|
||||
}
|
||||
if admin_password_hash.is_empty() || looks_placeholder(admin_password_hash) {
|
||||
problems.push(
|
||||
"ADMIN_PASSWORD_HASH is unset or still the .env.example placeholder — generate one \
|
||||
(docker run --rm caddy:2-alpine caddy hash-password --plaintext '<password>').",
|
||||
);
|
||||
}
|
||||
// The DATABASE_URL carries the Postgres password, so a placeholder here means the stack is
|
||||
// running on `CHANGE_ME_use_a_strong_password` — a credential published in the repo. The
|
||||
// app used to boot green on it, because this guard only ever covered the two secrets it
|
||||
// was written for and nothing else looked at POSTGRES_PASSWORD at all.
|
||||
//
|
||||
// Read POSTGRES_PASSWORD's docs before changing this: it is applied ONLY at initdb, so the
|
||||
// remedy is not "edit .env and restart" — see the 28P01 diagnostic in db.rs.
|
||||
if looks_placeholder(database_url) {
|
||||
problems.push(
|
||||
"DATABASE_URL still carries the .env.example placeholder password — set a strong \
|
||||
one (openssl rand -hex 24) in BOTH DATABASE_URL and POSTGRES_PASSWORD.",
|
||||
);
|
||||
}
|
||||
if !problems.is_empty() {
|
||||
return Err(anyhow!(
|
||||
"Refusing to start in production without a real ADMIN_PASSWORD_HASH — \
|
||||
generate one (htpasswd -bnBC 12 '' <password> | tr -d ':\\n')."
|
||||
"Refusing to start in production — {} secret(s) still unset or placeholder:\n - {}\n\
|
||||
ALL secrets must be set BEFORE the first `docker compose up -d`: Postgres bakes \
|
||||
POSTGRES_PASSWORD into its data directory on first boot and ignores later changes.",
|
||||
problems.len(),
|
||||
problems.join("\n - ")
|
||||
));
|
||||
}
|
||||
} else if jwt_secret == DEV_JWT_SECRET_SENTINEL {
|
||||
@@ -89,11 +121,12 @@ impl AppConfig {
|
||||
|
||||
let jwt_secret = std::env::var("JWT_SECRET").context("JWT_SECRET must be set")?;
|
||||
let admin_password_hash = std::env::var("ADMIN_PASSWORD_HASH").unwrap_or_default();
|
||||
let database_url = std::env::var("DATABASE_URL").context("DATABASE_URL must be set")?;
|
||||
|
||||
validate_secrets(is_prod, &jwt_secret, &admin_password_hash)?;
|
||||
validate_secrets(is_prod, &jwt_secret, &admin_password_hash, &database_url)?;
|
||||
|
||||
Ok(Self {
|
||||
database_url: std::env::var("DATABASE_URL").context("DATABASE_URL must be set")?,
|
||||
database_url,
|
||||
jwt_secret,
|
||||
session_expiry_days: std::env::var("SESSION_EXPIRY_DAYS")
|
||||
.unwrap_or_else(|_| "30".to_string())
|
||||
@@ -141,12 +174,18 @@ mod tests {
|
||||
|
||||
const REAL_SECRET: &str = "a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6e7f8a9b0c1d2";
|
||||
const REAL_HASH: &str = "$2y$12$abcdefghijklmnopqrstuv.wxyzABCDEFGHIJKLMNOPQRSTUVWXYZ012";
|
||||
const REAL_DB_URL: &str = "postgres://eventsnap:7f3a9c1e5b2d8a4f@db:5432/eventsnap";
|
||||
|
||||
#[test]
|
||||
fn prod_rejects_shipped_placeholder_secret() {
|
||||
// The exact string shipped in `.env` — >32 chars, so it must be caught by
|
||||
// the substring guard, not the length check.
|
||||
let err = validate_secrets(true, "change_me_to_a_random_64_byte_hex_string", REAL_HASH);
|
||||
let err = validate_secrets(
|
||||
true,
|
||||
"change_me_to_a_random_64_byte_hex_string",
|
||||
REAL_HASH,
|
||||
REAL_DB_URL,
|
||||
);
|
||||
assert!(
|
||||
err.is_err(),
|
||||
"placeholder JWT_SECRET must be rejected in prod"
|
||||
@@ -155,29 +194,108 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn prod_rejects_dev_sentinel_and_short_secret() {
|
||||
assert!(validate_secrets(true, DEV_JWT_SECRET_SENTINEL, REAL_HASH).is_err());
|
||||
assert!(validate_secrets(true, "tooshort", REAL_HASH).is_err());
|
||||
assert!(validate_secrets(true, DEV_JWT_SECRET_SENTINEL, REAL_HASH, REAL_DB_URL).is_err());
|
||||
assert!(validate_secrets(true, "tooshort", REAL_HASH, REAL_DB_URL).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn prod_rejects_missing_or_placeholder_admin_hash() {
|
||||
assert!(validate_secrets(true, REAL_SECRET, "").is_err());
|
||||
assert!(validate_secrets(true, REAL_SECRET, "$2y$12$placeholder_replace_me").is_err());
|
||||
assert!(validate_secrets(true, REAL_SECRET, "", REAL_DB_URL).is_err());
|
||||
assert!(
|
||||
validate_secrets(
|
||||
true,
|
||||
REAL_SECRET,
|
||||
"$2y$12$placeholder_replace_me",
|
||||
REAL_DB_URL
|
||||
)
|
||||
.is_err()
|
||||
);
|
||||
}
|
||||
|
||||
/// The stack used to come up GREEN on the database password published in the repo: this guard
|
||||
/// covered the two secrets it was written for, and nothing anywhere looked at the Postgres
|
||||
/// credential. README step 2 doesn't name POSTGRES_PASSWORD either, so following the
|
||||
/// documented procedure verbatim shipped it.
|
||||
#[test]
|
||||
fn prod_rejects_the_shipped_placeholder_database_password() {
|
||||
let shipped = "postgres://eventsnap:CHANGE_ME_use_a_strong_password@db:5432/eventsnap";
|
||||
let err = validate_secrets(true, REAL_SECRET, REAL_HASH, shipped).unwrap_err();
|
||||
assert!(
|
||||
err.to_string().contains("DATABASE_URL"),
|
||||
"the refusal must name DATABASE_URL, not just fail: {err}"
|
||||
);
|
||||
// And it must point at the initdb trap, or the operator edits .env, restarts, and lands
|
||||
// in a permanent auth-failure loop instead.
|
||||
assert!(
|
||||
err.to_string().contains("POSTGRES_PASSWORD"),
|
||||
"the refusal must name POSTGRES_PASSWORD as the other half: {err}"
|
||||
);
|
||||
}
|
||||
|
||||
/// Every problem in ONE message. Reporting them one per boot made fixing two secrets cost two
|
||||
/// restart cycles, on a stack where Caddy waits on the unhealthy app the whole time.
|
||||
#[test]
|
||||
fn prod_reports_every_placeholder_at_once() {
|
||||
let err = validate_secrets(
|
||||
true,
|
||||
"change_me_to_a_random_64_byte_hex_string",
|
||||
"$2y$12$placeholder_replace_me",
|
||||
"postgres://eventsnap:CHANGE_ME_use_a_strong_password@db:5432/eventsnap",
|
||||
)
|
||||
.unwrap_err()
|
||||
.to_string();
|
||||
for expected in ["JWT_SECRET", "ADMIN_PASSWORD_HASH", "DATABASE_URL"] {
|
||||
assert!(err.contains(expected), "{expected} missing from: {err}");
|
||||
}
|
||||
assert!(
|
||||
err.contains("3 secret(s)"),
|
||||
"the count must match what is listed: {err}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn prod_accepts_real_secrets() {
|
||||
assert!(validate_secrets(true, REAL_SECRET, REAL_HASH).is_ok());
|
||||
assert!(validate_secrets(true, REAL_SECRET, REAL_HASH, REAL_DB_URL).is_ok());
|
||||
}
|
||||
|
||||
/// A real password that happens to contain no placeholder substring must pass — including one
|
||||
/// with URL-ish punctuation, so the guard can't be mistaken for a URL validator.
|
||||
#[test]
|
||||
fn prod_accepts_a_real_database_url_with_awkward_punctuation() {
|
||||
assert!(
|
||||
validate_secrets(
|
||||
true,
|
||||
REAL_SECRET,
|
||||
REAL_HASH,
|
||||
"postgres://eventsnap:aB3%24xY9-_.qW@db:5432/eventsnap"
|
||||
)
|
||||
.is_ok()
|
||||
);
|
||||
}
|
||||
|
||||
/// The e2e stack runs without APP_ENV=production, so none of this applies there — but assert
|
||||
/// it, because a guard that tripped in e2e would be found the hard way.
|
||||
#[test]
|
||||
fn non_prod_ignores_a_placeholder_database_url() {
|
||||
assert!(
|
||||
validate_secrets(
|
||||
false,
|
||||
REAL_SECRET,
|
||||
"",
|
||||
"postgres://eventsnap:CHANGE_ME_use_a_strong_password@db:5432/eventsnap"
|
||||
)
|
||||
.is_ok()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn non_prod_tolerates_dev_sentinel() {
|
||||
assert!(validate_secrets(false, DEV_JWT_SECRET_SENTINEL, "").is_ok());
|
||||
assert!(validate_secrets(false, DEV_JWT_SECRET_SENTINEL, "", REAL_DB_URL).is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn non_prod_still_rejects_short_non_sentinel_secret() {
|
||||
assert!(validate_secrets(false, "tooshort", "").is_err());
|
||||
assert!(validate_secrets(false, "tooshort", "", REAL_DB_URL).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -185,9 +303,23 @@ mod tests {
|
||||
// looks_placeholder lowercases before matching — an upper/mixed-case
|
||||
// placeholder must still be rejected in prod.
|
||||
assert!(
|
||||
validate_secrets(true, "CHANGE_ME_TO_A_RANDOM_64_BYTE_HEX_STRING", REAL_HASH).is_err()
|
||||
validate_secrets(
|
||||
true,
|
||||
"CHANGE_ME_TO_A_RANDOM_64_BYTE_HEX_STRING",
|
||||
REAL_HASH,
|
||||
REAL_DB_URL
|
||||
)
|
||||
.is_err()
|
||||
);
|
||||
assert!(
|
||||
validate_secrets(
|
||||
true,
|
||||
REAL_SECRET,
|
||||
"$2Y$12$PLACEHOLDER_replace_me",
|
||||
REAL_DB_URL
|
||||
)
|
||||
.is_err()
|
||||
);
|
||||
assert!(validate_secrets(true, REAL_SECRET, "$2Y$12$PLACEHOLDER_replace_me").is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -197,7 +329,7 @@ mod tests {
|
||||
const LEN_31: &str = "abcdefghijklmnopqrstuvwxyz01234";
|
||||
assert_eq!(LEN_32.len(), 32);
|
||||
assert_eq!(LEN_31.len(), 31);
|
||||
assert!(validate_secrets(true, LEN_32, REAL_HASH).is_ok());
|
||||
assert!(validate_secrets(true, LEN_31, REAL_HASH).is_err());
|
||||
assert!(validate_secrets(true, LEN_32, REAL_HASH, REAL_DB_URL).is_ok());
|
||||
assert!(validate_secrets(true, LEN_31, REAL_HASH, REAL_DB_URL).is_err());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -4,17 +4,65 @@ use sqlx::postgres::PgPoolOptions;
|
||||
|
||||
const DEFAULT_MAX_CONNECTIONS: u32 = 10;
|
||||
|
||||
/// SQLSTATE for `invalid_password`.
|
||||
const PG_INVALID_PASSWORD: &str = "28P01";
|
||||
|
||||
/// Turn the one connect failure with an unguessable cause into a self-explaining one.
|
||||
///
|
||||
/// `POSTGRES_PASSWORD` is honoured ONLY when Postgres initialises its data directory. Change it in
|
||||
/// `.env` afterwards and the app authenticates with the new password against a volume that still
|
||||
/// holds the old one — a permanent restart loop whose only symptom is
|
||||
/// `password authentication failed`.
|
||||
///
|
||||
/// The production secret guard makes that sequence NEARLY CERTAIN rather than rare: it stops the
|
||||
/// app on the first `docker compose up -d`, but not the `db` service in that same command, which
|
||||
/// initialises and bakes in whatever password was in `.env` at that moment. So the intended
|
||||
/// recovery — see the refusal, fix your secrets, boot again — is exactly the sequence that breaks
|
||||
/// it. Nothing in the error names the cause, and the remedy destroys data, so it is the last thing
|
||||
/// an operator should guess at.
|
||||
fn explain_auth_failure(err: &sqlx::Error) {
|
||||
let is_auth_failure = match err {
|
||||
sqlx::Error::Database(db) => db.code().as_deref() == Some(PG_INVALID_PASSWORD),
|
||||
_ => false,
|
||||
};
|
||||
if !is_auth_failure {
|
||||
return;
|
||||
}
|
||||
tracing::error!(
|
||||
"Postgres rejected the credentials in DATABASE_URL (SQLSTATE {PG_INVALID_PASSWORD}).\n\
|
||||
\n\
|
||||
This almost always means POSTGRES_PASSWORD was changed AFTER the database volume was \
|
||||
first created. Postgres applies that variable only when it initialises its data \
|
||||
directory; editing .env and restarting does not change the stored password, so the two \
|
||||
drift apart permanently.\n\
|
||||
\n\
|
||||
If the event has NOT started and you have no data worth keeping:\n\n \
|
||||
docker compose down -v && docker compose up -d\n\n\
|
||||
(-v DELETES the database, the uploaded media and the exports. There is no undo.)\n\
|
||||
\n\
|
||||
If you DO have data: restore the old password into DATABASE_URL instead, or change the \
|
||||
stored one with ALTER ROLE inside the running db container. Never reach for -v to fix a \
|
||||
login problem on a live event."
|
||||
);
|
||||
}
|
||||
|
||||
pub async fn create_pool(database_url: &str) -> Result<PgPool> {
|
||||
let max_connections = std::env::var("DATABASE_MAX_CONNECTIONS")
|
||||
.ok()
|
||||
.and_then(|s| s.parse::<u32>().ok())
|
||||
.unwrap_or(DEFAULT_MAX_CONNECTIONS);
|
||||
|
||||
let pool = PgPoolOptions::new()
|
||||
let pool = match PgPoolOptions::new()
|
||||
.max_connections(max_connections)
|
||||
.connect(database_url)
|
||||
.await
|
||||
.context("failed to connect to database")?;
|
||||
{
|
||||
Ok(pool) => pool,
|
||||
Err(e) => {
|
||||
explain_auth_failure(&e);
|
||||
return Err(e).context("failed to connect to database");
|
||||
}
|
||||
};
|
||||
|
||||
sqlx::migrate!()
|
||||
.run(&pool)
|
||||
|
||||
@@ -172,9 +172,26 @@ impl CompressionWorker {
|
||||
Upload::set_derivatives_rev(&self.pool, upload_id, Self::DERIVATIVES_REV).await?;
|
||||
tracing::info!("preview + display generated for upload {upload_id}");
|
||||
} else if mime_type.starts_with("video/") {
|
||||
let thumb_rel = self.generate_video_thumbnail(upload_id, &original).await?;
|
||||
Upload::set_thumbnail_path(&self.pool, upload_id, &thumb_rel).await?;
|
||||
tracing::info!("thumbnail generated for upload {upload_id}");
|
||||
// A missing poster must NOT fail the upload. `set_thumbnail_path` is only reached when
|
||||
// a file really exists, so `thumbnail_path` stays NULL otherwise — which every consumer
|
||||
// already handles (FeedListCard, VirtualFeed, LightboxModal are all null-safe).
|
||||
//
|
||||
// The `?` here used to hide the defect; making the check strict without also making
|
||||
// this non-fatal would have been far worse than the bug. Every clip of a second or less
|
||||
// would fail compression, exhaust its retries and be soft-deleted — a cosmetic defect
|
||||
// turned into data loss, on exactly the mis-tap/Live-Photo clips guests produce most.
|
||||
match self.generate_video_thumbnail(upload_id, &original).await? {
|
||||
Some(thumb_rel) => {
|
||||
Upload::set_thumbnail_path(&self.pool, upload_id, &thumb_rel).await?;
|
||||
tracing::info!("thumbnail generated for upload {upload_id}");
|
||||
}
|
||||
None => {
|
||||
tracing::warn!(
|
||||
%upload_id,
|
||||
"no poster frame could be extracted; the video keeps its own tile"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Upload::set_compression_status(&self.pool, upload_id, "done").await?;
|
||||
@@ -325,55 +342,23 @@ impl CompressionWorker {
|
||||
}
|
||||
}
|
||||
|
||||
async fn generate_video_thumbnail(&self, upload_id: Uuid, original: &Path) -> Result<String> {
|
||||
/// Extract the feed poster for a video. `Ok(None)` when the clip yields no frame — see
|
||||
/// [`crate::services::video::extract_poster_frame`], which owns the seek order, the timeout and
|
||||
/// the artifact check that this function used to be missing.
|
||||
async fn generate_video_thumbnail(
|
||||
&self,
|
||||
upload_id: Uuid,
|
||||
original: &Path,
|
||||
) -> Result<Option<String>> {
|
||||
let thumbs_dir = self.media_path.join("thumbnails");
|
||||
tokio::fs::create_dir_all(&thumbs_dir).await?;
|
||||
|
||||
let thumb_filename = format!("{upload_id}.jpg");
|
||||
let thumb_path = thumbs_dir.join(&thumb_filename);
|
||||
|
||||
// Hard timeout — a malformed video can hang `ffmpeg` indefinitely. Without a
|
||||
// cap, the held compression-worker semaphore permit is never released and the
|
||||
// pool eventually deadlocks (no further uploads ever processed). 120s is well
|
||||
// above the time to extract one frame from any sane input.
|
||||
let mut child = tokio::process::Command::new("ffmpeg")
|
||||
.args([
|
||||
"-i",
|
||||
original.to_str().unwrap_or_default(),
|
||||
"-vframes",
|
||||
"1",
|
||||
"-ss",
|
||||
"00:00:01",
|
||||
"-vf",
|
||||
"scale=800:-1",
|
||||
"-y",
|
||||
thumb_path.to_str().unwrap_or_default(),
|
||||
])
|
||||
.stdout(std::process::Stdio::piped())
|
||||
.stderr(std::process::Stdio::piped())
|
||||
.kill_on_drop(true)
|
||||
.spawn()
|
||||
.context("failed to spawn ffmpeg")?;
|
||||
let produced =
|
||||
crate::services::video::extract_poster_frame(original, &thumb_path, 800).await?;
|
||||
|
||||
let status =
|
||||
match tokio::time::timeout(std::time::Duration::from_secs(120), child.wait()).await {
|
||||
Ok(res) => res.context("ffmpeg wait failed")?,
|
||||
Err(_) => {
|
||||
let _ = child.kill().await;
|
||||
anyhow::bail!("ffmpeg timeout after 120s");
|
||||
}
|
||||
};
|
||||
|
||||
if !status.success() {
|
||||
// Best-effort: drain stderr for the log.
|
||||
let mut stderr = Vec::new();
|
||||
if let Some(mut handle) = child.stderr.take() {
|
||||
use tokio::io::AsyncReadExt;
|
||||
let _ = handle.read_to_end(&mut stderr).await;
|
||||
}
|
||||
anyhow::bail!("ffmpeg failed: {}", String::from_utf8_lossy(&stderr));
|
||||
}
|
||||
|
||||
Ok(format!("thumbnails/{thumb_filename}"))
|
||||
Ok(produced.then(|| format!("thumbnails/{thumb_filename}")))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -785,38 +785,32 @@ async fn run_html_export_inner(
|
||||
let full_ext = ext_from_path(&row.original_path);
|
||||
let full = format!("{id_str}.{full_ext}");
|
||||
|
||||
// Video thumbnail via ffmpeg
|
||||
// Poster frame via the shared helper, which owns the seek order, the 120s timeout
|
||||
// (this call site had NONE — a hung ffmpeg would strand the export at `running`
|
||||
// forever) and the artifact check.
|
||||
let thumb_path = media_tmp.join(&thumb);
|
||||
let ffmpeg_result = tokio::process::Command::new("ffmpeg")
|
||||
.args([
|
||||
"-i",
|
||||
src.to_str().unwrap_or_default(),
|
||||
"-vframes",
|
||||
"1",
|
||||
"-ss",
|
||||
"00:00:01",
|
||||
"-vf",
|
||||
"scale=400:-1",
|
||||
"-y",
|
||||
thumb_path.to_str().unwrap_or_default(),
|
||||
])
|
||||
.output()
|
||||
.await;
|
||||
|
||||
match ffmpeg_result {
|
||||
Ok(output) if output.status.success() => {}
|
||||
_ => {
|
||||
tracing::warn!(
|
||||
"ffmpeg thumbnail failed for upload {}, skipping thumb",
|
||||
row.id
|
||||
);
|
||||
// Missing thumb entry — viewer handles missing thumbs gracefully.
|
||||
}
|
||||
let produced =
|
||||
match crate::services::video::extract_poster_frame(&src, &thumb_path, 400).await {
|
||||
Ok(produced) => produced,
|
||||
Err(e) => {
|
||||
tracing::warn!("poster extraction errored for upload {}: {e:#}", row.id);
|
||||
false
|
||||
}
|
||||
};
|
||||
if !produced {
|
||||
tracing::info!(
|
||||
upload_id = %row.id,
|
||||
"no poster frame for this video; exporting it without one"
|
||||
);
|
||||
}
|
||||
|
||||
// Stream the video full-res straight from the original at ZIP time — no
|
||||
// copy to temp (that used to transiently double disk usage per video).
|
||||
(thumb, full, MediaSource::Original(src.clone()))
|
||||
(
|
||||
produced.then(|| thumb.clone()),
|
||||
full,
|
||||
MediaSource::Original(src.clone()),
|
||||
)
|
||||
} else {
|
||||
let thumb = format!("{id_str}_thumb.jpg");
|
||||
let ext = ext_from_path(&row.original_path);
|
||||
@@ -842,9 +836,17 @@ async fn run_html_export_inner(
|
||||
})
|
||||
.await?;
|
||||
|
||||
if let Err(e) = thumb_result {
|
||||
tracing::warn!("thumbnail generation failed for upload {}: {e:#}", row.id);
|
||||
}
|
||||
// Same dangling-reference hazard as the video branch: a failure here left `thumb`
|
||||
// pointing at a file the ZIP writer would then skip, so `data.json` advertised an
|
||||
// entry the archive didn't contain. An undecodable image is rarer than a sub-second
|
||||
// clip, but the broken tile is identical.
|
||||
let thumb_ok = match thumb_result {
|
||||
Ok(()) => true,
|
||||
Err(e) => {
|
||||
tracing::warn!("thumbnail generation failed for upload {}: {e:#}", row.id);
|
||||
false
|
||||
}
|
||||
};
|
||||
|
||||
// Full variant: compress to temp if >5MB, otherwise stream the original
|
||||
// as-is (no temp copy). `src_meta` was stat'd once at the top of the loop.
|
||||
@@ -882,15 +884,16 @@ async fn run_html_export_inner(
|
||||
MediaSource::Original(src.clone())
|
||||
};
|
||||
|
||||
(thumb, full, full_source)
|
||||
(thumb_ok.then_some(thumb), full, full_source)
|
||||
};
|
||||
|
||||
// Register this post's two media entries. Thumbnails always come from temp
|
||||
// (they're freshly generated); the full variant's source was decided above.
|
||||
media_manifest.push((
|
||||
thumb_name.clone(),
|
||||
MediaSource::Temp(media_tmp.join(&thumb_name)),
|
||||
));
|
||||
// Register this post's media entries. The thumbnail is registered ONLY when one was
|
||||
// actually produced: pushing a manifest entry for a file that doesn't exist made the ZIP
|
||||
// writer skip it silently while `data.json` still advertised it — the viewer then drew a
|
||||
// broken image tile for an entry the archive never contained.
|
||||
if let Some(name) = &thumb_name {
|
||||
media_manifest.push((name.clone(), MediaSource::Temp(media_tmp.join(name))));
|
||||
}
|
||||
media_manifest.push((full_name.clone(), full_source));
|
||||
|
||||
// Build comments for this upload
|
||||
@@ -925,7 +928,15 @@ async fn run_html_export_inner(
|
||||
} else {
|
||||
"image".to_string()
|
||||
},
|
||||
thumb: format!("media/{thumb_name}"),
|
||||
// Empty when there is no poster. The viewer already guards on this
|
||||
// (`{#if post.media.thumb}` → a video tile with a play glyph, or the placeholder
|
||||
// icon for an image), so telling it the truth is the entire fix — no schema
|
||||
// change, no viewer rebuild. What was broken was the backend always claiming a
|
||||
// thumbnail existed.
|
||||
thumb: thumb_name
|
||||
.as_ref()
|
||||
.map(|n| format!("media/{n}"))
|
||||
.unwrap_or_default(),
|
||||
full: format!("media/{full_name}"),
|
||||
},
|
||||
});
|
||||
|
||||
@@ -6,3 +6,4 @@ pub mod imaging;
|
||||
pub mod maintenance;
|
||||
pub mod rate_limiter;
|
||||
pub mod sse_tickets;
|
||||
pub mod video;
|
||||
|
||||
140
backend/src/services/video.rs
Normal file
140
backend/src/services/video.rs
Normal file
@@ -0,0 +1,140 @@
|
||||
//! Poster-frame extraction, shared by the compression worker and the HTML export.
|
||||
//!
|
||||
//! Both used to spawn `ffmpeg` themselves with the same broken invocation:
|
||||
//!
|
||||
//! ```text
|
||||
//! ffmpeg -i <src> -vframes 1 -ss 00:00:01 -vf scale=… -y <out>
|
||||
//! ```
|
||||
//!
|
||||
//! `-ss` AFTER `-i` is an output-side seek. Against a clip of a second or less ffmpeg exits **0 and
|
||||
//! writes nothing** — and both call sites gated on the exit status, so neither noticed. The worker
|
||||
//! then wrote `thumbnail_path` for a file that was never created (404 in the live feed) and the
|
||||
//! export listed the entry in `data.json` while the ZIP writer skipped it (a broken image tile in
|
||||
//! the keepsake). Every server-side signal stayed green. Phones produce such clips constantly:
|
||||
//! mis-taps, Live Photos, boomerangs.
|
||||
//!
|
||||
//! This module exists for the same reason `imaging.rs` does — that one was created when compression
|
||||
//! and export duplicated decode logic, and it paid off immediately when the `max_alloc` fix landed
|
||||
//! in both workers at once. Same duplication, same fix.
|
||||
|
||||
use std::path::Path;
|
||||
use std::time::Duration;
|
||||
|
||||
use anyhow::{Context, Result};
|
||||
|
||||
/// A malformed video can hang `ffmpeg` indefinitely. In the compression worker that never releases
|
||||
/// the semaphore permit and the pool eventually deadlocks; in the export worker it strands the job
|
||||
/// at `running` so the keepsake never completes. `export.rs` had NO timeout at all before this
|
||||
/// module — sharing the spawn fixes that too.
|
||||
const FFMPEG_TIMEOUT: Duration = Duration::from_secs(120);
|
||||
|
||||
/// Seek positions to try, in order.
|
||||
///
|
||||
/// One second first: the opening frame of a real video is often black, a fade-in, or motion-blurred
|
||||
/// as the camera settles, so it makes a poor poster. Zero second as the fallback, which is what
|
||||
/// makes short clips work — and it is genuinely required, not defensive. Moving `-ss` before `-i`
|
||||
/// (an input-side seek) is necessary but NOT sufficient: seeking to 1 s in a 1.000 s clip is still
|
||||
/// past the last frame, and ffmpeg still exits 0 having written nothing. Verified against the real
|
||||
/// production image.
|
||||
const SEEK_POSITIONS: &[&str] = &["00:00:01", "0"];
|
||||
|
||||
/// Extract one poster frame from `src` into `dest`, scaled to `width` px wide.
|
||||
///
|
||||
/// `Ok(false)` means the video yielded no frame — a normal outcome for a very short or unusual
|
||||
/// clip, NOT an error. Callers must degrade (no poster) rather than fail the upload: treating this
|
||||
/// as an error would soft-delete every sub-second video, turning a cosmetic defect into data loss.
|
||||
///
|
||||
/// `Err` is reserved for something genuinely wrong — a hang we had to kill, or a failure to spawn.
|
||||
pub async fn extract_poster_frame(src: &Path, dest: &Path, width: u32) -> Result<bool> {
|
||||
for seek in SEEK_POSITIONS {
|
||||
// A stale file from a previous attempt would be indistinguishable from a fresh success.
|
||||
let _ = tokio::fs::remove_file(dest).await;
|
||||
|
||||
run_ffmpeg(src, dest, width, seek).await?;
|
||||
|
||||
// THE CHECK BOTH CALL SITES WERE MISSING: ask the filesystem, not the exit status.
|
||||
// Non-empty, because a zero-byte file is not a poster either.
|
||||
if tokio::fs::metadata(dest)
|
||||
.await
|
||||
.map(|m| m.is_file() && m.len() > 0)
|
||||
.unwrap_or(false)
|
||||
{
|
||||
return Ok(true);
|
||||
}
|
||||
}
|
||||
|
||||
// Leave nothing behind for a caller to mistake for a result.
|
||||
let _ = tokio::fs::remove_file(dest).await;
|
||||
Ok(false)
|
||||
}
|
||||
|
||||
/// Run one ffmpeg attempt. A non-zero exit is NOT an error here — the artifact check above is the
|
||||
/// authority, and a corrupt input that fails at 1 s may still yield a frame at 0.
|
||||
async fn run_ffmpeg(src: &Path, dest: &Path, width: u32, seek: &str) -> Result<()> {
|
||||
let mut child = tokio::process::Command::new("ffmpeg")
|
||||
.args([
|
||||
// BEFORE -i: an input-side seek. See SEEK_POSITIONS.
|
||||
"-ss",
|
||||
seek,
|
||||
"-i",
|
||||
src.to_str().unwrap_or_default(),
|
||||
"-vframes",
|
||||
"1",
|
||||
"-vf",
|
||||
&format!("scale={width}:-1"),
|
||||
"-y",
|
||||
dest.to_str().unwrap_or_default(),
|
||||
])
|
||||
.stdout(std::process::Stdio::piped())
|
||||
.stderr(std::process::Stdio::piped())
|
||||
.kill_on_drop(true)
|
||||
.spawn()
|
||||
.context("failed to spawn ffmpeg")?;
|
||||
|
||||
match tokio::time::timeout(FFMPEG_TIMEOUT, child.wait()).await {
|
||||
Ok(res) => {
|
||||
res.context("ffmpeg wait failed")?;
|
||||
}
|
||||
Err(_) => {
|
||||
let _ = child.kill().await;
|
||||
anyhow::bail!("ffmpeg timed out after {}s", FFMPEG_TIMEOUT.as_secs());
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
/// The order is the whole fix. `-ss` must precede `-i`, and 0 must be tried after 1 s.
|
||||
#[test]
|
||||
fn the_fallback_seek_exists_and_comes_last() {
|
||||
assert_eq!(
|
||||
SEEK_POSITIONS,
|
||||
&["00:00:01", "0"],
|
||||
"1s first for a better poster, 0 as the fallback that makes short clips work"
|
||||
);
|
||||
}
|
||||
|
||||
/// A missing input yields no frame rather than an error: the caller must degrade to "no
|
||||
/// poster", never fail the upload. `Err` is reserved for a hang or a spawn failure.
|
||||
#[tokio::test]
|
||||
async fn a_missing_source_yields_no_frame_rather_than_an_error() {
|
||||
let dir = std::env::temp_dir().join(format!("es-video-{}", std::process::id()));
|
||||
std::fs::create_dir_all(&dir).unwrap();
|
||||
let dest = dir.join("out.jpg");
|
||||
|
||||
let got = extract_poster_frame(Path::new("/nonexistent/clip.mp4"), &dest, 400).await;
|
||||
|
||||
match got {
|
||||
Ok(false) => {}
|
||||
other => panic!("expected Ok(false) for a missing input, got {other:?}"),
|
||||
}
|
||||
assert!(
|
||||
!dest.exists(),
|
||||
"a failed extraction must leave nothing a caller could mistake for a poster"
|
||||
);
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
}
|
||||
BIN
e2e/fixtures/media/huge-99mp.jpg
Normal file
BIN
e2e/fixtures/media/huge-99mp.jpg
Normal file
Binary file not shown.
|
After Width: | Height: | Size: 568 KiB |
BIN
e2e/fixtures/media/portrait-exif6.jpg
Normal file
BIN
e2e/fixtures/media/portrait-exif6.jpg
Normal file
Binary file not shown.
|
After Width: | Height: | Size: 807 B |
BIN
e2e/fixtures/media/sample-5s.mp4
Normal file
BIN
e2e/fixtures/media/sample-5s.mp4
Normal file
Binary file not shown.
BIN
e2e/fixtures/media/sample.jpg
Normal file
BIN
e2e/fixtures/media/sample.jpg
Normal file
Binary file not shown.
|
After Width: | Height: | Size: 27 KiB |
BIN
e2e/fixtures/media/sample.mp4
Normal file
BIN
e2e/fixtures/media/sample.mp4
Normal file
Binary file not shown.
BIN
e2e/fixtures/media/sample2.jpg
Normal file
BIN
e2e/fixtures/media/sample2.jpg
Normal file
Binary file not shown.
|
After Width: | Height: | Size: 35 KiB |
@@ -69,6 +69,17 @@ test.describe('Video — the lightbox plays it', () => {
|
||||
// The poster SHOULD still be the thumbnail — that's what it's for.
|
||||
await expect(video).toHaveAttribute('poster', `/api/v1/upload/${id}/thumbnail`);
|
||||
|
||||
// …and it must actually RESOLVE. Asserting only the attribute is what let a phantom thumbnail
|
||||
// survive nine rounds of green: `thumbnail_path` was written for a file ffmpeg never created,
|
||||
// so this URL 404'd for every clip of a second or less while the attribute looked perfect.
|
||||
// One extra fetch is the whole difference.
|
||||
const poster = await fetch(`${BASE}/api/v1/upload/${id}/thumbnail`, {
|
||||
headers: { Authorization: `Bearer ${g.jwt}` },
|
||||
});
|
||||
expect(poster.status, 'the poster URL must serve real bytes, not just exist').toBe(200);
|
||||
expect(poster.headers.get('content-type')).toContain('image/');
|
||||
expect((await poster.arrayBuffer()).byteLength).toBeGreaterThan(0);
|
||||
|
||||
// And the browser must accept the bytes as media. preload="none" means nothing is
|
||||
// fetched until we ask, so drive a load explicitly and wait for metadata.
|
||||
const readyState = await video.evaluate(async (el: HTMLVideoElement) => {
|
||||
|
||||
84
e2e/specs/03-feed/video-poster.spec.ts
Normal file
84
e2e/specs/03-feed/video-poster.spec.ts
Normal file
@@ -0,0 +1,84 @@
|
||||
/**
|
||||
* Regression guard — a short video gets a real poster frame, and the keepsake never shows a broken
|
||||
* tile.
|
||||
*
|
||||
* Both the compression worker and the HTML export ran the same invocation:
|
||||
*
|
||||
* ffmpeg -i <src> -vframes 1 -ss 00:00:01 -vf scale=… -y <out>
|
||||
*
|
||||
* `-ss` AFTER `-i` is an output-side seek. Against a clip of a second or less ffmpeg exits **0 and
|
||||
* writes nothing**, and both call sites gated on the exit status. So:
|
||||
*
|
||||
* - the worker wrote `thumbnail_path` and logged "thumbnail generated" for a file that was never
|
||||
* created → `GET /upload/{id}/thumbnail` 404s in the live feed;
|
||||
* - the export listed `media/…_thumb.jpg` in `data.json` while the ZIP writer skipped the
|
||||
* unopenable file → the keepsake rendered a broken image tile.
|
||||
*
|
||||
* Any clip at or under a second, which phones produce constantly: mis-taps, Live Photos, boomerangs.
|
||||
* Every server-side signal stayed green throughout.
|
||||
*
|
||||
* Two fixtures on purpose, because they take different paths through the fix:
|
||||
* - `sample.mp4` is exactly 1.000 s. An input-side seek to 1 s is STILL past its last frame, so it
|
||||
* is the 0 s fallback that saves it. Moving `-ss` before `-i` alone does not fix this file.
|
||||
* - `sample-5s.mp4` is 5 s and succeeds on the first seek — the normal path, which no test covered
|
||||
* at all before, because the suite only ever had the boundary fixture.
|
||||
*/
|
||||
import { test, expect } from '../../fixtures/test';
|
||||
import { uploadRaw } from '../../helpers/upload-client';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { join } from 'node:path';
|
||||
import { BASE } from '../../helpers/env';
|
||||
|
||||
const CLIPS = [
|
||||
{ file: 'sample.mp4', label: '1.000s — needs the 0s fallback' },
|
||||
{ file: 'sample-5s.mp4', label: '5s — succeeds on the first seek' },
|
||||
];
|
||||
|
||||
async function uploadClip(jwt: string, file: string): Promise<string> {
|
||||
const bytes = readFileSync(join(process.cwd(), 'fixtures', 'media', file));
|
||||
const res = await uploadRaw(jwt, bytes, { filename: file, contentType: 'video/mp4' });
|
||||
expect(res.status, `uploading ${file}`).toBe(201);
|
||||
return ((await res.json()) as { id: string }).id;
|
||||
}
|
||||
|
||||
test.describe('Video — the poster frame is real', () => {
|
||||
for (const { file, label } of CLIPS) {
|
||||
test(`${file} (${label}) gets a fetchable poster`, async ({ guest, db }) => {
|
||||
test.setTimeout(60_000);
|
||||
const g = await guest(`Poster${file.replace(/\W/g, '')}`);
|
||||
const id = await uploadClip(g.jwt, file);
|
||||
await expect.poll(() => db.compressionStatus(id), { timeout: 45_000 }).toBe('done');
|
||||
|
||||
// The DB must not claim a thumbnail that isn't there — that claim IS the defect.
|
||||
const res = await fetch(`${BASE}/api/v1/upload/${id}/thumbnail`, {
|
||||
headers: { Authorization: `Bearer ${g.jwt}` },
|
||||
});
|
||||
expect(res.status, `${file}: the poster must exist, not just be recorded`).toBe(200);
|
||||
expect(res.headers.get('content-type')).toContain('image/');
|
||||
expect((await res.arrayBuffer()).byteLength).toBeGreaterThan(0);
|
||||
});
|
||||
}
|
||||
|
||||
test('a video upload still succeeds even if no poster can be extracted', async ({
|
||||
guest,
|
||||
db,
|
||||
}) => {
|
||||
// The mirror that keeps the fix honest. Tightening the check to "the file must exist" without
|
||||
// also making a missing poster non-fatal would have been far worse than the bug: the worker's
|
||||
// call used `?`, so every sub-second clip would fail compression, exhaust its retries and be
|
||||
// soft-deleted. A cosmetic defect turned into data loss.
|
||||
//
|
||||
// `compression_status = 'done'` with the upload still present is exactly that guarantee.
|
||||
const g = await guest('PosterSurvivor');
|
||||
const id = await uploadClip(g.jwt, 'sample.mp4');
|
||||
await expect.poll(() => db.compressionStatus(id), { timeout: 45_000 }).toBe('done');
|
||||
expect(await db.countUploadsForUser(g.userId)).toBe(1);
|
||||
|
||||
// And the video itself is playable regardless of the poster.
|
||||
const orig = await fetch(`${BASE}/api/v1/upload/${id}/original`, {
|
||||
headers: { Authorization: `Bearer ${g.jwt}` },
|
||||
});
|
||||
expect(orig.status).toBe(200);
|
||||
expect(orig.headers.get('content-type')).toContain('video/');
|
||||
});
|
||||
});
|
||||
@@ -6,9 +6,16 @@
|
||||
* this drives a real video upload → export → and proves the video entry lands in
|
||||
* Memories.zip (i.e. the streamed-from-original path works and the video isn't dropped).
|
||||
*
|
||||
* The fixture clip is <1s, so ffmpeg extracts no thumbnail frame — but exits 0, so the
|
||||
* compression worker keeps the upload (it isn't auto-cleaned). That's the intended
|
||||
* shape here: the full video is exported even when its thumbnail is absent.
|
||||
* NOTE ON A PREVIOUS VERSION OF THIS COMMENT. It used to read: "The fixture clip is <1s, so ffmpeg
|
||||
* extracts no thumbnail frame — but exits 0, so the compression worker keeps the upload. That's the
|
||||
* intended shape here." None of that was intended. `-ss` sat AFTER `-i` (an output-side seek), so
|
||||
* against `sample.mp4` — which is exactly 1.000 s — ffmpeg exited 0 having written nothing, and
|
||||
* both the worker and the export gated on the exit status. The missing thumbnail was observed here
|
||||
* and written down as expected behaviour instead of investigated; every video test in the suite ran
|
||||
* against that one boundary fixture, and none of them ever fetched the poster.
|
||||
*
|
||||
* The seek is now input-side with a 0 s fallback and the artifact is verified rather than the exit
|
||||
* code, so this clip DOES get a thumbnail. The assertion at the bottom pins that.
|
||||
*/
|
||||
import { test, expect } from '../../fixtures/test';
|
||||
import { uploadRaw } from '../../helpers/upload-client';
|
||||
@@ -65,5 +72,13 @@ test.describe('Export — video streaming (P4)', () => {
|
||||
const needle = `media/${id}.mp4`;
|
||||
const haystack = new TextDecoder('latin1').decode(bytes);
|
||||
expect(haystack.includes(needle), `Memories.zip must contain ${needle}`).toBe(true);
|
||||
|
||||
// And its poster is really in the archive. This clip is 1.000 s — the exact case the old
|
||||
// output-side seek produced nothing for, silently, while `data.json` still advertised the
|
||||
// entry. See the note at the top of this file.
|
||||
expect(
|
||||
haystack.includes(`media/${id}_thumb.jpg`),
|
||||
`Memories.zip must contain the poster for ${id}, not just reference it`
|
||||
).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
113
e2e/specs/06-export/viewer-no-broken-tiles.spec.ts
Normal file
113
e2e/specs/06-export/viewer-no-broken-tiles.spec.ts
Normal file
@@ -0,0 +1,113 @@
|
||||
/**
|
||||
* Regression guard — the keepsake never renders a broken image tile.
|
||||
*
|
||||
* The HTML export wrote `thumb: "media/<id>_thumb.jpg"` into `data.json` unconditionally. When
|
||||
* ffmpeg produced no poster frame — which it did, silently and with exit 0, for any clip of a
|
||||
* second or less — the ZIP writer skipped the unopenable file but `data.json` still advertised it.
|
||||
* The viewer then requested an entry the archive did not contain and drew a broken `<img>`.
|
||||
*
|
||||
* The viewer was never the problem: `+page.svelte` already guards `{#if post.media.thumb}` and
|
||||
* falls back to a proper dark video tile with a play glyph. The guard simply never fired, because
|
||||
* the backend always handed it a non-empty string. The fix is the backend telling the truth —
|
||||
* `thumb: ""` when there is no poster — so no viewer change was needed.
|
||||
*
|
||||
* This asserts the property that actually matters to a guest and that no server-side signal can
|
||||
* report: **every image in the opened keepsake resolves**. `naturalWidth > 0` is false for exactly
|
||||
* the broken-tile case, whatever produced it — a missing video poster, a failed image thumbnail, or
|
||||
* some future path nobody has thought of yet. It is deliberately not an assertion about ffmpeg.
|
||||
*
|
||||
* Runs over `file://`, the way a guest opens it.
|
||||
*/
|
||||
import { test, expect } from '../../fixtures/test';
|
||||
import { execFileSync } from 'node:child_process';
|
||||
import { mkdtempSync, writeFileSync, rmSync, readFileSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { join } from 'node:path';
|
||||
import { uploadRaw } from '../../helpers/upload-client';
|
||||
import { seedUpload } from '../../helpers/seed';
|
||||
import { BASE } from '../../helpers/env';
|
||||
|
||||
test.describe('Export — the keepsake has no broken tiles', () => {
|
||||
test('every image in the opened viewer resolves', async ({ page, host, guest, db }) => {
|
||||
test.setTimeout(150_000);
|
||||
const bearer = { Authorization: `Bearer ${host.jwt}` };
|
||||
|
||||
const g = await guest('TileChecker');
|
||||
const ids: string[] = [await seedUpload(g.jwt, { caption: 'ein Foto' })];
|
||||
|
||||
// Both clips: the 1.000 s one is the case that produced the broken tile, the 5 s one is the
|
||||
// ordinary path that had no coverage at all.
|
||||
for (const file of ['sample.mp4', 'sample-5s.mp4']) {
|
||||
const bytes = readFileSync(join(process.cwd(), 'fixtures', 'media', file));
|
||||
const res = await uploadRaw(g.jwt, bytes, { filename: file, contentType: 'video/mp4' });
|
||||
expect(res.status).toBe(201);
|
||||
ids.push(((await res.json()) as { id: string }).id);
|
||||
}
|
||||
for (const id of ids) {
|
||||
await expect.poll(() => db.compressionStatus(id), { timeout: 45_000 }).toBe('done');
|
||||
}
|
||||
|
||||
expect(
|
||||
(await fetch(`${BASE}/api/v1/host/gallery/release`, { method: 'POST', headers: bearer }))
|
||||
.status
|
||||
).toBe(204);
|
||||
await expect
|
||||
.poll(
|
||||
async () => {
|
||||
const res = await fetch(`${BASE}/api/v1/export/status`, { headers: bearer });
|
||||
return (await res.json()).html?.status;
|
||||
},
|
||||
{ timeout: 120_000, intervals: [500] }
|
||||
)
|
||||
.toBe('done');
|
||||
|
||||
const ticketRes = await fetch(`${BASE}/api/v1/export/ticket`, {
|
||||
method: 'POST',
|
||||
headers: bearer,
|
||||
});
|
||||
const { ticket } = (await ticketRes.json()) as { ticket: string };
|
||||
const dl = await fetch(`${BASE}/api/v1/export/html?ticket=${encodeURIComponent(ticket)}`);
|
||||
expect(dl.status).toBe(200);
|
||||
|
||||
const dir = mkdtempSync(join(tmpdir(), 'eventsnap-tiles-'));
|
||||
try {
|
||||
const zipPath = join(dir, 'Memories.zip');
|
||||
writeFileSync(zipPath, Buffer.from(await dl.arrayBuffer()));
|
||||
execFileSync('unzip', ['-qo', zipPath, '-d', dir]);
|
||||
|
||||
await page.goto('file://' + join(dir, 'index.html'));
|
||||
|
||||
// Every post is present, whether or not it has a poster.
|
||||
const posts = await page.evaluate(() => {
|
||||
const d = (
|
||||
window as unknown as {
|
||||
__EXPORT_DATA__?: { posts?: { media?: { thumb?: string; type?: string } }[] };
|
||||
}
|
||||
).__EXPORT_DATA__;
|
||||
return d?.posts?.map((p) => ({ thumb: p.media?.thumb ?? '', type: p.media?.type })) ?? null;
|
||||
});
|
||||
expect(posts, '__EXPORT_DATA__ was never assigned').not.toBeNull();
|
||||
expect(posts!.length).toBe(3);
|
||||
|
||||
// Any thumb data.json DOES advertise must be a file the archive actually contains.
|
||||
const entries = execFileSync('unzip', ['-Z1', zipPath], { encoding: 'utf8' }).split('\n');
|
||||
for (const p of posts!.filter((p) => p.thumb)) {
|
||||
expect(
|
||||
entries.includes(p.thumb),
|
||||
`data.json advertises ${p.thumb} but the archive does not contain it`
|
||||
).toBe(true);
|
||||
}
|
||||
|
||||
// THE assertion: nothing rendered broken. Give the images a moment to settle first.
|
||||
await page.waitForLoadState('networkidle');
|
||||
const broken = await page.evaluate(() =>
|
||||
Array.from(document.querySelectorAll('img'))
|
||||
.filter((i) => i.complete && i.naturalWidth === 0)
|
||||
.map((i) => i.getAttribute('src') ?? '(no src)')
|
||||
);
|
||||
expect(broken, `broken image tiles in the keepsake: ${broken.join(', ')}`).toEqual([]);
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user