max / makenotwork
9 files changed,
+163 insertions,
-25 deletions
| @@ -0,0 +1,61 @@ | |||
| 1 | + | //! Branded 404/500 responses for in-handler error paths. | |
| 2 | + | //! | |
| 3 | + | //! The router `.fallback` renders a branded, session-aware error page, but | |
| 4 | + | //! errors raised *inside* a handler (missing thread/community, a bad UUID in the | |
| 5 | + | //! path, a DB failure) used to short-circuit with bare `(StatusCode, &str)` | |
| 6 | + | //! plaintext — unbranded and jarring. These helpers render the same | |
| 7 | + | //! `Error404Template`/`Error500Template` instead. | |
| 8 | + | //! | |
| 9 | + | //! They render a **session-less** shell (logged-out header), because the deep | |
| 10 | + | //! shared helpers that raise these errors don't carry the session. The only | |
| 11 | + | //! per-process value the templates need is `mnw_base_url` (all the chrome nav | |
| 12 | + | //! links), which is immutable config; it's stashed in a `OnceLock` at startup | |
| 13 | + | //! rather than threaded through ~80 call sites. The fallback handler still | |
| 14 | + | //! renders the fully session-aware page for unmatched routes. | |
| 15 | + | ||
| 16 | + | use std::sync::{Arc, OnceLock}; | |
| 17 | + | ||
| 18 | + | use axum::http::StatusCode; | |
| 19 | + | use axum::response::{IntoResponse, Response}; | |
| 20 | + | ||
| 21 | + | use crate::templates::{Error404Template, Error500Template}; | |
| 22 | + | ||
| 23 | + | static MNW_BASE_URL: OnceLock<Arc<str>> = OnceLock::new(); | |
| 24 | + | ||
| 25 | + | /// Record the MNW base URL for branded error pages. Called once at startup | |
| 26 | + | /// (and by the test harness); later calls are ignored. | |
| 27 | + | pub fn init(mnw_base_url: Arc<str>) { | |
| 28 | + | let _ = MNW_BASE_URL.set(mnw_base_url); | |
| 29 | + | } | |
| 30 | + | ||
| 31 | + | fn base_url() -> Arc<str> { | |
| 32 | + | MNW_BASE_URL.get().cloned().unwrap_or_else(|| Arc::from("")) | |
| 33 | + | } | |
| 34 | + | ||
| 35 | + | /// Branded 404 page (session-less). Use from shared handler helpers in place of | |
| 36 | + | /// a bare `(StatusCode::NOT_FOUND, "Not found")`. | |
| 37 | + | pub fn not_found() -> Response { | |
| 38 | + | ( | |
| 39 | + | StatusCode::NOT_FOUND, | |
| 40 | + | Error404Template { | |
| 41 | + | csrf_token: None, | |
| 42 | + | session_user: None, | |
| 43 | + | mnw_base_url: base_url(), | |
| 44 | + | }, | |
| 45 | + | ) | |
| 46 | + | .into_response() | |
| 47 | + | } | |
| 48 | + | ||
| 49 | + | /// Branded 500 page (session-less). Use from shared handler helpers in place of | |
| 50 | + | /// a bare `(StatusCode::INTERNAL_SERVER_ERROR, "Internal server error")`. | |
| 51 | + | pub fn internal_error() -> Response { | |
| 52 | + | ( | |
| 53 | + | StatusCode::INTERNAL_SERVER_ERROR, | |
| 54 | + | Error500Template { | |
| 55 | + | csrf_token: None, | |
| 56 | + | session_user: None, | |
| 57 | + | mnw_base_url: base_url(), | |
| 58 | + | }, | |
| 59 | + | ) | |
| 60 | + | .into_response() | |
| 61 | + | } |
| @@ -3,6 +3,7 @@ | |||
| 3 | 3 | pub mod auth; | |
| 4 | 4 | pub mod config; | |
| 5 | 5 | pub mod csrf; | |
| 6 | + | pub mod error_page; | |
| 6 | 7 | pub mod internal_auth; | |
| 7 | 8 | pub mod link_preview; | |
| 8 | 9 | pub mod maintenance; |
| @@ -45,6 +45,7 @@ async fn main() { | |||
| 45 | 45 | } | |
| 46 | 46 | ||
| 47 | 47 | let config = Config::from_env(); | |
| 48 | + | multithreaded::error_page::init(config.mnw_base_url.clone()); | |
| 48 | 49 | ||
| 49 | 50 | // Optional S3 storage for image uploads | |
| 50 | 51 | let s3 = if let Some(ref s3_config) = config.s3 { |
| @@ -43,7 +43,12 @@ pub(in crate::routes) async fn forum_directory( | |||
| 43 | 43 | mt_db::queries::count_communities(&state.db).await | |
| 44 | 44 | } | |
| 45 | 45 | .unwrap_or(0); | |
| 46 | - | let pagination = Pagination::new(query.page.unwrap_or(1).max(1), total, per_page); | |
| 46 | + | let mut pagination = Pagination::new(query.page.unwrap_or(1).max(1), total, per_page); | |
| 47 | + | // Keep the archived filter across page links (else page 2+ reverts to the | |
| 48 | + | // default, non-archived listing). | |
| 49 | + | if viewing_archived { | |
| 50 | + | pagination = pagination.with_query_suffix("&filter=archived"); | |
| 51 | + | } | |
| 47 | 52 | let offset = pagination.offset(per_page); | |
| 48 | 53 | ||
| 49 | 54 | let rows = if viewing_archived { | |
| @@ -92,7 +97,7 @@ pub(in crate::routes) async fn project_forum( | |||
| 92 | 97 | .await | |
| 93 | 98 | .map_err(|e| { | |
| 94 | 99 | tracing::error!(error = ?e, "db error listing categories"); | |
| 95 | - | StatusCode::INTERNAL_SERVER_ERROR.into_response() | |
| 100 | + | crate::error_page::internal_error() | |
| 96 | 101 | })?; | |
| 97 | 102 | ||
| 98 | 103 | let role = if let Some(ref user) = session_user { | |
| @@ -144,7 +149,7 @@ pub(in crate::routes) async fn community_members( | |||
| 144 | 149 | .await | |
| 145 | 150 | .map_err(|e| { | |
| 146 | 151 | tracing::error!(error = ?e, "db error listing members"); | |
| 147 | - | StatusCode::INTERNAL_SERVER_ERROR.into_response() | |
| 152 | + | crate::error_page::internal_error() | |
| 148 | 153 | })?; | |
| 149 | 154 | ||
| 150 | 155 | let members = db_members | |
| @@ -186,9 +191,9 @@ pub(in crate::routes) async fn category( | |||
| 186 | 191 | .await | |
| 187 | 192 | .map_err(|e| { | |
| 188 | 193 | tracing::error!(error = ?e, "db error fetching category"); | |
| 189 | - | StatusCode::INTERNAL_SERVER_ERROR.into_response() | |
| 194 | + | crate::error_page::internal_error() | |
| 190 | 195 | })? | |
| 191 | - | .ok_or_else(|| StatusCode::NOT_FOUND.into_response())?; | |
| 196 | + | .ok_or_else(crate::error_page::not_found)?; | |
| 192 | 197 | ||
| 193 | 198 | let per_page: i64 = 25; | |
| 194 | 199 | ||
| @@ -204,7 +209,7 @@ pub(in crate::routes) async fn category( | |||
| 204 | 209 | .await | |
| 205 | 210 | .map_err(|e| { | |
| 206 | 211 | tracing::error!(error = ?e, "db error counting threads"); | |
| 207 | - | StatusCode::INTERNAL_SERVER_ERROR.into_response() | |
| 212 | + | crate::error_page::internal_error() | |
| 208 | 213 | })?; | |
| 209 | 214 | ||
| 210 | 215 | let pagination = Pagination::new(query.page.unwrap_or(1).max(1), total, per_page); | |
| @@ -216,7 +221,7 @@ pub(in crate::routes) async fn category( | |||
| 216 | 221 | .await | |
| 217 | 222 | .map_err(|e| { | |
| 218 | 223 | tracing::error!(error = ?e, "db error listing threads"); | |
| 219 | - | StatusCode::INTERNAL_SERVER_ERROR.into_response() | |
| 224 | + | crate::error_page::internal_error() | |
| 220 | 225 | })?; | |
| 221 | 226 | ||
| 222 | 227 | // Batch-fetch tags for all threads | |
| @@ -225,7 +230,7 @@ pub(in crate::routes) async fn category( | |||
| 225 | 230 | .await | |
| 226 | 231 | .map_err(|e| { | |
| 227 | 232 | tracing::error!(error = ?e, "db error fetching thread tags"); | |
| 228 | - | StatusCode::INTERNAL_SERVER_ERROR.into_response() | |
| 233 | + | crate::error_page::internal_error() | |
| 229 | 234 | })?; | |
| 230 | 235 | ||
| 231 | 236 | let mut tags_by_thread: HashMap<String, Vec<TagBadge>> = HashMap::new(); | |
| @@ -274,7 +279,7 @@ pub(in crate::routes) async fn category( | |||
| 274 | 279 | .await | |
| 275 | 280 | .map_err(|e| { | |
| 276 | 281 | tracing::error!(error = ?e, "db error listing tags"); | |
| 277 | - | StatusCode::INTERNAL_SERVER_ERROR.into_response() | |
| 282 | + | crate::error_page::internal_error() | |
| 278 | 283 | })?; | |
| 279 | 284 | let available_tags = db_tags | |
| 280 | 285 | .into_iter() | |
| @@ -317,15 +322,15 @@ pub(in crate::routes) async fn new_thread( | |||
| 317 | 322 | .await | |
| 318 | 323 | .map_err(|e| { | |
| 319 | 324 | tracing::error!(error = ?e, "db error fetching category"); | |
| 320 | - | StatusCode::INTERNAL_SERVER_ERROR.into_response() | |
| 325 | + | crate::error_page::internal_error() | |
| 321 | 326 | })? | |
| 322 | - | .ok_or_else(|| StatusCode::NOT_FOUND.into_response())?; | |
| 327 | + | .ok_or_else(crate::error_page::not_found)?; | |
| 323 | 328 | ||
| 324 | 329 | let db_tags = mt_db::queries::list_tags_for_community(&state.db, community.id) | |
| 325 | 330 | .await | |
| 326 | 331 | .map_err(|e| { | |
| 327 | 332 | tracing::error!(error = ?e, "db error listing tags"); | |
| 328 | - | StatusCode::INTERNAL_SERVER_ERROR.into_response() | |
| 333 | + | crate::error_page::internal_error() | |
| 329 | 334 | })?; | |
| 330 | 335 | let available_tags = db_tags | |
| 331 | 336 | .into_iter() | |
| @@ -363,9 +368,9 @@ pub(in crate::routes) async fn user_profile( | |||
| 363 | 368 | .await | |
| 364 | 369 | .map_err(|e| { | |
| 365 | 370 | tracing::error!(error = ?e, "db error fetching user profile"); | |
| 366 | - | StatusCode::INTERNAL_SERVER_ERROR.into_response() | |
| 371 | + | crate::error_page::internal_error() | |
| 367 | 372 | })? | |
| 368 | - | .ok_or_else(|| StatusCode::NOT_FOUND.into_response())?; | |
| 373 | + | .ok_or_else(crate::error_page::not_found)?; | |
| 369 | 374 | ||
| 370 | 375 | let activity = mt_db::queries::get_user_activity_in_community( | |
| 371 | 376 | &state.db, community.id, profile.user_id, 20, | |
| @@ -373,7 +378,7 @@ pub(in crate::routes) async fn user_profile( | |||
| 373 | 378 | .await | |
| 374 | 379 | .map_err(|e| { | |
| 375 | 380 | tracing::error!(error = ?e, "db error fetching user activity"); | |
| 376 | - | StatusCode::INTERNAL_SERVER_ERROR.into_response() | |
| 381 | + | crate::error_page::internal_error() | |
| 377 | 382 | })?; | |
| 378 | 383 | ||
| 379 | 384 | let activity_rows = activity | |
| @@ -427,7 +432,7 @@ pub(in crate::routes) async fn user_summary_api( | |||
| 427 | 432 | .await | |
| 428 | 433 | .map_err(|e| { | |
| 429 | 434 | tracing::error!(error = ?e, "db error fetching membership summary"); | |
| 430 | - | StatusCode::INTERNAL_SERVER_ERROR.into_response() | |
| 435 | + | crate::error_page::internal_error() | |
| 431 | 436 | })?; | |
| 432 | 437 | ||
| 433 | 438 | Ok(Json(serde_json::json!({ "memberships": memberships }))) |
| @@ -96,9 +96,9 @@ pub(crate) async fn get_community( | |||
| 96 | 96 | .await | |
| 97 | 97 | .map_err(|e| { | |
| 98 | 98 | tracing::error!(error = ?e, "db error fetching community"); | |
| 99 | - | (StatusCode::INTERNAL_SERVER_ERROR, "Internal server error").into_response() | |
| 99 | + | crate::error_page::internal_error() | |
| 100 | 100 | })? | |
| 101 | - | .ok_or_else(|| (StatusCode::NOT_FOUND, "Not found").into_response()) | |
| 101 | + | .ok_or_else(crate::error_page::not_found) | |
| 102 | 102 | } | |
| 103 | 103 | ||
| 104 | 104 | /// Fetch thread with breadcrumb, returning 404/500 on failure. | |
| @@ -112,15 +112,15 @@ pub(crate) async fn get_thread( | |||
| 112 | 112 | .await | |
| 113 | 113 | .map_err(|e| { | |
| 114 | 114 | tracing::error!(error = ?e, "db error fetching thread"); | |
| 115 | - | (StatusCode::INTERNAL_SERVER_ERROR, "Internal server error").into_response() | |
| 115 | + | crate::error_page::internal_error() | |
| 116 | 116 | })? | |
| 117 | - | .ok_or_else(|| (StatusCode::NOT_FOUND, "Not found").into_response()) | |
| 117 | + | .ok_or_else(crate::error_page::not_found) | |
| 118 | 118 | } | |
| 119 | 119 | ||
| 120 | 120 | /// Parse a UUID from a string, returning 404 on failure. | |
| 121 | 121 | #[allow(clippy::result_large_err)] | |
| 122 | 122 | pub(crate) fn parse_uuid(id_str: &str) -> Result<Uuid, Response> { | |
| 123 | - | Uuid::parse_str(id_str).map_err(|_| (StatusCode::NOT_FOUND, "Not found").into_response()) | |
| 123 | + | Uuid::parse_str(id_str).map_err(|_| crate::error_page::not_found()) | |
| 124 | 124 | } | |
| 125 | 125 | ||
| 126 | 126 | /// Fetch a user's role in a community, returning 500 on DB error. | |
| @@ -134,7 +134,7 @@ pub(crate) async fn get_role( | |||
| 134 | 134 | .await | |
| 135 | 135 | .map_err(|e| { | |
| 136 | 136 | tracing::error!(error = ?e, "db error fetching role"); | |
| 137 | - | (StatusCode::INTERNAL_SERVER_ERROR, "Internal server error").into_response() | |
| 137 | + | crate::error_page::internal_error() | |
| 138 | 138 | }) | |
| 139 | 139 | } | |
| 140 | 140 | ||
| @@ -148,8 +148,10 @@ pub(crate) async fn get_user_by_username( | |||
| 148 | 148 | .await | |
| 149 | 149 | .map_err(|e| { | |
| 150 | 150 | tracing::error!(error = ?e, "db error looking up user"); | |
| 151 | - | (StatusCode::INTERNAL_SERVER_ERROR, "Internal server error").into_response() | |
| 151 | + | crate::error_page::internal_error() | |
| 152 | 152 | })? | |
| 153 | + | // 422 stays a plain form-validation response (consumed inline by mt.js), | |
| 154 | + | // not a full branded page. | |
| 153 | 155 | .ok_or_else(|| (StatusCode::UNPROCESSABLE_ENTITY, "User not found.").into_response()) | |
| 154 | 156 | } | |
| 155 | 157 |
| @@ -101,6 +101,10 @@ pub struct Pagination { | |||
| 101 | 101 | pub total_pages: u32, | |
| 102 | 102 | pub has_prev: bool, | |
| 103 | 103 | pub has_next: bool, | |
| 104 | + | /// Extra query string appended after `?page=N` in the prev/next links | |
| 105 | + | /// (e.g. `&filter=archived`), so a filtered listing stays filtered across | |
| 106 | + | /// pages. Empty by default; set with [`Pagination::with_query_suffix`]. | |
| 107 | + | pub query_suffix: String, | |
| 104 | 108 | } | |
| 105 | 109 | ||
| 106 | 110 | impl Pagination { | |
| @@ -113,9 +117,18 @@ impl Pagination { | |||
| 113 | 117 | total_pages, | |
| 114 | 118 | has_prev: current_page > 1, | |
| 115 | 119 | has_next: current_page < total_pages, | |
| 120 | + | query_suffix: String::new(), | |
| 116 | 121 | } | |
| 117 | 122 | } | |
| 118 | 123 | ||
| 124 | + | /// Set the query-string suffix preserved across page links. The argument is | |
| 125 | + | /// the part after `?page=N` and must include its leading `&` (e.g. | |
| 126 | + | /// `&filter=archived`). | |
| 127 | + | pub fn with_query_suffix(mut self, suffix: impl Into<String>) -> Self { | |
| 128 | + | self.query_suffix = suffix.into(); | |
| 129 | + | self | |
| 130 | + | } | |
| 131 | + | ||
| 119 | 132 | /// SQL OFFSET for the current page. `(current_page - 1) * per_page`, | |
| 120 | 133 | /// saturating so a clamped current_page can never wrap. | |
| 121 | 134 | pub fn offset(&self, per_page: i64) -> i64 { | |
| @@ -228,6 +241,7 @@ mod pagination_tests { | |||
| 228 | 241 | total_pages: 1, | |
| 229 | 242 | has_prev: false, | |
| 230 | 243 | has_next: false, | |
| 244 | + | query_suffix: String::new(), | |
| 231 | 245 | }; | |
| 232 | 246 | assert_eq!(p.offset(25), 0); | |
| 233 | 247 | } |
| @@ -1,11 +1,11 @@ | |||
| 1 | 1 | {% if pagination.total_pages > 1 %} | |
| 2 | 2 | <nav class="pagination" aria-label="Page navigation"> | |
| 3 | 3 | {% if pagination.has_prev %} | |
| 4 | - | <a href="?page={{ pagination.current_page - 1 }}" class="pagination-link">Previous</a> | |
| 4 | + | <a href="?page={{ pagination.current_page - 1 }}{{ pagination.query_suffix }}" class="pagination-link">Previous</a> | |
| 5 | 5 | {% endif %} | |
| 6 | 6 | <span class="pagination-info">Page {{ pagination.current_page }} of {{ pagination.total_pages }}</span> | |
| 7 | 7 | {% if pagination.has_next %} | |
| 8 | - | <a href="?page={{ pagination.current_page + 1 }}" class="pagination-link">Next</a> | |
| 8 | + | <a href="?page={{ pagination.current_page + 1 }}{{ pagination.query_suffix }}" class="pagination-link">Next</a> | |
| 9 | 9 | {% endif %} | |
| 10 | 10 | </nav> | |
| 11 | 11 | {% endif %} |
| @@ -65,6 +65,8 @@ impl TestHarness { | |||
| 65 | 65 | trusted_proxies: std::sync::Arc::from([std::net::IpAddr::from([127, 0, 0, 1])]), | |
| 66 | 66 | }; | |
| 67 | 67 | ||
| 68 | + | multithreaded::error_page::init(config.mnw_base_url.clone()); | |
| 69 | + | ||
| 68 | 70 | let state = AppState { | |
| 69 | 71 | db: pool.clone(), | |
| 70 | 72 | config, |
| @@ -268,6 +268,58 @@ async fn archived_excluded_from_default_listing() { | |||
| 268 | 268 | ); | |
| 269 | 269 | } | |
| 270 | 270 | ||
| 271 | + | #[tokio::test] | |
| 272 | + | async fn archived_directory_pagination_preserves_filter() { | |
| 273 | + | // With >1 page of archived communities, the next/prev links must keep | |
| 274 | + | // ?filter=archived, else page 2 reverts to the default listing (audit U2). | |
| 275 | + | let mut h = TestHarness::new().await; | |
| 276 | + | // 26 archived communities → 2 pages at 25/page. | |
| 277 | + | for i in 0..26 { | |
| 278 | + | let id = h.create_community(&format!("Arch {i}"), &format!("arch-{i}")).await; | |
| 279 | + | sqlx::query("UPDATE communities SET state = 'archived' WHERE id = $1") | |
| 280 | + | .bind(id) | |
| 281 | + | .execute(&h.db) | |
| 282 | + | .await | |
| 283 | + | .unwrap(); | |
| 284 | + | } | |
| 285 | + | ||
| 286 | + | let resp = h.client.get("/?filter=archived").await; | |
| 287 | + | assert_eq!(resp.status.as_u16(), 200); | |
| 288 | + | // The Next link must carry the filter forward. Askama HTML-escapes the `&` | |
| 289 | + | // to the numeric entity `&`, which the browser decodes back to `&`. | |
| 290 | + | assert!( | |
| 291 | + | resp.text.contains("?page=2&filter=archived"), | |
| 292 | + | "archived pagination Next link must preserve filter; got body:\n{}", | |
| 293 | + | resp.text | |
| 294 | + | ); | |
| 295 | + | ||
| 296 | + | // Page 2 of the archived view still shows archived communities (filter held). | |
| 297 | + | let resp2 = h.client.get("/?page=2&filter=archived").await; | |
| 298 | + | assert_eq!(resp2.status.as_u16(), 200); | |
| 299 | + | assert!( | |
| 300 | + | resp2.text.contains("?page=1&filter=archived"), | |
| 301 | + | "Previous link on page 2 must preserve filter" | |
| 302 | + | ); | |
| 303 | + | } | |
| 304 | + | ||
| 305 | + | #[tokio::test] | |
| 306 | + | async fn missing_community_page_renders_branded_404() { | |
| 307 | + | // An in-handler 404 (missing community) must render the branded error page | |
| 308 | + | // with site chrome, not bare "Not found" plaintext (audit U1). | |
| 309 | + | let mut h = TestHarness::new().await; | |
| 310 | + | let resp = h.client.get("/p/no-such-community").await; | |
| 311 | + | assert_eq!(resp.status.as_u16(), 404); | |
| 312 | + | assert!( | |
| 313 | + | resp.text.contains("The page you're looking for doesn't exist."), | |
| 314 | + | "should render the branded 404 body" | |
| 315 | + | ); | |
| 316 | + | assert!( | |
| 317 | + | resp.text.contains("Back to Forums"), | |
| 318 | + | "branded 404 includes the styled call-to-action" | |
| 319 | + | ); | |
| 320 | + | assert_ne!(resp.text.trim(), "Not found", "must not be bare plaintext"); | |
| 321 | + | } | |
| 322 | + | ||
| 271 | 323 | // ── State-change route ── | |
| 272 | 324 | ||
| 273 | 325 | #[tokio::test] |