max / makenotwork
- Co-Authored-By
- Claude Opus 5 (1M context) <noreply@anthropic.com>
8 files changed,
+229 insertions,
-46 deletions
| @@ -33,6 +33,7 @@ | |||
| 33 | 33 | pub mod extractors; | |
| 34 | 34 | pub mod fee_calculator; | |
| 35 | 35 | pub mod formatting; | |
| 36 | + | pub mod fragment_redirect; | |
| 36 | 37 | pub mod git; | |
| 37 | 38 | pub mod git_ssh; | |
| 38 | 39 | pub mod helpers; | |
| @@ -587,31 +588,37 @@ | |||
| 587 | 588 | // process lifetime (Run #14 CHRONIC 1). Guarded by `Once` internally. | |
| 588 | 589 | crate::rate_limit::start_governor_sweeper(); | |
| 589 | 590 | ||
| 590 | - | app.layer(middleware::from_fn(request_timeout_middleware)) | |
| 591 | - | .layer(middleware::from_fn_with_state( | |
| 592 | - | state.clone(), | |
| 593 | - | access_gate::access_gate_middleware, | |
| 594 | - | )) | |
| 595 | - | .layer(middleware::from_fn_with_state( | |
| 596 | - | state.clone(), | |
| 597 | - | security_headers_middleware, | |
| 598 | - | )) | |
| 599 | - | .layer(middleware::from_fn(metrics::cache_control_middleware)) | |
| 600 | - | .layer(middleware::from_fn(metrics::metrics_middleware)) | |
| 601 | - | .layer(middleware::from_fn_with_state( | |
| 602 | - | state.clone(), | |
| 603 | - | metrics::idempotency_middleware, | |
| 604 | - | )) | |
| 605 | - | .layer(session_layer) | |
| 606 | - | .layer(RequestBodyLimitLayer::new(1024 * 1024)) | |
| 607 | - | // Outermost: requests to the user-pages host (`u.makenot.work`) are | |
| 608 | - | // served custom pages here and short-circuit before the session and | |
| 609 | - | // access-gate layers, so that host stays cookieless and ungated. | |
| 610 | - | // Everything else (and `/static`) falls through to the normal app. | |
| 611 | - | .layer(middleware::from_fn_with_state( | |
| 612 | - | state.clone(), | |
| 613 | - | routes::user_pages::dispatch, | |
| 614 | - | )) | |
| 591 | + | // Innermost of this chain, so it runs closest to the routes: a direct | |
| 592 | + | // navigation to a fragment endpoint is redirected after the session and | |
| 593 | + | // access-gate layers have done their work, not instead of them. | |
| 594 | + | app.layer(middleware::from_fn( | |
| 595 | + | fragment_redirect::fragment_redirect_middleware, | |
| 596 | + | )) | |
| 597 | + | .layer(middleware::from_fn(request_timeout_middleware)) | |
| 598 | + | .layer(middleware::from_fn_with_state( | |
| 599 | + | state.clone(), | |
| 600 | + | access_gate::access_gate_middleware, | |
| 601 | + | )) | |
| 602 | + | .layer(middleware::from_fn_with_state( | |
| 603 | + | state.clone(), | |
| 604 | + | security_headers_middleware, | |
| 605 | + | )) | |
| 606 | + | .layer(middleware::from_fn(metrics::cache_control_middleware)) | |
| 607 | + | .layer(middleware::from_fn(metrics::metrics_middleware)) | |
| 608 | + | .layer(middleware::from_fn_with_state( | |
| 609 | + | state.clone(), | |
| 610 | + | metrics::idempotency_middleware, | |
| 611 | + | )) | |
| 612 | + | .layer(session_layer) | |
| 613 | + | .layer(RequestBodyLimitLayer::new(1024 * 1024)) | |
| 614 | + | // Outermost: requests to the user-pages host (`u.makenot.work`) are | |
| 615 | + | // served custom pages here and short-circuit before the session and | |
| 616 | + | // access-gate layers, so that host stays cookieless and ungated. | |
| 617 | + | // Everything else (and `/static`) falls through to the normal app. | |
| 618 | + | .layer(middleware::from_fn_with_state( | |
| 619 | + | state.clone(), | |
| 620 | + | routes::user_pages::dispatch, | |
| 621 | + | )) | |
| 615 | 622 | } | |
| 616 | 623 | ||
| 617 | 624 | /// Wall-clock ceiling on response generation. Generous, every normal page/API |
| @@ -30,6 +30,17 @@ | |||
| 30 | 30 | self.client.get(format!("{BASE_URL}{path}")).send().await | |
| 31 | 31 | } | |
| 32 | 32 | ||
| 33 | + | /// GET with the `HX-Request` header HTMX sets on every request it makes. | |
| 34 | + | /// Fragment endpoints redirect a plain GET to their parent page, so a test | |
| 35 | + | /// asserting on the partial has to ask for it the way the page does. | |
| 36 | + | async fn htmx_get(&self, path: &str) -> reqwest::Result<reqwest::Response> { | |
| 37 | + | self.client | |
| 38 | + | .get(format!("{BASE_URL}{path}")) | |
| 39 | + | .header("HX-Request", "true") | |
| 40 | + | .send() | |
| 41 | + | .await | |
| 42 | + | } | |
| 43 | + | ||
| 33 | 44 | async fn post_form( | |
| 34 | 45 | &self, | |
| 35 | 46 | path: &str, | |
| @@ -542,7 +553,7 @@ | |||
| 542 | 553 | #[tokio::test] | |
| 543 | 554 | async fn test_discover_results_partial() { | |
| 544 | 555 | let client = TestClient::new(); | |
| 545 | - | let resp = client.get("/discover/results").await; | |
| 556 | + | let resp = client.htmx_get("/discover/results").await; | |
| 546 | 557 | match resp { | |
| 547 | 558 | Ok(r) => { | |
| 548 | 559 | assert_eq!( |
| @@ -363,7 +363,7 @@ | |||
| 363 | 363 | ||
| 364 | 364 | // /discover/results returns the inner partial used by HTMX filter swaps. | |
| 365 | 365 | // It must NOT include the full page chrome (header, footer, <html>). | |
| 366 | - | let resp = h.client.get("/discover/results?mode=items").await; | |
| 366 | + | let resp = h.client.htmx_get("/discover/results?mode=items").await; | |
| 367 | 367 | assert!( | |
| 368 | 368 | resp.status.is_success(), | |
| 369 | 369 | "GET /discover/results: {}", | |
| @@ -763,7 +763,7 @@ | |||
| 763 | 763 | #[tokio::test] | |
| 764 | 764 | async fn results_partial_refreshes_the_total_count() { | |
| 765 | 765 | let mut h = TestHarness::new().await; | |
| 766 | - | let resp = h.client.get("/discover/results?mode=items").await; | |
| 766 | + | let resp = h.client.htmx_get("/discover/results?mode=items").await; | |
| 767 | 767 | assert!(resp.status.is_success(), "{}", resp.status); | |
| 768 | 768 | assert!( | |
| 769 | 769 | resp.text.contains(r#"id="total-count" hx-swap-oob="true""#), | |
| @@ -785,7 +785,10 @@ | |||
| 785 | 785 | ||
| 786 | 786 | for query in ["mode=items", "mode=items&q=guitar", "mode=projects"] { | |
| 787 | 787 | let page = h.client.get(&format!("/discover?{query}")).await; | |
| 788 | - | let partial = h.client.get(&format!("/discover/results?{query}")).await; | |
| 788 | + | let partial = h | |
| 789 | + | .client | |
| 790 | + | .htmx_get(&format!("/discover/results?{query}")) | |
| 791 | + | .await; | |
| 789 | 792 | assert!(page.status.is_success(), "{}", page.status); | |
| 790 | 793 | assert!(partial.status.is_success(), "{}", partial.status); | |
| 791 | 794 | ||
| @@ -1087,7 +1090,7 @@ | |||
| 1087 | 1090 | let mut h = TestHarness::new().await; | |
| 1088 | 1091 | make_discoverable_item(&mut h, "oobsidebar", "OOB Item", "audio").await; | |
| 1089 | 1092 | ||
| 1090 | - | let resp = h.client.get("/discover/results?mode=items").await; | |
| 1093 | + | let resp = h.client.htmx_get("/discover/results?mode=items").await; | |
| 1091 | 1094 | assert!(resp.status.is_success(), "{}", resp.status); | |
| 1092 | 1095 | assert!( | |
| 1093 | 1096 | resp.text | |
| @@ -1108,7 +1111,7 @@ | |||
| 1108 | 1111 | make_discoverable_item(&mut h, "oobvideo", "A Video", "video").await; | |
| 1109 | 1112 | ||
| 1110 | 1113 | // Unfiltered: both types present, so the audio checkbox is not checked. | |
| 1111 | - | let all = h.client.get("/discover/results?mode=items").await; | |
| 1114 | + | let all = h.client.htmx_get("/discover/results?mode=items").await; | |
| 1112 | 1115 | assert!( | |
| 1113 | 1116 | all.text.contains(r#"id="typesel-audio""#), | |
| 1114 | 1117 | "audio facet should render" | |
| @@ -1121,7 +1124,7 @@ | |||
| 1121 | 1124 | // Filtered to audio: the audio control comes back checked in the OOB payload. | |
| 1122 | 1125 | let audio = h | |
| 1123 | 1126 | .client | |
| 1124 | - | .get("/discover/results?mode=items&item_type=audio") | |
| 1127 | + | .htmx_get("/discover/results?mode=items&item_type=audio") | |
| 1125 | 1128 | .await; | |
| 1126 | 1129 | assert!( | |
| 1127 | 1130 | audio.text.contains("checked"), |
| @@ -673,7 +673,7 @@ | |||
| 673 | 673 | .await; | |
| 674 | 674 | seed_active_fan_plus(&h, user_id, "sub_pane_1").await; | |
| 675 | 675 | ||
| 676 | - | let resp = h.client.get("/dashboard/tabs/account").await; | |
| 676 | + | let resp = h.client.htmx_get("/dashboard/tabs/account").await; | |
| 677 | 677 | assert_eq!(resp.status, 200); | |
| 678 | 678 | assert!(resp.text.contains("Fan+ membership")); | |
| 679 | 679 | assert!(resp.text.contains("Cancel")); | |
| @@ -697,7 +697,7 @@ | |||
| 697 | 697 | .await | |
| 698 | 698 | .unwrap(); | |
| 699 | 699 | ||
| 700 | - | let resp = h.client.get("/dashboard/tabs/account").await; | |
| 700 | + | let resp = h.client.htmx_get("/dashboard/tabs/account").await; | |
| 701 | 701 | assert_eq!(resp.status, 200); | |
| 702 | 702 | assert!(resp.text.contains("Cancellation scheduled")); | |
| 703 | 703 | assert!(resp.text.contains("Resume")); | |
| @@ -709,7 +709,7 @@ | |||
| 709 | 709 | h.signup("notsub", "notsub@example.com", "password123") | |
| 710 | 710 | .await; | |
| 711 | 711 | ||
| 712 | - | let resp = h.client.get("/dashboard/tabs/account").await; | |
| 712 | + | let resp = h.client.htmx_get("/dashboard/tabs/account").await; | |
| 713 | 713 | assert_eq!(resp.status, 200); | |
| 714 | 714 | assert!(resp.text.contains("Learn about Fan+")); | |
| 715 | 715 | assert!(!resp.text.contains("Manage billing")); |
| @@ -209,7 +209,10 @@ | |||
| 209 | 209 | h.signup("feedpage", "feedpage@test.com", "password123") | |
| 210 | 210 | .await; | |
| 211 | 211 | ||
| 212 | - | let resp = h.client.get("/library/tabs/feed?page=4000000000").await; | |
| 212 | + | let resp = h | |
| 213 | + | .client | |
| 214 | + | .htmx_get("/library/tabs/feed?page=4000000000") | |
| 215 | + | .await; | |
| 213 | 216 | assert_eq!( | |
| 214 | 217 | resp.status, 200, | |
| 215 | 218 | "large page should clamp to a valid empty page, got {} {}", |
| @@ -68,23 +68,70 @@ | |||
| 68 | 68 | } | |
| 69 | 69 | } | |
| 70 | 70 | ||
| 71 | + | /// A tab URL typed, bookmarked or crawled is a navigation, not an hx-get, and | |
| 72 | + | /// serving the bare partial for it showed chromeless HTML to a human and let a | |
| 73 | + | /// crawler index a fragment as a page. It redirects to the page the fragment | |
| 74 | + | /// belongs to instead. Replaces the older assertion that a plain GET returned | |
| 75 | + | /// 200 with the partial, which is the behaviour that was wrong. | |
| 71 | 76 | #[tokio::test] | |
| 72 | - | async fn dashboard_tab_without_htmx_returns_full_page_or_error() { | |
| 77 | + | async fn dashboard_tab_without_htmx_redirects_to_the_dashboard() { | |
| 73 | 78 | let mut h = TestHarness::new().await; | |
| 74 | 79 | let _user_id = h | |
| 75 | 80 | .signup("nohtmx", "nohtmx@example.com", "password123") | |
| 76 | 81 | .await; | |
| 77 | 82 | ||
| 78 | - | // Regular GET (no HX-Request header) to a tab route | |
| 79 | - | // The tab handlers are plain GET routes that return template partials. | |
| 80 | - | // Without HTMX, they still return 200 with the partial. This is expected | |
| 81 | - | // since the tab routes don't check is_htmx_request themselves. | |
| 82 | 83 | let resp = h.client.get("/dashboard/tabs/profile").await; | |
| 83 | 84 | assert_eq!( | |
| 84 | - | resp.status, 200, | |
| 85 | - | "Tab route should still respond to regular GET, got {}", | |
| 85 | + | resp.status, 302, | |
| 86 | + | "plain GET of a tab should redirect, got {}", | |
| 86 | 87 | resp.status | |
| 87 | 88 | ); | |
| 89 | + | assert_eq!( | |
| 90 | + | resp.headers | |
| 91 | + | .get("location") | |
| 92 | + | .and_then(|v| v.to_str().ok()) | |
| 93 | + | .unwrap_or_default(), | |
| 94 | + | "/dashboard" | |
| 95 | + | ); | |
| 96 | + | } | |
| 97 | + | ||
| 98 | + | /// The other three fragment families redirect the same way. `/discover/results` | |
| 99 | + | /// and `/pricing/compare` are public, so they need no session to check. | |
| 100 | + | #[tokio::test] | |
| 101 | + | async fn public_fragments_without_htmx_redirect_to_their_page() { | |
| 102 | + | let mut h = TestHarness::new().await; | |
| 103 | + | ||
| 104 | + | for (fragment, parent) in [ | |
| 105 | + | ("/discover/results?mode=items", "/discover"), | |
| 106 | + | ("/pricing/compare", "/pricing"), | |
| 107 | + | ] { | |
| 108 | + | let resp = h.client.get(fragment).await; | |
| 109 | + | assert_eq!(resp.status, 302, "{fragment} should redirect"); | |
| 110 | + | assert_eq!( | |
| 111 | + | resp.headers | |
| 112 | + | .get("location") | |
| 113 | + | .and_then(|v| v.to_str().ok()) | |
| 114 | + | .unwrap_or_default(), | |
| 115 | + | parent, | |
| 116 | + | "{fragment} redirected somewhere unexpected" | |
| 117 | + | ); | |
| 118 | + | } | |
| 119 | + | } | |
| 120 | + | ||
| 121 | + | /// The redirect is GET-only. A POST to a fragment endpoint carries a write the | |
| 122 | + | /// caller is waiting on, and redirecting it would swallow the submission. | |
| 123 | + | /// Asserted against a route that exists rather than a hypothetical one: the | |
| 124 | + | /// check here is only that the middleware did not turn it into a 302. | |
| 125 | + | #[tokio::test] | |
| 126 | + | async fn fragment_redirect_does_not_touch_post() { | |
| 127 | + | let mut h = TestHarness::new().await; | |
| 128 | + | h.client.fetch_csrf_token().await; | |
| 129 | + | ||
| 130 | + | let resp = h.client.post_form("/discover/results", "mode=items").await; | |
| 131 | + | assert_ne!( | |
| 132 | + | resp.status, 302, | |
| 133 | + | "POST to a fragment path must not be redirected by the fragment guard" | |
| 134 | + | ); | |
| 88 | 135 | } | |
| 89 | 136 | ||
| 90 | 137 | #[tokio::test] |
| @@ -255,13 +255,13 @@ | |||
| 255 | 255 | async fn discover_htmx_partial() { | |
| 256 | 256 | let mut h = TestHarness::new().await; | |
| 257 | 257 | ||
| 258 | - | let resp = h.client.get("/discover/results").await; | |
| 258 | + | let resp = h.client.htmx_get("/discover/results").await; | |
| 259 | 259 | assert_eq!( | |
| 260 | 260 | resp.status, 200, | |
| 261 | 261 | "Discover results partial should return 200" | |
| 262 | 262 | ); | |
| 263 | 263 | ||
| 264 | - | let resp = h.client.get("/discover/results?mode=projects").await; | |
| 264 | + | let resp = h.client.htmx_get("/discover/results?mode=projects").await; | |
| 265 | 265 | assert_eq!( | |
| 266 | 266 | resp.status, 200, | |
| 267 | 267 | "Discover results projects mode should return 200" | |
| @@ -269,7 +269,7 @@ | |||
| 269 | 269 | ||
| 270 | 270 | let resp = h | |
| 271 | 271 | .client | |
| 272 | - | .get("/discover/results?item_type=audio&sort=newest") | |
| 272 | + | .htmx_get("/discover/results?item_type=audio&sort=newest") | |
| 273 | 273 | .await; | |
| 274 | 274 | assert_eq!( | |
| 275 | 275 | resp.status, 200, |
| @@ -1,0 +1,112 @@ | |||
| 1 | + | //! Direct navigation to an HTMX fragment endpoint redirects to its page. | |
| 2 | + | //! | |
| 3 | + | //! Several routes exist only to answer an `hx-get` and return a bare partial: | |
| 4 | + | //! no `<html>`, no header, no footer. Reached directly (a stale bookmark, a | |
| 5 | + | //! shared link, a crawler) they served that partial as if it were a page, so a | |
| 6 | + | //! human saw chromeless HTML and a crawler indexed a fragment under a URL that | |
| 7 | + | //! is not a page. | |
| 8 | + | //! | |
| 9 | + | //! The fix branches on `HX-Request`, which HTMX sets on every request it makes: | |
| 10 | + | //! present means the fragment was asked for by the page that knows what to do | |
| 11 | + | //! with it, absent means somebody navigated. Absent gets a 302 to the parent. | |
| 12 | + | //! | |
| 13 | + | //! One table rather than a branch per handler. The route list grows (there are | |
| 14 | + | //! 20 tab endpoints across two families already), and a check copied into each | |
| 15 | + | //! handler is one somebody forgets to copy. | |
| 16 | + | ||
| 17 | + | use axum::{ | |
| 18 | + | extract::Request, | |
| 19 | + | http::{Method, StatusCode, header::LOCATION}, | |
| 20 | + | middleware::Next, | |
| 21 | + | response::{IntoResponse, Response}, | |
| 22 | + | }; | |
| 23 | + | ||
| 24 | + | /// Fragment endpoints and the page each one belongs to. | |
| 25 | + | /// | |
| 26 | + | /// A key ending in `/` matches by prefix (a family of endpoints under it); any | |
| 27 | + | /// other key matches the exact path. Add a route here when you add a handler | |
| 28 | + | /// that returns a partial. | |
| 29 | + | const FRAGMENT_PARENTS: &[(&str, &str)] = &[ | |
| 30 | + | ("/dashboard/tabs/", "/dashboard"), | |
| 31 | + | ("/library/tabs/", "/library"), | |
| 32 | + | ("/discover/results", "/discover"), | |
| 33 | + | ("/pricing/compare", "/pricing"), | |
| 34 | + | ]; | |
| 35 | + | ||
| 36 | + | /// The page a fragment path belongs to, if it is a fragment path. | |
| 37 | + | pub fn parent_page(path: &str) -> Option<&'static str> { | |
| 38 | + | FRAGMENT_PARENTS.iter().find_map(|&(key, parent)| { | |
| 39 | + | let matched = if key.ends_with('/') { | |
| 40 | + | path.starts_with(key) | |
| 41 | + | } else { | |
| 42 | + | path == key | |
| 43 | + | }; | |
| 44 | + | matched.then_some(parent) | |
| 45 | + | }) | |
| 46 | + | } | |
| 47 | + | ||
| 48 | + | /// Redirect a non-HTMX GET of a fragment endpoint to its parent page. | |
| 49 | + | /// | |
| 50 | + | /// GET only. A POST to a fragment endpoint is a form submission whose response | |
| 51 | + | /// the caller is waiting on, and redirecting it would swallow the write. | |
| 52 | + | pub async fn fragment_redirect_middleware(request: Request, next: Next) -> Response { | |
| 53 | + | if request.method() == Method::GET | |
| 54 | + | && !crate::helpers::is_htmx_request(request.headers()) | |
| 55 | + | && let Some(parent) = parent_page(request.uri().path()) | |
| 56 | + | { | |
| 57 | + | return (StatusCode::FOUND, [(LOCATION, parent)]).into_response(); | |
| 58 | + | } | |
| 59 | + | next.run(request).await | |
| 60 | + | } | |
| 61 | + | ||
| 62 | + | #[cfg(test)] | |
| 63 | + | mod tests { | |
| 64 | + | use super::parent_page; | |
| 65 | + | ||
| 66 | + | #[test] | |
| 67 | + | fn tab_families_match_by_prefix() { | |
| 68 | + | assert_eq!(parent_page("/dashboard/tabs/details"), Some("/dashboard")); | |
| 69 | + | assert_eq!( | |
| 70 | + | parent_page("/dashboard/tabs/payout-summary"), | |
| 71 | + | Some("/dashboard") | |
| 72 | + | ); | |
| 73 | + | assert_eq!(parent_page("/library/tabs/purchases"), Some("/library")); | |
| 74 | + | } | |
| 75 | + | ||
| 76 | + | #[test] | |
| 77 | + | fn single_endpoints_match_exactly() { | |
| 78 | + | assert_eq!(parent_page("/discover/results"), Some("/discover")); | |
| 79 | + | assert_eq!(parent_page("/pricing/compare"), Some("/pricing")); | |
| 80 | + | } | |
| 81 | + | ||
| 82 | + | /// The parents themselves are real pages and must fall through. | |
| 83 | + | #[test] | |
| 84 | + | fn parent_pages_are_not_fragments() { | |
| 85 | + | for page in ["/dashboard", "/library", "/discover", "/pricing"] { | |
| 86 | + | assert_eq!(parent_page(page), None, "{page} redirected to itself"); | |
| 87 | + | } | |
| 88 | + | } | |
| 89 | + | ||
| 90 | + | /// An exact key must not match by prefix, or `/pricing/compare-plans` | |
| 91 | + | /// would redirect the day someone adds it. | |
| 92 | + | #[test] | |
| 93 | + | fn exact_keys_do_not_match_by_prefix() { | |
| 94 | + | assert_eq!(parent_page("/pricing/compare-plans"), None); | |
| 95 | + | assert_eq!(parent_page("/discover/results-archive"), None); | |
| 96 | + | } | |
| 97 | + | ||
| 98 | + | /// `/library/tabs` without the trailing slash is not a fragment route, and | |
| 99 | + | /// the prefix keys must not swallow it. | |
| 100 | + | #[test] | |
| 101 | + | fn bare_family_path_is_not_a_fragment() { | |
| 102 | + | assert_eq!(parent_page("/library/tabs"), None); | |
| 103 | + | assert_eq!(parent_page("/dashboard/tabs"), None); | |
| 104 | + | } | |
| 105 | + | ||
| 106 | + | #[test] | |
| 107 | + | fn unrelated_paths_fall_through() { | |
| 108 | + | for page in ["/", "/docs/video", "/static/style.css", "/u/max"] { | |
| 109 | + | assert_eq!(parent_page(page), None); | |
| 110 | + | } | |
| 111 | + | } | |
| 112 | + | } |