From 12694e43c25f11f4081f1dcb9a2cdf1e85dee267 Mon Sep 17 00:00:00 2001 From: csd113 Date: Wed, 30 Sep 2026 19:36:26 -0700 Subject: [PATCH 1/5] Polish public posting recovery and reading interactions --- CHANGELOG.md | 4 + docs/public-interface-polish-audit.md | 102 ++++ src/db/threads.rs | 76 ++- src/handlers/board/create_thread.rs | 48 +- src/handlers/mod.rs | 593 +++++++++++++++++++++++- src/handlers/render.rs | 1 + src/handlers/thread.rs | 48 +- src/models.rs | 2 + src/templates/board.rs | 7 +- src/templates/forms.rs | 270 +++++++++-- src/templates/thread.rs | 131 +++++- src/utils/sanitize.rs | 54 ++- static/main.js | 218 +++++++-- static/style.css | 5 + tests/e2e/public-catalog-polish.spec.ts | 80 ++++ tests/e2e/public-polish-native.spec.ts | 244 ++++++++++ tests/e2e/public-polish-posting.spec.ts | 255 ++++++++++ tests/e2e/public-polish-visual.spec.ts | 166 +++++++ tests/e2e/public-reading-polish.spec.ts | 400 ++++++++++++++++ 19 files changed, 2555 insertions(+), 149 deletions(-) create mode 100644 docs/public-interface-polish-audit.md create mode 100644 tests/e2e/public-catalog-polish.spec.ts create mode 100644 tests/e2e/public-polish-native.spec.ts create mode 100644 tests/e2e/public-polish-posting.spec.ts create mode 100644 tests/e2e/public-polish-visual.spec.ts create mode 100644 tests/e2e/public-reading-polish.spec.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index b706bd4e..e6f152de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ All notable changes to RustChan will be documented in this file. ### Improved +- Improved public posting recovery, quote and search navigation, long-filename presentation, and mobile keyboard focus while preserving the compact imageboard layout and existing themes. - Expanded the terminal administration console with task-oriented browsing, contextual forms and dialogs, logs, history, telemetry, and responsive operator views. - Reorganized the web admin panel into 21 task-focused sections with searchable configuration controls, account management, and separate active, saved, overridden, and next-restart settings states. - Standardized admin spacing, typography, controls, tables, status treatments, and expandable details across all nine existing themes, including loading, error, empty, and disabled states. @@ -14,6 +15,9 @@ All notable changes to RustChan will be documented in this file. ### Fixed +- Rejected public posts retain poll settings and submitted drafts, explain attachment reselection, and keep enhanced CAPTCHA refresh in place. Upload interruptions and unconfirmed responses retain input with check-before-retry guidance; pending uploads cannot fall through to duplicate native submissions. +- Local quotes and backlinks preserve browser history, dense previews keep their triggers reachable, search links open complete thread context, poll result anchors work, and bounded filename sanitization retains extensions. Mobile preferences use dialog focus containment and restoration, and explicit scrolling honors reduced motion. +- Catalog “last reply” sorting includes sage replies independently of bump order. Failed audio/video playback offers readable recovery advice and an original-file download action. - Rejected network and runtime configuration forms now retain the administrator's selected theme and submitted values. - Admin health polling no longer starts requests from an unloading page: polling schedules after completion, cancels and aborts on navigation, ignores late updates, and resumes after back/forward-cache restoration. This fixes intermittent WebKit access-control diagnostics without increasing timeouts or enabling retries. - Firefox launches automatically through the normal browser harness on macOS 27 despite Playwright's upstream shared-application-identity defect ([Playwright #42768](https://github.com/microsoft/playwright/issues/42768)). The isolated identity uses the installed browser binary, requires no manual shell setup, and is protected by launch regressions and a version guard. diff --git a/docs/public-interface-polish-audit.md b/docs/public-interface-polish-audit.md new file mode 100644 index 00000000..8e47116b --- /dev/null +++ b/docs/public-interface-polish-audit.md @@ -0,0 +1,102 @@ +# Public interface polish audit — 2026-09-30 + +Scope: home, board indexes, catalog, threads, search, archives, preferences, media, and polls. Admin changes were excluded; shared behavior was checked with the maintained regression harness. ChanNet is already removed from this branch; no obsolete infrastructure was restored. + +The initial `interface-audit` working tree was clean. At the audit handoff, all changes were local and uncommitted. No dependencies, framework, route renames, deployment, or visual redesign were introduced. + +## Baseline and findings + +The documented build and maintained `tests/e2e` workflow launched the actual RustChan binary in unique loopback runtimes, with synthetic SQLite data, uploads, credentials, settings, and disabled Tor/background maintenance. Existing protections were examined first: server-side CSRF, one-use CAPTCHA, upload limits, submission-token atomicity, input recovery, native forms, quote highlighting, thumbnail fallbacks, dialog containment, and cookie-backed preferences were retained. + +The initial Chromium posting/upload/idempotency baseline passed all nine selected tests. Findings below are functional defects; no subjective visual changes were necessary. + +| Reproduction | Impact | Contained fix and regression | +| --- | --- | --- | +| Create a native thread with three poll options and a nondefault duration, then submit a wrong CAPTCHA. | Rejection retained body/name/subject but reset the poll. | Retain escaped question/options and original duration controls; reopen the populated poll. `public-polish-native.spec.ts`, forms unit tests. | +| Reject a native post with attachments. | Browser clears file controls after navigation without explaining reselection. | Show attachment reselection advice only when files were submitted; preserve text/sage. Native posting regressions. | +| Submit a parsed reply after its thread is locked/deleted, or with invalid CSRF. | Standalone failure lacked a copyable submitted draft. | Preserve status/security checks and display an escaped readonly draft plus recovery advice; dedicated ban/appeal pages remain intact. Native posting regressions. | +| Reject a native oversized multipart request before parsing finishes. | Firefox could reset the connection; after receiving 413, Back could restore an empty form. | Retain received text and read trailing bounded poll controls for an escaped copyable draft. One shared 8 MiB/1 s discard allowance includes excess upload and remaining controls; no excess media is staged, original rejection and aggregate/field/concurrency limits remain enforced. Native 413/copy/fresh-form/smaller-file retry regression plus cap/timeout tests. | +| Select a file and fill text/poll fields, then choose a new CAPTCHA. | Full-page refresh silently discards unsent choices and attachments. | Refresh only CAPTCHA fields in place with JavaScript, retain the submission token/files/other controls, explain refresh failure; native refresh warns about copying before navigation. Posting regressions. | +| Upload receives an unconfirmed HTTP 200, aborts, times out, or fails synchronously. | Unconfirmed response could reload away input; abort was silent; a synchronous send exception could leave controls busy. | Retain input, restore controls on failure, give an uncertainty/check-before-retry action, and keep success navigation tied to the server redirect. Actual SQLite-delayed request and token-reusing retry regression. | +| Trigger a second submit while an upload is pending. | Busy handler could fall through into native submission. | Prevent pending submissions; keep controls busy through confirmed navigation. Posting regression. | +| Upload progress event has no computable total. | Full progress bar could imply measured completion. | Show unmeasured upload state; announce upload/server-wait status using actual upload completion, without invented server percentages. | +| A validation response returns submitted reply text while another tab has written stale autosave. | Client restore could overwrite the newer server-rendered text. | Prefer nonempty server-rendered text over shared autosave. Posting regression. | +| Activate a local quote/backlink and use browser Back, or hover a quote into a dense post. | Enhanced clicks bypassed native history; an oversized preview could cover its trigger. | Retain native fragment/history navigation and constrain previews to available viewport space. Reading regression. | +| Activate a search result post number or a quote whose target is outside the results. | Links had no complete-thread context and could appear deleted. | Context-aware search links resolve through existing thread/post routes; preserve modified-click navigation. Reading and renderer regressions. | +| Vote in a poll. | Existing `#poll` redirect had no target element. | Add the target and header scroll margin. Reading regression. | +| Upload a filename longer than the existing 100-character bound. | Truncation could remove the extension. | Preserve a bounded extension and Unicode stem with unchanged forbidden-character replacements and length limit. Reading and sanitization regressions. | +| Load a stored audio/video file the browser cannot play, including before the application script finishes loading. | Native controls could remain unusable without an application explanation. | Add a compact announced failure message and original-file download action; recognize existing failure state when wiring and clear it after successful reload. Real WAV/H.264 playback, corruption, download, and recovery regressions; use primary theme text for readable captions. | +| Add a sage reply to an older thread and select catalog “last reply”. | Sorting reused the bump timestamp and ignored the newest reply. | Compute the latest post time in the existing grouped thread query and use it for this catalog choice; preserve bump ordering, image counts, and saved preferences. Catalog and database regressions. | +| Open preferences by keyboard at mobile width, dismiss it, or resize an open panel. | Focus remained outside the overlay; mobile panel lacked dialog semantics/containment. | Use the existing dialog trap with mobile-only metadata, initial Close focus, and restoration after native details has rendered closed. Visual regression. | +| Open a mobile composer with reduced motion enabled. | Explicit smooth scroll ignored the preference. | Shared scroll behavior also covers errors and new-reply navigation. Visual regression. | + +## Evidence and verification + +Generated reports, diagnostics, traces, videos, runtime logs, and before/after screenshots stay ignored under `output/playwright/public-polish/`. Representative screenshots were visually inspected, including native poll recovery, uncertain upload response, dense Unicode threads, search context, portrait plus audio attachments, report dialogs, preferences, and native unavailable/oversized states. + +Built-in themes: Forest, Blue Sky, Deep Orbit, Terminal, DORFic, ChanClassic, Frutiger Aero, NeonCubicle, and FluoroGrid. Each was exercised at 1280×800, 320×720, and 667×320 in the configured browser projects. The theme audit checks document overflow, post/metadata readability, focus, and dialog controls while retaining each theme's appearance. + +Final Rust/source validation: + +| Command | Result | +| --- | --- | +| `cargo fmt --all --check` | Passed. | +| `cargo check --workspace --all-targets --all-features` | Passed. | +| `cargo clippy --workspace --all-targets --all-features -- -D warnings -D clippy::all -D clippy::pedantic -D clippy::nursery -D clippy::cargo` | Passed without new lint suppressions. | +| `cargo test --workspace --all-features` | Passed: 1,227 library + 1,227 binary + 5 runtime integration + 1 documentation test = 2,460 executions; none failed or ignored. | +| `cargo build --locked --bin rustchan-cli` | Passed; refreshed embedded CSS/JavaScript for the browser runs. | +| `cargo test --lib --all-features rejected_upload` | Passed all 7 size/field/aggregate/deadline/draft recovery regressions. | +| `cargo test --lib --all-features listing_last_post_includes_non_bumping_text_and_audio_without_counting_them_as_images` | Passed; active/archive aggregates and image counts retained. | +| `node --check static/main.js` and `git diff --check` | Passed. | +| `node tests/e2e/media-toolchain-check.mjs --self-test` | Passed. | +| `node tests/e2e/media-toolchain-check.mjs` | Failed host preflight: FFmpeg lacks `libwebp`; full optional media-toolchain suite not launched or counted as passed. | + +Final browser runs used `RUSTCHAN_E2E_SKIP_BUILD=1`, unique `RUSTCHAN_AUDIT_OUTPUT` directories, the freshly rebuilt binary, and the existing Playwright configuration with zero automatic retries: + +| Maintained-harness selection | Passed | Explicit skips | Failed | Evidence directory | +| --- | ---: | ---: | ---: | --- | +| `npx playwright test tests/e2e/public-polish-native.spec.ts --workers=1` | 56 | 0 | 0 | `native-final-latest/` | +| `npx playwright test tests/e2e/public-polish-posting.spec.ts tests/e2e/public-polish-visual.spec.ts --workers=2` | 67 | 24 | 0 | `posting-visual-final/` | +| `npx playwright test tests/e2e/public-reading-polish.spec.ts tests/e2e/public-catalog-polish.spec.ts --workers=1` | 59 | 4 | 0 | `reading-complete/` | +| `npx playwright test tests/e2e/submission-idempotency.spec.ts tests/e2e/phase3-media-runtime.spec.ts tests/e2e/phase4-empty-error-permission.spec.ts --workers=2` | 21 | 35 | 0 | `shared-final/` | +| `npx playwright test tests/e2e/media.spec.ts --project=chromium --project=webkit --workers=1` | 6 | 0 | 0 | `media-maintained-final/` | +| **Final total** | **209** | **63** | **0** | No browser launch failures or automatic retries. | + +The five new public specs cover all seven configured profiles: Chromium, WebKit, Firefox, mobile Firefox, mobile WebKit, Firefox without JavaScript, and Chromium without JavaScript. Their total is 182 passes and 28 enhancement-only skips. Native posting tests explicitly disable JavaScript in all seven profiles. Real WAV and H.264 native playback was verified in all seven; enhanced corruption/download/recovery runs in the five JavaScript profiles. All nine themes were exercised at desktop, narrow portrait, and short landscape widths; new audio error text/download actions also met the measured 4.5 contrast threshold in all nine themes. + +Additional broader maintained-suite evidence from earlier in implementation: 196 passed / 140 explicit project/mode skips across twelve specs (`regressions/`), plus eight passed / thirteen applicability skips for search, board pagination, preferences, sage and poll journeys (`public-journeys/`). These are separate earlier runs, not added to the final total. Shared error/permission, media and atomic-submission paths were rechecked against the final implementation as listed above. + +Earlier failures remain archived. They include the reproduced Firefox transport/history failures, the DORFic caption contrast defect, an early media failure before script initialization, test fixtures that did not match route body limits, pointer-click focus assumptions in WebKit, and intermittent native modified-click behavior under concurrent runs. Corrected keyboard tests and an unchanged-timeout clean reading matrix passed; click diagnostics verified real Meta input with `defaultPrevented: false`. No browser skips or failed attempts were counted as passes. + +Representative before screenshots: `before-chromium-chanclassic-thread-320.png`, `before-chromium-report-landscape.png`, native recovery captures under `native-before-poll/`, and catalog reproduction under `catalog-before/screenshots/`. Representative inspected after screenshots: + +- `after-native-oversized-chromium.png` and `after-native-oversized-mobile-firefox.png`: received draft/poll and explicit reselection recovery. +- `captcha-refreshed-after-chromium.png` and `uncertain-response-after-chromium.png`: preserved enhanced composer. +- `after-chromium-chanclassic-thread-320.png` and `after-mobile-webkit-preferences-landscape.png`: density, wrapping and reachable focused controls. +- `reading-complete/screenshots/chromium-wav-corrupt-dorfic.png` and `reading-complete/screenshots/mobile-webkit-video-corrupt.png`: readable native-media failure actions. +- `reading-complete/screenshots/firefox-catalog-last-reply.png` and `reading-complete/screenshots/mobile-firefox-search-page-restored.png`: sorting and restored search context. + +All paths in this section are relative to `output/playwright/public-polish/`; each run also retains its `results.json`, HTML report and diagnostics. + +## Changed files + +- `src/db/threads.rs`: latest-post aggregate and database regression. +- `src/models.rs`: latest-post time in the existing thread model. +- `src/handlers/board/create_thread.rs`, `src/handlers/thread.rs`: posting error and draft recovery. +- `src/handlers/mod.rs`: shared bounded multipart recovery and parser regressions. +- `src/handlers/render.rs`: existing test factory updated for the thread field. +- `src/templates/board.rs`, `src/templates/thread.rs`: search context, catalog sort data, poll anchor, renderer regressions and test factories. +- `src/templates/forms.rs`: retained poll/draft controls, native recovery advice and upload status semantics. +- `src/utils/sanitize.rs`: extension-preserving bounded filename sanitization and regressions. +- `static/main.js`, `static/style.css`: posting/CAPTCHA recovery, quote history/previews, mobile dialog focus, reduced-motion scrolling, catalog sorting and readable media failure feedback. +- `tests/e2e/public-polish-posting.spec.ts`, `tests/e2e/public-polish-native.spec.ts`, `tests/e2e/public-reading-polish.spec.ts`, `tests/e2e/public-catalog-polish.spec.ts`, `tests/e2e/public-polish-visual.spec.ts`: maintained-harness public journey regressions. +- `CHANGELOG.md`, `docs/public-interface-polish-audit.md`: change summary, reproductions, evidence and coverage limits. + +## Coverage boundaries + +- Mobile Firefox/WebKit are emulated viewport/touch/user-agent profiles, not physical devices. Physical mobile keyboards, screen-reader announcement order, and browser-native zoom UI were not exercised. +- No-JavaScript CAPTCHA refresh is a document navigation: its visible warning explains copying unsent content first. Browsers cannot programmatically restore file selections into a newly rendered native form. +- Malformed, stalled, or sufficiently large rejected streams may exceed the bounded recovery allowance. Only received controls can be saved; partial recovery says so explicitly. Browser Back is unreliable after some native upload errors, so delivered error pages provide copyable drafts. Abrupt transport failures can prevent an error page from arriving at all. Upload size, aggregate, field, timeout, CSRF, and concurrency checks remain enforced. +- Full optional media-toolchain preflight cannot pass on this host because FFmpeg lacks `libwebp`; it is not counted as a passing codec/transcoding matrix. +- CAPTCHA expiry rejection is checked through the real consumed-challenge HTTP path and existing timestamp-expiry Rust tests. A five-minute elapsed browser expiry wait is not counted as exercised. +- Skips are reported separately from passes. Superseded attempts retain evidence of test-authoring assumptions, stale binary runs, native-details focus timing, and browser interception behavior; they are not final passes or silently retried tests. diff --git a/src/db/threads.rs b/src/db/threads.rs index 6386f90f..29915f21 100644 --- a/src/db/threads.rs +++ b/src/db/threads.rs @@ -49,6 +49,7 @@ pub struct PollInsert<'a> { /// 1 `t.board_id` 5 t.locked 9 `op.file_path` 13 op.id (`op_id`) /// 2 t.subject 6 t.sticky 10 `op.thumb_path` 14 t.archived /// 3 `t.created_at` 7 `t.reply_count` 11 op.name 15 `image_count` +/// 16 `last_post_at` fn map_thread(row: &rusqlite::Row<'_>) -> rusqlite::Result { Ok(Thread { id: row.get(0)?, @@ -56,6 +57,7 @@ fn map_thread(row: &rusqlite::Row<'_>) -> rusqlite::Result { subject: row.get(2)?, created_at: row.get(3)?, bumped_at: row.get(4)?, + last_post_at: row.get(16)?, locked: row.get::<_, i32>(5)? != 0, sticky: row.get::<_, i32>(6)? != 0, reply_count: row.get(7)?, @@ -130,10 +132,11 @@ const THREAD_SELECT: &str = " t.locked, t.sticky, t.reply_count, op.body, op.file_path, op.thumb_path, op.name, op.tripcode, op.id, t.archived, - COUNT(DISTINCT fp.id) AS image_count + COUNT(DISTINCT CASE WHEN fp.file_path IS NOT NULL THEN fp.id END) AS image_count, + COALESCE(MAX(fp.created_at), t.created_at) AS last_post_at FROM threads t JOIN posts op ON op.thread_id = t.id AND op.is_op = 1 - LEFT JOIN posts fp ON fp.thread_id = t.id AND fp.file_path IS NOT NULL"; + LEFT JOIN posts fp ON fp.thread_id = t.id"; /// Get paginated threads for a board with OP preview data. /// Sticky threads float to the top, then sorted by most recent bump. @@ -1041,6 +1044,75 @@ mod tests { } } + #[test] + fn listing_last_post_includes_non_bumping_text_and_audio_without_counting_them_as_images( + ) -> Result<()> { + let conn = test_conn()?; + let board_id = create_board(&conn, "read", "Reading", "", false)?; + let thread_id = create_plain_thread(&conn, board_id, "opening post only")?; + conn.execute( + "UPDATE threads SET created_at = 100, bumped_at = 100 WHERE id = ?1", + params![thread_id], + )?; + conn.execute( + "UPDATE posts SET created_at = 100 WHERE thread_id = ?1", + params![thread_id], + )?; + let thread = super::get_thread(&conn, thread_id)?.context("opening thread should exist")?; + anyhow::ensure!( + thread.last_post_at == 100 && thread.image_count == 0, + "a thread without replies uses the opening-post time and has no images" + ); + + let mut image = plain_reply(board_id, thread_id); + image.file_path = Some("read/image.png".to_owned()); + let image_id = create_reply_with_thread_update(&conn, &image, "", false, None)?; + let mut audio = plain_reply(board_id, thread_id); + audio.audio_file_path = Some("read/audio.wav".to_owned()); + let audio_id = create_reply_with_thread_update(&conn, &audio, "", false, None)?; + let text_id = create_reply_with_thread_update( + &conn, + &plain_reply(board_id, thread_id), + "", + false, + None, + )?; + for (post_id, timestamp) in [(image_id, 200), (audio_id, 300), (text_id, 400)] { + conn.execute( + "UPDATE posts SET created_at = ?1 WHERE id = ?2", + params![timestamp, post_id], + )?; + } + let thread = + super::get_thread(&conn, thread_id)?.context("thread should exist after replies")?; + anyhow::ensure!( + thread.last_post_at == 400 && thread.bumped_at == 100 && thread.image_count == 1, + "sage text replies update last-post time while bump time and image counts remain correct" + ); + let active = super::get_threads_for_board(&conn, board_id, 20, 0)?; + anyhow::ensure!(active.len() == 1, "the active listing contains one thread"); + let active = active.first().context("active thread should exist")?; + anyhow::ensure!( + active.last_post_at == 400 && active.image_count == 1, + "catalog/index listing must retain the same post and image aggregates" + ); + conn.execute( + "UPDATE threads SET archived = 1 WHERE id = ?1", + params![thread_id], + )?; + let archived = super::get_archived_threads_for_board(&conn, board_id, 20, 0)?; + anyhow::ensure!( + archived.len() == 1, + "the archive listing contains one thread" + ); + let archived = archived.first().context("archived thread should exist")?; + anyhow::ensure!( + archived.last_post_at == 400 && archived.image_count == 1, + "archive listing must retain the same post and image aggregates" + ); + Ok(()) + } + fn pending_upload_op(id: &str) -> crate::pending_fs::PendingFsOpInsert { crate::pending_fs::PendingFsOpInsert { id: id.to_owned(), diff --git a/src/handlers/board/create_thread.rs b/src/handlers/board/create_thread.rs index f2a6f445..878fe4a4 100644 --- a/src/handlers/board/create_thread.rs +++ b/src/handlers/board/create_thread.rs @@ -3,7 +3,7 @@ use super::{ handled_post_error_status, identity_key, index_reply_previews, index_threads_per_page, is_xml_http_request, make_scoped_csrf_form_token, parse_post_multipart, posting, remember_owned_post_until_with_secure, render, self_action_window_secs, - should_set_public_secure_cookie, templates, unlock_redirect_url, user_preferences_from_jar, + should_set_public_secure_cookie, unlock_redirect_url, user_preferences_from_jar, xhr_error_response, xhr_post_error_response, xhr_redirect_response, AppError, AppState, BoardAccessDecision, BoardAccessRequirement, CookieJar, HashMap, HeaderMap, Html, Multipart, Path, PostFormData, Redirect, Response, Result, SecureCookieContext, State, @@ -54,7 +54,8 @@ pub(in crate::server) async fn create_thread( }; let csrf_cookie = jar.get("csrf_token").map(|c| c.value().to_owned()); - let form = tokio::time::timeout( + let mut parse_draft = crate::templates::forms::PostFormState::default(); + let parsed_form = tokio::time::timeout( crate::handlers::PUBLIC_UPLOAD_TIMEOUT, parse_post_multipart( multipart, @@ -63,23 +64,41 @@ pub(in crate::server) async fn create_thread( access_context.board.max_video_size_bytes(), access_context.board.max_audio_size_bytes(), access_context.board.max_pdf_size_bytes(), - &state.media_upload_gate, + crate::handlers::PostMultipartContext { + media_upload_gate: &state.media_upload_gate, + draft: &mut parse_draft, + }, ), ) .await - .map_err(|_error| AppError::BadRequest("Upload timed out. Please try again.".into()))??; + .map_err(|_error| AppError::BadRequest("Upload timed out. Please try again.".into())) + .and_then(std::convert::identity); + let form = match parsed_form { + Ok(form) => form, + Err(error) => { + if xhr_request { + return xhr_post_error_response(error); + } + return Ok(crate::handlers::native_post_error_response( + error, + Some(&parse_draft), + )); + } + }; + + let post_form_state = parse_draft; if !form.csrf_verified { - return Err(AppError::Forbidden("CSRF token mismatch.".into())); + let error = AppError::Forbidden("CSRF token mismatch. Reload the posting page for a fresh form after copying your draft.".into()); + if xhr_request { + return xhr_post_error_response(error); + } + return Ok(crate::handlers::native_post_error_response( + error, + Some(&post_form_state), + )); } - let post_form_state = templates::forms::PostFormState { - name: form.name.clone(), - subject: form.subject.clone(), - body: form.body.clone(), - sage: form.sage, - }; - // Also extract csrf_token before spawn_blocking so the ban page appeal form works. let ban_csrf_token = csrf_cookie.clone().unwrap_or_default(); @@ -167,7 +186,10 @@ pub(in crate::server) async fn create_thread( if xhr_request { return xhr_post_error_response(error); } - return Err(error); + return Ok(crate::handlers::native_post_error_response( + error, + Some(&post_form_state), + )); } }; if xhr_request { diff --git a/src/handlers/mod.rs b/src/handlers/mod.rs index 84831424..2f8ad974 100644 --- a/src/handlers/mod.rs +++ b/src/handlers/mod.rs @@ -24,7 +24,7 @@ use axum::{ body::Body, extract::{Multipart, Request}, middleware::Next, - response::Response, + response::{IntoResponse as _, Response}, }; use futures::StreamExt as _; use std::collections::HashSet; @@ -459,8 +459,22 @@ fn error_chain_contains(error: &(dyn std::error::Error + 'static), needle: &str) #[derive(Default)] struct PublicMultipartBudget { + /// Count shared by normal parsing and rejected-upload recovery. fields_seen: usize, + /// Aggregate field-data bytes observed across the request. bytes_seen: usize, + /// Additional allowance activated only after a board upload limit rejects a file. + rejected_upload: Option, +} + +/// One shared allowance for rejected field data and trailing draft controls. +struct RejectedUploadRecovery { + /// Rejected excess and trailing controls consumed within the shared allowance. + bytes_seen: usize, + /// Single deadline for the rejected field and every trailing control. + deadline: tokio::time::Instant, + /// Whether the rejected field ended before the allowance was exhausted. + field_complete: bool, } impl PublicMultipartBudget { @@ -481,6 +495,16 @@ impl PublicMultipartBudget { "Multipart upload is too large.".into(), )); } + if let Some(recovery) = &mut self.rejected_upload { + recovery.bytes_seen = recovery.bytes_seen.saturating_add(len); + if recovery.bytes_seen > REJECTED_UPLOAD_DISCARD_MAX_BYTES + || tokio::time::Instant::now() >= recovery.deadline + { + return Err(AppError::UploadTooLarge( + "Rejected upload recovery limit reached.".into(), + )); + } + } Ok(()) } } @@ -562,13 +586,59 @@ async fn discard_unknown_public_multipart_field( // Streaming multipart size limit // -// Upload fields stream directly to disk and abort with HTTP 413 as soon as the -// running total exceeds the configured board limit. +// Upload fields stream directly to disk and stop staging at the configured +// board limit. A small, time-bounded discard can then deliver the original 413 +// without retaining or processing excess bytes. // // Text fields (CSRF token, post body, …) use the same chunked parser with a // small fixed cap, so disabling Axum's route-level body limit for upload routes // does not leave text fields unbounded. +/// Maximum additional upload bytes discarded after a board-size rejection. +const REJECTED_UPLOAD_DISCARD_MAX_BYTES: usize = 8 * 1024 * 1024; +/// Maximum time spent discarding a rejected upload before closing its request. +const REJECTED_UPLOAD_DISCARD_TIMEOUT: Duration = Duration::from_secs(1); + +/// Discards a small finite rejected field so browsers can receive its original error. +/// Excess bytes never reach disk, and every read retains the aggregate upload budget. +async fn discard_rejected_upload_field( + field: &mut axum::extract::multipart::Field<'_>, + budget: &mut PublicMultipartBudget, + initial_excess_bytes: usize, +) -> bool { + let deadline = tokio::time::Instant::now() + REJECTED_UPLOAD_DISCARD_TIMEOUT; + budget.rejected_upload = Some(RejectedUploadRecovery { + bytes_seen: initial_excess_bytes, + deadline, + field_complete: false, + }); + if initial_excess_bytes > REJECTED_UPLOAD_DISCARD_MAX_BYTES { + return false; + } + let discard = async { + loop { + // Check ready streams too; the outer timer also bounds stalled reads. + if tokio::time::Instant::now() >= deadline { + return false; + } + let Ok(chunk) = field.chunk().await else { + return false; + }; + let Some(chunk) = chunk else { + return true; + }; + if budget.note_chunk(chunk.len()).is_err() { + return false; + } + } + }; + let complete = matches!(tokio::time::timeout_at(deadline, discard).await, Ok(true)); + if let Some(recovery) = &mut budget.rejected_upload { + recovery.field_complete = complete; + } + complete +} + async fn stream_field_to_temp_file( mut field: axum::extract::multipart::Field<'_>, max_bytes: usize, @@ -612,6 +682,13 @@ async fn stream_field_to_temp_file( limit_bytes = max_bytes, "multipart upload field exceeded board limit" ); + // Keep the media permit until this bounded discard completes; reject + // regardless of the outcome and never stage or inspect excess bytes. + let excess = size_bytes + .saturating_add(chunk.len()) + .saturating_sub(max_bytes); + let _discard_completed = + discard_rejected_upload_field(&mut field, budget, excess).await; return Err(AppError::UploadTooLarge(format!( "File too large. Maximum upload size is {}.", format_upload_limit(max_bytes) @@ -722,6 +799,168 @@ pub(crate) struct PostFormData { pub captcha_answer: String, } +/// Keeps native posting failures actionable without changing their status or security checks. +pub(crate) fn native_post_error_response( + error: AppError, + prefill: Option<&crate::templates::forms::PostFormState>, +) -> Response { + let mut response = error.into_response(); + let Some(crate::error::ErrorPage::Message(message)) = + response.extensions().get::() + else { + // Ban pages retain their dedicated explanation and appeal form. + return response; + }; + let body = crate::templates::forms::post_error_recovery_body(message, prefill); + let html = crate::templates::base_layout( + "Posting error", + None, + &body, + "", + &crate::templates::live_boards(), + None, + None, + false, + "/", + ); + response + .extensions_mut() + .insert(crate::error::ErrorPage::Content { + title: "Posting error".into(), + body, + }); + *response.body_mut() = Body::from(html); + response +} + +/// Request resources and a bounded draft retained even when parsing fails. +pub(crate) struct PostMultipartContext<'a> { + /// Existing concurrency gate held while upload bytes are staged or discarded. + pub media_upload_gate: &'a crate::middleware::MediaUploadGate, + /// Escaped at rendering; records only bounded recoverable user controls. + pub draft: &'a mut crate::templates::forms::PostFormState, +} + +/// Records only known bounded user controls; never tokens, CAPTCHA answers or uploads. +fn capture_post_draft( + name: &str, + value: &str, + draft: &mut crate::templates::forms::PostFormState, +) -> Result<()> { + match name { + "name" => value.clone_into(&mut draft.name), + "subject" => value.clone_into(&mut draft.subject), + "body" => value.clone_into(&mut draft.body), + "sage" => { + draft.sage = value == "1" + || value.eq_ignore_ascii_case("on") + || value.eq_ignore_ascii_case("true"); + } + "poll_question" if value.chars().count() <= 500 => { + value.clone_into(&mut draft.poll.get_or_insert_default().question); + } + "poll_option" => { + let value = value.trim(); + if !value.is_empty() { + let poll = draft.poll.get_or_insert_default(); + if poll.options.len() >= 20 || value.chars().count() > 200 { + return Err(AppError::BadRequest( + "Poll recovery control exceeds its limit.".into(), + )); + } + poll.options.push(value.to_owned()); + } + } + "poll_duration_value" => { + value.clone_into(&mut draft.poll.get_or_insert_default().duration_value); + } + "poll_duration_unit" => { + value.clone_into(&mut draft.poll.get_or_insert_default().duration_unit); + } + "poll_question" => { + return Err(AppError::BadRequest( + "Poll recovery question exceeds its limit.".into(), + )) + } + _ => {} + } + Ok(()) +} + +/// Reads trailing controls only after a finite rejected field; shares its cap and deadline. +async fn recover_rejected_post_tail( + multipart: &mut Multipart, + budget: &mut PublicMultipartBudget, + draft: &mut crate::templates::forms::PostFormState, +) { + let Some(recovery) = budget + .rejected_upload + .as_ref() + .filter(|state| state.field_complete) + else { + return; + }; + let deadline = recovery.deadline; + let recover = async { + loop { + if tokio::time::Instant::now() >= deadline { + return Err(AppError::BadRequest("Draft recovery timed out.".into())); + } + let next_field = multipart + .next_field() + .await + .map_err(|error| multipart_read_error("draft recovery", &error))?; + let Some(mut field) = next_field else { + return Ok(()); + }; + budget.note_field()?; + let name = field.name().unwrap_or_default().to_owned(); + match name.as_str() { + "name" + | "subject" + | "body" + | "sage" + | "poll_question" + | "poll_option" + | "poll_duration_value" + | "poll_duration_unit" => { + let value = read_text_field(field, budget).await?; + capture_post_draft(&name, &value, draft)?; + } + "file" | "audio_file" | "image_file" => { + draft.had_attachments |= field.file_name().is_some_and(|name| !name.is_empty()); + loop { + let next_chunk = field.chunk().await.map_err(|error| { + multipart_read_error("rejected upload tail", &error) + })?; + let Some(chunk) = next_chunk else { + break; + }; + budget.note_chunk(chunk.len())?; + } + } + _ => discard_unknown_public_multipart_field(field, budget).await?, + } + } + }; + if matches!(tokio::time::timeout_at(deadline, recover).await, Ok(Ok(()))) { + draft.incomplete = false; + } +} + +/// Retains the original upload error after bounded best-effort draft recovery. +async fn recover_upload_parse_error( + result: Result, + multipart: &mut Multipart, + budget: &mut PublicMultipartBudget, + draft: &mut crate::templates::forms::PostFormState, +) -> Result { + if result.is_err() { + recover_rejected_post_tail(multipart, budget, draft).await; + } + result +} + /// Drain all fields from a multipart form into [`PostFormData`]. /// `csrf_cookie` is the value from the browser cookie for CSRF verification. #[expect( @@ -739,8 +978,17 @@ pub(crate) async fn parse_post_multipart( max_video_size: usize, max_audio_size: usize, max_pdf_size: usize, - media_upload_gate: &crate::middleware::MediaUploadGate, + context: PostMultipartContext<'_>, ) -> Result { + let PostMultipartContext { + media_upload_gate, + draft, + } = context; + draft.incomplete = true; + draft.poll = Some(crate::templates::forms::PollFormState { + duration_unit: "hours".into(), + ..Default::default() + }); tracing::info!(target: LOG_TARGET, max_image_bytes = max_image_size, max_video_bytes = max_video_size, @@ -760,7 +1008,7 @@ pub(crate) async fn parse_post_multipart( let mut image_file: Option<(TempUpload, String)> = None; let mut poll_question = String::new(); let mut poll_options: Vec = Vec::new(); - let mut poll_duration_value: Option = None; + let mut poll_duration_value = String::new(); let mut poll_duration_unit = String::from("hours"); let mut sage = false; let mut captcha_id = String::new(); @@ -788,13 +1036,23 @@ pub(crate) async fn parse_post_multipart( Some("submission_token") => { submission_token = read_text_field(field, &mut budget).await?; } - Some("name") => name = read_text_field(field, &mut budget).await?, - Some("subject") => subject = read_text_field(field, &mut budget).await?, - Some("body") => body = read_text_field(field, &mut budget).await?, + Some("name") => { + name = read_text_field(field, &mut budget).await?; + capture_post_draft("name", &name, draft)?; + } + Some("subject") => { + subject = read_text_field(field, &mut budget).await?; + capture_post_draft("subject", &subject, draft)?; + } + Some("body") => { + body = read_text_field(field, &mut budget).await?; + capture_post_draft("body", &body, draft)?; + } Some("deletion_token") => deletion_token = read_text_field(field, &mut budget).await?, Some("sage") => { let v = read_text_field(field, &mut budget).await?; sage = v == "1" || v.eq_ignore_ascii_case("on") || v.eq_ignore_ascii_case("true"); + capture_post_draft("sage", &v, draft)?; } Some("captcha_id") => captcha_id = read_text_field(field, &mut budget).await?, Some("captcha_answer") => { @@ -807,6 +1065,7 @@ pub(crate) async fn parse_post_multipart( "Poll question must be 500 characters or fewer.".into(), )); } + capture_post_draft("poll_question", &v, draft)?; poll_question = v; } Some("poll_option") => { @@ -823,15 +1082,17 @@ pub(crate) async fn parse_post_multipart( "Each poll option must be 200 characters or fewer.".into(), )); } + capture_post_draft("poll_option", &trimmed, draft)?; poll_options.push(trimmed); } } Some("poll_duration_value") => { - let v = read_text_field(field, &mut budget).await?; - poll_duration_value = v.trim().parse::().ok(); + poll_duration_value = read_text_field(field, &mut budget).await?; + capture_post_draft("poll_duration_value", &poll_duration_value, draft)?; } Some("poll_duration_unit") => { poll_duration_unit = read_text_field(field, &mut budget).await?; + capture_post_draft("poll_duration_unit", &poll_duration_unit, draft)?; } Some("file") => { if !seen_upload_slots.insert("file") { @@ -839,7 +1100,8 @@ pub(crate) async fn parse_post_multipart( "Duplicate upload field 'file'.".into(), )); } - file = read_upload_field( + draft.had_attachments |= field.file_name().is_some_and(|name| !name.is_empty()); + let upload_result = read_upload_field( field, max_image_size .max(max_video_size) @@ -851,7 +1113,11 @@ pub(crate) async fn parse_post_multipart( media_upload_gate, &mut media_upload_guard, ) - .await?; + .await; + draft.had_attachments |= upload_result.as_ref().is_ok_and(Option::is_some); + file = + recover_upload_parse_error(upload_result, &mut multipart, &mut budget, draft) + .await?; } Some("audio_file") => { if !seen_upload_slots.insert("audio_file") { @@ -859,7 +1125,8 @@ pub(crate) async fn parse_post_multipart( "Duplicate upload field 'audio_file'.".into(), )); } - audio_file = read_upload_field( + draft.had_attachments |= field.file_name().is_some_and(|name| !name.is_empty()); + let upload_result = read_upload_field( field, max_audio_size, "audio", @@ -868,7 +1135,11 @@ pub(crate) async fn parse_post_multipart( media_upload_gate, &mut media_upload_guard, ) - .await?; + .await; + draft.had_attachments |= upload_result.as_ref().is_ok_and(Option::is_some); + audio_file = + recover_upload_parse_error(upload_result, &mut multipart, &mut budget, draft) + .await?; } Some("image_file") => { if !seen_upload_slots.insert("image_file") { @@ -876,7 +1147,8 @@ pub(crate) async fn parse_post_multipart( "Duplicate upload field 'image_file'.".into(), )); } - image_file = read_upload_field( + draft.had_attachments |= field.file_name().is_some_and(|name| !name.is_empty()); + let upload_result = read_upload_field( field, max_image_size, "image", @@ -885,7 +1157,11 @@ pub(crate) async fn parse_post_multipart( media_upload_gate, &mut media_upload_guard, ) - .await?; + .await; + draft.had_attachments |= upload_result.as_ref().is_ok_and(Option::is_some); + image_file = + recover_upload_parse_error(upload_result, &mut multipart, &mut budget, draft) + .await?; } _ => { discard_unknown_public_multipart_field(field, &mut budget).await?; @@ -893,13 +1169,15 @@ pub(crate) async fn parse_post_multipart( } } + draft.incomplete = false; + // Convert duration value + unit → seconds (saturating to prevent overflow). // The unit is validated against an explicit allow-list (case-insensitive) so // that a tampered form field does not silently multiply by an arbitrary factor. let poll_duration_secs = if poll_question.trim().is_empty() { None } else { - match poll_duration_value { + match poll_duration_value.trim().parse::().ok() { None => None, Some(v) => { let unit = poll_duration_unit.trim().to_ascii_lowercase(); @@ -1458,7 +1736,7 @@ trailer << /Root 1 0 R >> boundary: &str, body: Vec, ) -> anyhow::Result { - let request = Request::builder() + let mut request = Request::builder() .method("POST") .uri("/parse") .header( @@ -1467,11 +1745,260 @@ trailer << /Root 1 0 R >> ) .body(Body::from(body)) .context("build multipart extraction request")?; + // Match public posting routes: the parser enforces its own board and aggregate caps. + axum::extract::DefaultBodyLimit::disable().apply(&mut request); axum::extract::Multipart::from_request(request, &()) .await .map_err(|rejection| anyhow::anyhow!(rejection.to_string())) } + #[tokio::test] + async fn rejected_upload_discard_finishes_without_staging_bytes() -> anyhow::Result<()> { + let (boundary, body) = + multipart_body_with_files(&[], &[("file", "oversized.png", &[1; 2048], "image/png")]); + let mut multipart = multipart_from_bytes(&boundary, body).await?; + let mut field = multipart + .next_field() + .await? + .context("missing upload field")?; + let mut budget = super::PublicMultipartBudget::default(); + ensure!(super::discard_rejected_upload_field(&mut field, &mut budget, 0).await); + ensure!(budget.bytes_seen == 2048); + ensure!(field.chunk().await?.is_none()); + Ok(()) + } + + #[tokio::test] + async fn rejected_upload_discard_stops_at_excess_and_aggregate_limits() -> anyhow::Result<()> { + let (boundary, body) = + multipart_body_with_files(&[], &[("file", "oversized.png", &[1; 2048], "image/png")]); + let mut multipart = multipart_from_bytes(&boundary, body).await?; + let mut field = multipart + .next_field() + .await? + .context("missing upload field")?; + let mut budget = super::PublicMultipartBudget::default(); + ensure!( + !super::discard_rejected_upload_field( + &mut field, + &mut budget, + super::REJECTED_UPLOAD_DISCARD_MAX_BYTES + ) + .await + ); + ensure!(budget.bytes_seen == 2048); + + let (boundary, body) = + multipart_body_with_files(&[], &[("file", "oversized.png", &[1; 2048], "image/png")]); + let mut multipart = multipart_from_bytes(&boundary, body).await?; + let mut field = multipart + .next_field() + .await? + .context("missing upload field")?; + let mut budget = super::PublicMultipartBudget { + fields_seen: 0, + bytes_seen: super::PUBLIC_MULTIPART_AGGREGATE_MAX_BYTES, + ..super::PublicMultipartBudget::default() + }; + ensure!(!super::discard_rejected_upload_field(&mut field, &mut budget, 0).await); + Ok(()) + } + + #[tokio::test] + async fn rejected_upload_discard_times_out_on_a_stalled_field() -> anyhow::Result<()> { + use futures::StreamExt as _; + let prefix = axum::body::Bytes::from_static(b"--stalled\r\nContent-Disposition: form-data; name=\"file\"; filename=\"stalled.png\"\r\n\r\npartial bytes"); + let stream = futures::stream::once(async { Ok::<_, std::io::Error>(prefix) }) + .chain(futures::stream::pending()); + let request = Request::builder() + .header( + header::CONTENT_TYPE, + "multipart/form-data; boundary=stalled", + ) + .body(Body::from_stream(stream))?; + let mut multipart = axum::extract::Multipart::from_request(request, &()).await?; + let mut field = multipart + .next_field() + .await? + .context("missing stalled upload field")?; + let mut budget = super::PublicMultipartBudget::default(); + let completed = tokio::time::timeout( + std::time::Duration::from_secs(5), + super::discard_rejected_upload_field(&mut field, &mut budget, 0), + ) + .await?; + ensure!(!completed); + Ok(()) + } + + /// Constructs the native form order: user text, rejected upload, then poll controls. + fn rejected_upload_with_tail( + tail: &[(&str, &str)], + later_upload_bytes: &[u8], + ) -> (String, Vec) { + let (boundary, mut body) = multipart_body_with_files( + &[ + ("name", "匿名"), + ("subject", "Subject "), + ("body", "Draft 日本語"), + ], + &[("file", "oversized.png", &[1; 2048], "image/png")], + ); + let closing = format!("--{boundary}--\r\n"); + body.truncate(body.len().saturating_sub(closing.len())); + let files = if later_upload_bytes.is_empty() { + Vec::new() + } else { + vec![("image_file", "later.png", later_upload_bytes, "image/png")] + }; + let (_, trailing) = multipart_body_with_files(tail, &files); + body.extend_from_slice(&trailing); + (boundary, body) + } + + #[tokio::test] + async fn rejected_upload_preserves_received_text_and_trailing_poll() -> anyhow::Result<()> { + let (boundary, body) = rejected_upload_with_tail( + &[ + ("sage", "1"), + ("poll_question", "Question 日本語?"), + ("poll_option", "One"), + ("poll_option", "Two"), + ("poll_option", "Three"), + ("poll_duration_value", "3"), + ("poll_duration_unit", "days"), + ("captcha_answer", "never retained"), + ("_csrf", "never retained"), + ], + b"later upload is discarded", + ); + let gate = crate::middleware::MediaUploadGate::new(); + let mut draft = crate::templates::forms::PostFormState::default(); + let result = parse_post_multipart( + multipart_from_bytes(&boundary, body).await?, + None, + 1024, + 1024, + 1024, + 1024, + super::PostMultipartContext { + media_upload_gate: &gate, + draft: &mut draft, + }, + ) + .await; + ensure!( + matches!(&result, Err(crate::error::AppError::UploadTooLarge(message)) if message.starts_with("File too large.")) + ); + ensure!( + draft.body == "Draft 日本語" + && draft.subject == "Subject " + && draft.name == "匿名" + ); + ensure!(draft.sage && draft.had_attachments && !draft.incomplete); + let poll = draft.poll.context("trailing poll not retained")?; + ensure!(poll.question == "Question 日本語?" && poll.options == ["One", "Two", "Three"]); + ensure!(poll.duration_value == "3" && poll.duration_unit == "days"); + Ok(()) + } + + #[tokio::test] + async fn rejected_upload_tail_shares_excess_budget_and_reports_partial_capture( + ) -> anyhow::Result<()> { + let bytes = vec![1; super::REJECTED_UPLOAD_DISCARD_MAX_BYTES]; + let (boundary, body) = + rejected_upload_with_tail(&[("poll_question", "Already captured?")], &bytes); + let gate = crate::middleware::MediaUploadGate::new(); + let mut draft = crate::templates::forms::PostFormState::default(); + let result = parse_post_multipart( + multipart_from_bytes(&boundary, body).await?, + None, + 1024, + 1024, + 1024, + 1024, + super::PostMultipartContext { + media_upload_gate: &gate, + draft: &mut draft, + }, + ) + .await; + ensure!( + matches!(&result, Err(crate::error::AppError::UploadTooLarge(message)) if message.starts_with("File too large.")), + "expected original board-limit rejection, received {:?}", + result.as_ref().err().map(ToString::to_string) + ); + drop(result); + ensure!(draft.body == "Draft 日本語" && draft.incomplete); + ensure!( + draft.poll.context("bounded earlier poll missing")?.question == "Already captured?" + ); + Ok(()) + } + + #[tokio::test] + async fn rejected_upload_tail_keeps_text_poll_and_field_limits() -> anyhow::Result<()> { + let long_text = "x".repeat(super::TEXT_MULTIPART_FIELD_MAX_BYTES + 1); + let long_question = "q".repeat(501); + let long_option = "o".repeat(201); + for tail in [ + vec![("poll_question", "q"); super::PUBLIC_MULTIPART_MAX_FIELDS], + vec![("poll_option", "option"); 21], + vec![("body", long_text.as_str())], + vec![("poll_question", long_question.as_str())], + vec![("poll_option", long_option.as_str())], + ] { + let (boundary, body) = rejected_upload_with_tail(&tail, &[]); + let gate = crate::middleware::MediaUploadGate::new(); + let mut draft = crate::templates::forms::PostFormState::default(); + let result = parse_post_multipart( + multipart_from_bytes(&boundary, body).await?, + None, + 1024, + 1024, + 1024, + 1024, + super::PostMultipartContext { + media_upload_gate: &gate, + draft: &mut draft, + }, + ) + .await; + ensure!( + matches!(&result, Err(crate::error::AppError::UploadTooLarge(message)) if message.starts_with("File too large.")) + ); + drop(result); + ensure!(draft.incomplete); + ensure!(draft + .poll + .as_ref() + .is_none_or(|poll| poll.options.len() <= 20)); + } + Ok(()) + } + + #[tokio::test] + async fn rejected_upload_tail_does_not_receive_a_second_timeout() -> anyhow::Result<()> { + let (boundary, body) = multipart_body_with_files(&[("body", "unread")], &[]); + let mut multipart = multipart_from_bytes(&boundary, body).await?; + let mut draft = crate::templates::forms::PostFormState { + body: "saved".into(), + incomplete: true, + ..Default::default() + }; + let mut budget = super::PublicMultipartBudget { + rejected_upload: Some(super::RejectedUploadRecovery { + bytes_seen: 1, + deadline: tokio::time::Instant::now(), + field_complete: true, + }), + ..Default::default() + }; + super::recover_rejected_post_tail(&mut multipart, &mut budget, &mut draft).await; + ensure!(draft.body == "saved" && draft.incomplete && budget.bytes_seen == 0); + Ok(()) + } + #[test] fn public_multipart_boundary_parser_accepts_quoted_parameter() -> anyhow::Result<()> { let parsed = super::parse_public_multipart_boundary( @@ -1555,7 +2082,10 @@ trailer << /Root 1 0 R >> 1_024, 5_000, 1_024, - &gate, + super::PostMultipartContext { + media_upload_gate: &gate, + draft: &mut crate::templates::forms::PostFormState::default(), + }, ) .await?; let (upload, _) = form.audio_file.as_ref().ok_or_else(|| { @@ -1582,7 +2112,10 @@ trailer << /Root 1 0 R >> 1_024, 5_000, 1_024, - &gate, + super::PostMultipartContext { + media_upload_gate: &gate, + draft: &mut crate::templates::forms::PostFormState::default(), + }, ) .await?; Ok("ok") @@ -1671,7 +2204,10 @@ trailer << /Root 1 0 R >> 1_024, 1_024, 1_024, - &gate, + super::PostMultipartContext { + media_upload_gate: &gate, + draft: &mut crate::templates::forms::PostFormState::default(), + }, ) .await?; Ok("ok") @@ -1688,7 +2224,10 @@ trailer << /Root 1 0 R >> 1_024, 1_024, 2_048, - &gate, + super::PostMultipartContext { + media_upload_gate: &gate, + draft: &mut crate::templates::forms::PostFormState::default(), + }, ) .await?; let (upload, _) = form.file.as_ref().ok_or_else(|| { @@ -1752,7 +2291,10 @@ trailer << /Root 1 0 R >> 1_024, 1_024, 1_024, - &gate, + super::PostMultipartContext { + media_upload_gate: &gate, + draft: &mut crate::templates::forms::PostFormState::default(), + }, ) .await?; @@ -1783,7 +2325,10 @@ trailer << /Root 1 0 R >> 1_024, 1_024, 1_024, - &gate, + super::PostMultipartContext { + media_upload_gate: &gate, + draft: &mut crate::templates::forms::PostFormState::default(), + }, ) .await?; diff --git a/src/handlers/render.rs b/src/handlers/render.rs index 6d55194d..d5e8eab5 100644 --- a/src/handlers/render.rs +++ b/src/handlers/render.rs @@ -273,6 +273,7 @@ mod tests { subject: Some("subject".into()), created_at: 100, bumped_at: 200, + last_post_at: 200, locked: false, sticky: false, archived: false, diff --git a/src/handlers/thread.rs b/src/handlers/thread.rs index 2f810762..1c693b0e 100644 --- a/src/handlers/thread.rs +++ b/src/handlers/thread.rs @@ -316,7 +316,8 @@ pub(in crate::server) async fn post_reply( }; let csrf_cookie = jar.get("csrf_token").map(|c| c.value().to_owned()); - let form = tokio::time::timeout( + let mut parse_draft = crate::templates::forms::PostFormState::default(); + let parsed_form = tokio::time::timeout( crate::handlers::PUBLIC_UPLOAD_TIMEOUT, parse_post_multipart( multipart, @@ -325,22 +326,44 @@ pub(in crate::server) async fn post_reply( access_context.board.max_video_size_bytes(), access_context.board.max_audio_size_bytes(), access_context.board.max_pdf_size_bytes(), - &state.media_upload_gate, + crate::handlers::PostMultipartContext { + media_upload_gate: &state.media_upload_gate, + draft: &mut parse_draft, + }, ), ) .await - .map_err(|_error| AppError::BadRequest("Upload timed out. Please try again.".into()))??; + .map_err(|_error| AppError::BadRequest("Upload timed out. Please try again.".into())) + .and_then(std::convert::identity); + let form = match parsed_form { + Ok(form) => form, + Err(error) => { + if xhr_request { + return crate::handlers::board::xhr_post_error_response(error); + } + return Ok(crate::handlers::native_post_error_response( + error, + Some(&parse_draft), + )); + } + }; + + // Replies do not expose thread subject or poll controls. + parse_draft.subject.clear(); + parse_draft.poll = None; + let post_form_state = parse_draft; if !form.csrf_verified { - return Err(AppError::Forbidden("CSRF token mismatch.".into())); + let error = AppError::Forbidden("CSRF token mismatch. Reload the posting page for a fresh form after copying your draft.".into()); + if xhr_request { + return crate::handlers::board::xhr_post_error_response(error); + } + return Ok(crate::handlers::native_post_error_response( + error, + Some(&post_form_state), + )); } - let post_form_state = crate::templates::forms::PostFormState { - name: form.name.clone(), - subject: String::new(), - body: form.body.clone(), - sage: form.sage, - }; // Extract csrf_token before spawn_blocking so the ban page appeal form works. let ban_csrf_token = csrf_cookie.clone().unwrap_or_default(); @@ -424,7 +447,10 @@ pub(in crate::server) async fn post_reply( if xhr_request { return crate::handlers::board::xhr_post_error_response(error); } - return Err(error); + return Ok(crate::handlers::native_post_error_response( + error, + Some(&post_form_state), + )); } }; if xhr_request { diff --git a/src/models.rs b/src/models.rs index ff6e3223..82f98b86 100644 --- a/src/models.rs +++ b/src/models.rs @@ -485,6 +485,8 @@ pub struct Thread { pub created_at: i64, /// Last bump time as a Unix timestamp. pub bumped_at: i64, + /// Latest post time, including replies that do not bump the thread. + pub last_post_at: i64, /// Whether new replies are prohibited. pub locked: bool, /// Whether the thread remains ahead of non-sticky threads. diff --git a/src/templates/board.rs b/src/templates/board.rs index 381fccec..80569ee7 100644 --- a/src/templates/board.rs +++ b/src/templates/board.rs @@ -537,7 +537,7 @@ fn render_catalog_card( }; format!( - r#"
+ r#"
{thumb} @@ -558,6 +558,7 @@ fn render_catalog_card( replies = thread.reply_count, created = thread.created_at, bumped = thread.bumped_at, + last_reply = thread.last_post_at, is_sticky = if thread.sticky { "1" } else { "0" }, is_pinned = if is_pinned { "1" } else { "0" }, board = escape_html(&board.short_name), @@ -1510,7 +1511,7 @@ pub fn search_page( escape_html(&result_label) ); for post in posts { - body.push_str(&super::thread::render_post( + body.push_str(&super::thread::render_search_post( post, &board.short_name, csrf_token, @@ -1528,7 +1529,6 @@ pub fn search_page( thread_op_id: None, video_audio_muted: user_preferences.video_audio_muted, }, - 0, )); } body.push_str(&render_pagination( @@ -1650,6 +1650,7 @@ mod tests { subject: Some("Thread subject".into()), created_at: 1_700_000_000, bumped_at: 1_700_000_100, + last_post_at: 1_700_000_100, locked: true, sticky: true, archived: false, diff --git a/src/templates/forms.rs b/src/templates/forms.rs index f1ceab09..939601b2 100644 --- a/src/templates/forms.rs +++ b/src/templates/forms.rs @@ -6,6 +6,7 @@ use crate::config::CONFIG; use crate::models::Board; use crate::utils::sanitize::escape_html; +use std::fmt::Write as _; /// User-entered fields preserved when a post form must be rendered again. #[derive(Clone, Debug, Default, PartialEq, Eq)] @@ -18,6 +19,54 @@ pub struct PostFormState { pub body: String, /// Whether the reply should avoid bumping its thread. pub sage: bool, + /// Poll draft entered for a new thread. + pub poll: Option, + /// Whether a rejected request contained attachments that must be reselected. + pub had_attachments: bool, + /// Whether parsing stopped before every submitted control could be captured. + pub incomplete: bool, +} + +/// Poll controls preserved independently of normalized server-side validation. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct PollFormState { + /// Poll question field. + pub question: String, + /// Nonempty poll option fields, in submitted order. + pub options: Vec, + /// Duration number as entered by the user. + pub duration_value: String, + /// Duration unit selected by the user. + pub duration_unit: String, +} + +/// Renders posting-only recovery advice and a copyable draft when available. +#[must_use] +pub fn post_error_recovery_body(message: &str, prefill: Option<&PostFormState>) -> String { + let draft = prefill.map_or_else(String::new, |state| { + let poll = state.poll.as_ref().map_or_else(String::new, |poll| { + let mut options = String::new(); + for (index, option) in poll.options.iter().enumerate() { + // Writing formatted text to a String cannot fail. + let _ = write!(options, "

Option {}: {}

", index + 1, escape_html(option)); + } + format!("

Poll: {}

{options}

Duration: {} {}

", + escape_html(&poll.question), escape_html(&poll.duration_value), escape_html(&poll.duration_unit)) + }); + let partial = if state.incomplete { "

Only controls received before the upload stopped are saved below. Check your name, subject, body, sage and poll options before retrying.

" } else { "" }; + format!(r#"{partial}
[ saved draft — copy before leaving ]
+

Name: {name}

Subject: {subject}

+ +

Sage: {sage}

{poll}
"#, + name = escape_html(&state.name), subject = escape_html(&state.subject), body = escape_html(&state.body), + sage = if state.sage { "yes" } else { "no" }) + }); + format!( + r#"

Post could not be completed

{message}

+

Copy the saved draft below before leaving. Your browser’s Back button may restore the original form, but some browsers clear it. Open a fresh posting form and paste your saved text if needed. Choose attachments again; browsers cannot restore file selections on a new page.

+

If the result is uncertain, check the thread or board for your post before submitting again.

{draft}

return home

"#, + message = escape_html(message) + ) } /// Upload capabilities needed to choose the form's media controls. @@ -37,8 +86,8 @@ const fn upload_progress_row() -> &'static str { upload
-
-
Preparing upload…
+ +
Preparing upload…
"# } @@ -107,6 +156,7 @@ fn render_captcha_row(board_short: &str, reply_suffix: &str, refresh_href: &str) Enter the text shown in the image. If it expires or fails, request a new challenge. + "#, board = board, suffix = reply_suffix, @@ -118,9 +168,10 @@ fn render_captcha_row(board_short: &str, reply_suffix: &str, refresh_href: &str) } /// Renders one numbered poll-option input row. -fn render_poll_option_row(option_number: usize) -> String { +fn render_poll_option_row(option_number: usize, value: &str) -> String { format!( - r#"
"# + r#"
"#, + value = escape_html(value), ) } @@ -261,6 +312,83 @@ fn render_single_upload_row(board: &Board, audio_image_hint: &str) -> String { ) } +/// Renders the shared poll creator, including native additional-option controls. +fn render_poll_creator(poll: Option<&PollFormState>) -> String { + let visible_option_count = poll.map_or(2, |state| { + state.options.len().clamp(2, POLL_OPTION_MAX_COUNT) + }); + let poll_option_rows: String = (1..=visible_option_count) + .map(|number| { + render_poll_option_row( + number, + poll.and_then(|state| state.options.get(number - 1)) + .map_or("", String::as_str), + ) + }) + .collect(); + let extra_poll_option_rows: String = (visible_option_count + 1..=POLL_OPTION_MAX_COUNT) + .map(|number| render_poll_option_row(number, "")) + .collect(); + let poll_question = poll.map_or("", |state| state.question.as_str()); + let poll_open = + if poll.is_some_and(|state| !state.question.is_empty() || !state.options.is_empty()) { + " open" + } else { + "" + }; + let duration_value = poll.map_or("24", |state| state.duration_value.as_str()); + let duration_unit = poll.map_or("hours", |state| state.duration_unit.as_str()); + format!( + r#" + + Poll +
+ [ 📊 Add a Poll to this thread ] +
+
+ +
+
+ {poll_option_rows} + +
+ +
+ +
+
+
+ "#, + poll_option_max_length = POLL_OPTION_MAX_LENGTH, + poll_option_max_count = POLL_OPTION_MAX_COUNT, + poll_option_rows = poll_option_rows, + poll_question = escape_html(poll_question), + duration_value = escape_html(duration_value), + hours_selected = if duration_unit == "hours" { + " selected" + } else { + "" + }, + minutes_selected = if duration_unit == "minutes" { + " selected" + } else { + "" + }, + days_selected = if duration_unit == "days" { + " selected" + } else { + "" + }, + ) +} + /// New-thread submission form. Embedded on board index and catalog pages. pub(super) fn new_thread_form( board_short: &str, @@ -289,10 +417,8 @@ pub(super) fn new_thread_form( String::new() }; - let poll_option_rows = [render_poll_option_row(1), render_poll_option_row(2)].concat(); - let extra_poll_option_rows: String = (3..=POLL_OPTION_MAX_COUNT) - .map(render_poll_option_row) - .collect(); + let poll_creator = render_poll_creator(prefill.and_then(|state| state.poll.as_ref())); + let attachment_recovery = attachment_recovery_row(prefill); let name_value = prefill.map_or("", |state| state.name.as_str()); let subject_value = prefill.map_or("", |state| state.subject.as_str()); let body_value = prefill.map_or("", |state| state.body.as_str()); @@ -323,35 +449,10 @@ pub(super) fn new_thread_form( {uploads_disabled_row} {upload_row} + {attachment_recovery} {upload_progress_row} {captcha_row} - - - Poll -
- [ 📊 Add a Poll to this thread ] -
-
- -
-
- {poll_option_rows} - -
- -
- -
-
-
- + {poll_creator}
@@ -366,12 +467,18 @@ pub(super) fn new_thread_form( upload_row = upload_row, upload_progress_row = upload_progress_row(), captcha_row = captcha_row, - poll_option_max_length = POLL_OPTION_MAX_LENGTH, - poll_option_max_count = POLL_OPTION_MAX_COUNT, - poll_option_rows = poll_option_rows, ) } +/// Explains the browser file-input restriction after a rejected native submission. +fn attachment_recovery_row(prefill: Option<&PostFormState>) -> &'static str { + if prefill.is_some_and(|state| state.had_attachments) { + r#"attachmentsChoose your attachments again. Your browser cannot restore file selections after this page reload."# + } else { + "" + } +} + /// Reply form injected into thread pages. pub(super) fn reply_form( board_short: &str, @@ -403,6 +510,7 @@ pub(super) fn reply_form( } else { String::new() }; + let attachment_recovery = attachment_recovery_row(prefill); let name_value = prefill.map_or("", |state| state.name.as_str()); let body_value = prefill.map_or("", |state| state.body.as_str()); let sage_checked = if prefill.is_some_and(|state| state.sage) { @@ -425,6 +533,7 @@ pub(super) fn reply_form( {uploads_disabled_row} {upload_row} + {attachment_recovery} {upload_progress_row} options Options @@ -450,7 +559,8 @@ pub(super) fn reply_form( mod tests { use super::{ build_upload_form_policy, new_thread_form, render_captcha_row, render_poll_option_row, - reply_form, PostFormState, AUDIO_ACCEPT, POLL_OPTION_MAX_COUNT, POLL_OPTION_MAX_LENGTH, + reply_form, PollFormState, PostFormState, AUDIO_ACCEPT, POLL_OPTION_MAX_COUNT, + POLL_OPTION_MAX_LENGTH, }; fn uploads_disabled_board() -> crate::models::Board { @@ -550,7 +660,7 @@ mod tests { #[test] fn poll_option_rows_share_the_same_max_length() { - let initial_row = render_poll_option_row(1); + let initial_row = render_poll_option_row(1, ""); assert!(initial_row.contains(&format!(r#"maxlength="{POLL_OPTION_MAX_LENGTH}""#))); let html = new_thread_form("test", "csrf", &uploads_disabled_board(), None, "/test"); @@ -588,6 +698,7 @@ mod tests { subject: "subject".into(), body: "draft body".into(), sage: true, + ..PostFormState::default() }; let thread_html = new_thread_form("test", "csrf", &board, Some(&state), "/test"); let reply_html = reply_form("test", 42, "csrf", &board, Some(&state)); @@ -606,6 +717,73 @@ mod tests { assert!(reply_html.contains(r#"name="sage" value="1" checked"#)); } + #[test] + fn rejected_thread_retains_escaped_poll_controls_and_extra_options() { + let state = PostFormState { + poll: Some(PollFormState { + question: " & 日本語".into(), + options: vec!["First".into(), "".into(), "Third".into()], + duration_value: "3".into(), + duration_unit: "days".into(), + }), + ..PostFormState::default() + }; + let html = new_thread_form( + "test", + "csrf", + &uploads_disabled_board(), + Some(&state), + "/test", + ); + + assert!(html.contains(r#"class="poll-creator" open"#)); + assert!(html.contains(r#"name="poll_question" value="<question> & 日本語""#)); + assert!(html.contains(r#"name="poll_option" value="<second>""#)); + assert!(html.contains(r#"name="poll_duration_value" value="3""#)); + assert!(html.contains(r#""#)); + let third_option = html.find(r#"name="poll_option" value="Third""#); + assert!(third_option.is_some()); + assert!(third_option < html.find("