max / quasi
- Co-Authored-By
- Claude Opus 5 (1M context) <noreply@anthropic.com>
- Claude-Session
- https://claude.ai/code/session_0136sbU8F6i9WrcvA3wn4Lgk
7 files changed,
+219 insertions,
-23 deletions
| @@ -6233,4 +6233,4 @@ | |||
| 6233 | 6233 | ||
| 6234 | 6234 | [[patch.unused]] | |
| 6235 | 6235 | name = "quasi-type" | |
| 6236 | - | version = "0.1.2" | |
| 6236 | + | version = "0.1.3" |
| @@ -407,8 +407,14 @@ | |||
| 407 | 407 | /// cannot be built either, since the one thing a per-request renderer is handed | |
| 408 | 408 | /// is the state that could not be resolved. | |
| 409 | 409 | fn unresolved(error: &RouteError) -> http::Response<Vec<u8>> { | |
| 410 | - | http::Response::builder() | |
| 411 | - | .status(error.class.http_status()) | |
| 410 | + | let mut builder = http::Response::builder().status(error.class.http_status()); | |
| 411 | + | // Bodyless still owes `Allow` if the class is the one that claims verbs. | |
| 412 | + | // A state factory cannot produce that class today, and hard-coding the | |
| 413 | + | // assumption here is how it would stop being true silently. | |
| 414 | + | if let Some(allow) = error.allow_header() { | |
| 415 | + | builder = builder.header(http::header::ALLOW, allow); | |
| 416 | + | } | |
| 417 | + | builder | |
| 412 | 418 | .body(Vec::new()) | |
| 413 | 419 | .expect("a response with no body and no headers is always valid") | |
| 414 | 420 | } |
| @@ -212,6 +212,36 @@ | |||
| 212 | 212 | ); | |
| 213 | 213 | } | |
| 214 | 214 | ||
| 215 | + | /// `1e35bc8a`, and the case the PATCH test above does not cover: a verb the | |
| 216 | + | /// description layer *has*, at a path that does not register it. This answered | |
| 217 | + | /// 404 until 2026-08-29, which told a crawler that a page being served at that | |
| 218 | + | /// URL was gone. | |
| 219 | + | #[tokio::test] | |
| 220 | + | async fn a_known_verb_a_described_path_does_not_hold_is_a_405_naming_what_it_does() { | |
| 221 | + | let request = Request::builder() | |
| 222 | + | .method("DELETE") | |
| 223 | + | .uri("/task/7") | |
| 224 | + | .body(Body::empty()) | |
| 225 | + | .unwrap(); | |
| 226 | + | let response = service().oneshot(request).await.unwrap(); | |
| 227 | + | ||
| 228 | + | assert_eq!(response.status(), StatusCode::METHOD_NOT_ALLOWED); | |
| 229 | + | // Only the verbs registered at this path, which is the difference from the | |
| 230 | + | // layer-level refusal above: that one lists all four because the question | |
| 231 | + | // never reached the table. | |
| 232 | + | assert_eq!(response.headers().get(header::ALLOW).unwrap(), "GET"); | |
| 233 | + | } | |
| 234 | + | ||
| 235 | + | /// And an address that is genuinely absent still says so, with nothing to | |
| 236 | + | /// suggest. A 404 carrying an `Allow` would be describing a page that is not | |
| 237 | + | /// there. | |
| 238 | + | #[tokio::test] | |
| 239 | + | async fn an_absent_address_is_still_a_404_and_offers_no_allow() { | |
| 240 | + | let response = service().oneshot(get("/nowhere")).await.unwrap(); | |
| 241 | + | assert_eq!(response.status(), StatusCode::NOT_FOUND); | |
| 242 | + | assert!(response.headers().get(header::ALLOW).is_none()); | |
| 243 | + | } | |
| 244 | + | ||
| 215 | 245 | #[tokio::test] | |
| 216 | 246 | async fn a_panicking_handler_is_a_500_and_reads_as_ours() { | |
| 217 | 247 | let (status, body, _) = send(get("/boom")).await; |
| @@ -370,12 +370,22 @@ | |||
| 370 | 370 | tone: error.tone(), | |
| 371 | 371 | text: error.message.clone(), | |
| 372 | 372 | }; | |
| 373 | - | body( | |
| 373 | + | let mut response = body( | |
| 374 | 374 | render, | |
| 375 | 375 | error.class.http_status(), | |
| 376 | 376 | render.fragment(&node), | |
| 377 | 377 | None, | |
| 378 | - | ) | |
| 378 | + | ); | |
| 379 | + | // A 405 without `Allow` is a refusal with no way to learn what | |
| 380 | + | // would have worked, which RFC 9110 makes a MUST for exactly that | |
| 381 | + | // reason. Empty for every other class, so nothing is added to the | |
| 382 | + | // answers that were already right. | |
| 383 | + | if let Some(allow) = error.allow_header() | |
| 384 | + | && let Ok(value) = http::HeaderValue::from_str(&allow) | |
| 385 | + | { | |
| 386 | + | response.headers_mut().insert(http::header::ALLOW, value); | |
| 387 | + | } | |
| 388 | + | response | |
| 379 | 389 | } | |
| 380 | 390 | } | |
| 381 | 391 | } |
| @@ -16,13 +16,17 @@ | |||
| 16 | 16 | ||
| 17 | 17 | use makeover_layout::{Notice, Tone}; | |
| 18 | 18 | ||
| 19 | + | use crate::request::Method; | |
| 20 | + | ||
| 19 | 21 | /// What kind of failure it was. | |
| 20 | 22 | /// | |
| 21 | - | /// Four, and each one is a different thing for a host to do. `NotFound` and | |
| 23 | + | /// Five, and each one is a different thing for a host to do. `NotFound` and | |
| 22 | 24 | /// `Denied` are separate because a host that conflates them cannot answer the | |
| 23 | 25 | /// question its own access log asks. `Conflict` is separate because it is the | |
| 24 | 26 | /// one failure the user can fix by looking at the screen again, which is a | |
| 25 | - | /// banner rather than a toast. | |
| 27 | + | /// banner rather than a toast. `Unsupported` is separate because the address | |
| 28 | + | /// is real and only the verb is wrong, which is a different sentence and a | |
| 29 | + | /// different status code. | |
| 26 | 30 | /// | |
| 27 | 31 | /// `#[non_exhaustive]` for the reason `makeover-layout` puts it on the | |
| 28 | 32 | /// vocabularies renderers match against: growth must not be a lockstep event | |
| @@ -36,6 +40,14 @@ | |||
| 36 | 40 | Denied, | |
| 37 | 41 | /// The request was answerable and the current state refuses it. | |
| 38 | 42 | Conflict, | |
| 43 | + | /// The address is there and does not take this verb. | |
| 44 | + | /// | |
| 45 | + | /// `1e35bc8a`, measured against MNW's `/pricing`. Distinct from | |
| 46 | + | /// [`NotFound`](Self::NotFound), which says the address does not exist: | |
| 47 | + | /// answering that for a page a host serves at that URL tells a crawler the | |
| 48 | + | /// page is gone. What does not exist is the verb, and the verbs that do is | |
| 49 | + | /// information the caller can act on -- see [`RouteError::allow`]. | |
| 50 | + | Unsupported, | |
| 39 | 51 | /// We are broken. | |
| 40 | 52 | Internal, | |
| 41 | 53 | } | |
| @@ -49,7 +61,7 @@ | |||
| 49 | 61 | #[must_use] | |
| 50 | 62 | pub const fn tone(self) -> Tone { | |
| 51 | 63 | match self { | |
| 52 | - | Self::NotFound | Self::Denied | Self::Conflict => Tone::Warning, | |
| 64 | + | Self::NotFound | Self::Denied | Self::Conflict | Self::Unsupported => Tone::Warning, | |
| 53 | 65 | Self::Internal => Tone::Danger, | |
| 54 | 66 | } | |
| 55 | 67 | } | |
| @@ -65,6 +77,7 @@ | |||
| 65 | 77 | match self { | |
| 66 | 78 | Self::NotFound => 404, | |
| 67 | 79 | Self::Denied => 403, | |
| 80 | + | Self::Unsupported => 405, | |
| 68 | 81 | Self::Conflict => 409, | |
| 69 | 82 | Self::Internal => 500, | |
| 70 | 83 | } | |
| @@ -95,6 +108,18 @@ | |||
| 95 | 108 | pub notice: Notice, | |
| 96 | 109 | /// What to tell the user. Already user-facing text, not a debug string. | |
| 97 | 110 | pub message: String, | |
| 111 | + | /// The verbs the address does take, for a | |
| 112 | + | /// [`Class::Unsupported`](Class::Unsupported). | |
| 113 | + | /// | |
| 114 | + | /// Empty for every other class, and a caller should read it as "no claim | |
| 115 | + | /// is being made" rather than "no verbs". It rides on the error rather | |
| 116 | + | /// than being asked of the router afterwards because the two adapters that | |
| 117 | + | /// need it are not both holding one: `quasi_http::respond` is handed a | |
| 118 | + | /// `Result` and no table. | |
| 119 | + | /// | |
| 120 | + | /// Sorted and deduplicated by [`unsupported`](Self::unsupported), so | |
| 121 | + | /// `Allow` reads the same however the routes were registered. | |
| 122 | + | pub allow: Vec<Method>, | |
| 98 | 123 | } | |
| 99 | 124 | ||
| 100 | 125 | impl RouteError { | |
| @@ -104,9 +129,48 @@ | |||
| 104 | 129 | class, | |
| 105 | 130 | notice: Notice::Banner, | |
| 106 | 131 | message: message.into(), | |
| 132 | + | allow: Vec::new(), | |
| 107 | 133 | } | |
| 108 | 134 | } | |
| 109 | 135 | ||
| 136 | + | /// The address is there and does not take this verb. | |
| 137 | + | /// | |
| 138 | + | /// `allow` is what it does take, and is what an HTTP host puts in the | |
| 139 | + | /// header of the same name. Sorted and deduplicated here so that the | |
| 140 | + | /// answer does not depend on registration order. | |
| 141 | + | pub fn unsupported( | |
| 142 | + | message: impl Into<String>, | |
| 143 | + | allow: impl IntoIterator<Item = Method>, | |
| 144 | + | ) -> Self { | |
| 145 | + | let mut allow: Vec<Method> = allow.into_iter().collect(); | |
| 146 | + | allow.sort_unstable(); | |
| 147 | + | allow.dedup(); | |
| 148 | + | Self { | |
| 149 | + | allow, | |
| 150 | + | ..Self::new(Class::Unsupported, message) | |
| 151 | + | } | |
| 152 | + | } | |
| 153 | + | ||
| 154 | + | /// The `Allow` header's value, or `None` when nothing is being claimed. | |
| 155 | + | /// | |
| 156 | + | /// Here rather than in each adapter because both HTTP hosts want the same | |
| 157 | + | /// string, and because the comma-and-space spelling is the one RFC 9110 | |
| 158 | + | /// gives. A host with no notion of a header ignores it and reads | |
| 159 | + | /// [`allow`](Self::allow). | |
| 160 | + | #[must_use] | |
| 161 | + | pub fn allow_header(&self) -> Option<String> { | |
| 162 | + | if self.allow.is_empty() { | |
| 163 | + | return None; | |
| 164 | + | } | |
| 165 | + | Some( | |
| 166 | + | self.allow | |
| 167 | + | .iter() | |
| 168 | + | .map(ToString::to_string) | |
| 169 | + | .collect::<Vec<_>>() | |
| 170 | + | .join(", "), | |
| 171 | + | ) | |
| 172 | + | } | |
| 173 | + | ||
| 110 | 174 | /// The thing addressed is not there. | |
| 111 | 175 | pub fn not_found(message: impl Into<String>) -> Self { | |
| 112 | 176 | Self::new(Class::NotFound, message) |
| @@ -253,10 +253,69 @@ | |||
| 253 | 253 | let missing = router | |
| 254 | 254 | .handle(&state(), Request::post("/task/7")) | |
| 255 | 255 | .unwrap_err(); | |
| 256 | - | assert_eq!(missing.class, Class::NotFound); | |
| 256 | + | // `1e35bc8a`: not a `NotFound`, which it was until 2026-08-29. The | |
| 257 | + | // address is there and the verb is not, and saying the address is gone | |
| 258 | + | // is how a crawler drops a page a host is serving. | |
| 259 | + | assert_eq!(missing.class, Class::Unsupported); | |
| 260 | + | assert_eq!(missing.class.http_status(), 405); | |
| 257 | 261 | assert!(missing.message.contains("another method")); | |
| 258 | 262 | } | |
| 259 | 263 | ||
| 264 | + | #[test] | |
| 265 | + | fn a_verb_that_misses_names_the_verbs_that_would_not_have() { | |
| 266 | + | let refused = Router::<State>::new() | |
| 267 | + | .get("/pricing", home) | |
| 268 | + | .post("/pricing", home) | |
| 269 | + | .handle( | |
| 270 | + | &state(), | |
| 271 | + | Request { | |
| 272 | + | method: Method::Delete, | |
| 273 | + | ..Request::get("/pricing") | |
| 274 | + | }, | |
| 275 | + | ) | |
| 276 | + | .unwrap_err(); | |
| 277 | + | ||
| 278 | + | assert_eq!(refused.class, Class::Unsupported); | |
| 279 | + | assert_eq!(refused.allow, vec![Method::Get, Method::Post]); | |
| 280 | + | // The spelling an HTTP host puts in the header, built here so that two | |
| 281 | + | // adapters cannot disagree about the separator. | |
| 282 | + | assert_eq!(refused.allow_header().as_deref(), Some("GET, POST")); | |
| 283 | + | } | |
| 284 | + | ||
| 285 | + | #[test] | |
| 286 | + | fn the_allow_list_does_not_depend_on_the_order_routes_were_registered() { | |
| 287 | + | let late = Router::<State>::new() | |
| 288 | + | .post("/pricing", home) | |
| 289 | + | .get("/pricing", home) | |
| 290 | + | .handle( | |
| 291 | + | &state(), | |
| 292 | + | Request { | |
| 293 | + | method: Method::Delete, | |
| 294 | + | ..Request::get("/pricing") | |
| 295 | + | }, | |
| 296 | + | ) | |
| 297 | + | .unwrap_err(); | |
| 298 | + | assert_eq!(late.allow_header().as_deref(), Some("GET, POST")); | |
| 299 | + | } | |
| 300 | + | ||
| 301 | + | #[test] | |
| 302 | + | fn an_address_that_is_simply_absent_claims_no_verbs() { | |
| 303 | + | let missing = router() | |
| 304 | + | .handle(&state(), Request::get("/nowhere")) | |
| 305 | + | .unwrap_err(); | |
| 306 | + | // Empty is "no claim", and an adapter reads it as "send no `Allow`". | |
| 307 | + | // A 404 that named verbs would be describing a page that is not there. | |
| 308 | + | assert!(missing.allow.is_empty()); | |
| 309 | + | assert!(missing.allow_header().is_none()); | |
| 310 | + | } | |
| 311 | + | ||
| 312 | + | #[test] | |
| 313 | + | fn the_verbs_at_an_address_can_be_asked_for_directly() { | |
| 314 | + | assert_eq!(router().verbs_at("/task/new"), vec![Method::Get]); | |
| 315 | + | assert_eq!(router().verbs_at("/task/7/delete"), vec![Method::Post]); | |
| 316 | + | assert!(router().verbs_at("/nowhere").is_empty()); | |
| 317 | + | } | |
| 318 | + | ||
| 260 | 319 | #[test] | |
| 261 | 320 | fn an_unknown_path_says_so_without_mentioning_a_method() { | |
| 262 | 321 | let missing = router() |
| @@ -123,14 +123,19 @@ | |||
| 123 | 123 | /// here, into a third bag, because a capture is the route's own and was not | |
| 124 | 124 | /// sent by anybody. | |
| 125 | 125 | pub fn handle(&self, state: &S, request: Request) -> Result<Response, RouteError> { | |
| 126 | - | let mut wrong_method = false; | |
| 126 | + | // The two lookups are separate questions and have separate answers: a | |
| 127 | + | // path that matches nothing is a `NotFound`, and a path that matches | |
| 128 | + | // under other verbs is an `Unsupported` naming them. `1e35bc8a` is what | |
| 129 | + | // forced the split -- MNW's `/pricing` answered 404 to `DELETE` once it | |
| 130 | + | // became a described route, which tells a crawler the page is gone. | |
| 131 | + | let mut allowed: Vec<Method> = Vec::new(); | |
| 127 | 132 | ||
| 128 | 133 | for route in &self.routes { | |
| 129 | 134 | let Some(captures) = route.pattern.match_path(&request.path) else { | |
| 130 | 135 | continue; | |
| 131 | 136 | }; | |
| 132 | 137 | if route.method != request.method { | |
| 133 | - | wrong_method = true; | |
| 138 | + | allowed.push(route.method); | |
| 134 | 139 | continue; | |
| 135 | 140 | } | |
| 136 | 141 | ||
| @@ -145,21 +150,43 @@ | |||
| 145 | 150 | ||
| 146 | 151 | let (method, path) = (request.method, &request.path); | |
| 147 | 152 | ||
| 148 | - | // A path that exists under another verb is still a `NotFound` rather | |
| 149 | - | // than an `Internal`, even though reaching it means our own renderer | |
| 150 | - | // emitted the wrong verb. The reason is what a host does with the | |
| 151 | - | // class: an HTTP adapter answering 500 to a probe turns a scan into a | |
| 152 | - | // page, and the message carries the detail an operator needs anyway. | |
| 153 | - | Err(RouteError::new( | |
| 154 | - | Class::NotFound, | |
| 155 | - | if wrong_method { | |
| 156 | - | format!("no route for {method} {path}, though the path answers another method") | |
| 157 | - | } else { | |
| 158 | - | format!("no route for {method} {path}") | |
| 159 | - | }, | |
| 153 | + | // Not an `Internal`, even though reaching either of these means our own | |
| 154 | + | // renderer emitted an address or a verb nothing serves. The reason is | |
| 155 | + | // what a host does with the class: an HTTP adapter answering 500 to a | |
| 156 | + | // probe turns a scan into a page, and the message carries the detail an | |
| 157 | + | // operator needs anyway. | |
| 158 | + | if allowed.is_empty() { | |
| 159 | + | return Err(RouteError::new( | |
| 160 | + | Class::NotFound, | |
| 161 | + | format!("no route for {method} {path}"), | |
| 162 | + | )); | |
| 163 | + | } | |
| 164 | + | Err(RouteError::unsupported( | |
| 165 | + | format!("no route for {method} {path}, though the path answers another method"), | |
| 166 | + | allowed, | |
| 160 | 167 | )) | |
| 161 | 168 | } | |
| 162 | 169 | ||
| 170 | + | /// The verbs registered at `path`, sorted, or empty if nothing is. | |
| 171 | + | /// | |
| 172 | + | /// The same question [`handle`](Self::handle) answers on the way to an | |
| 173 | + | /// [`Unsupported`](Class::Unsupported), asked directly. For a scaffolder | |
| 174 | + | /// emitting a client, and for a host that wants to answer `OPTIONS` -- | |
| 175 | + | /// which this crate does not do for it, because `OPTIONS` is an HTTP verb | |
| 176 | + | /// and the router has no opinion about HTTP. | |
| 177 | + | #[must_use] | |
| 178 | + | pub fn verbs_at(&self, path: &str) -> Vec<Method> { | |
| 179 | + | let mut verbs: Vec<Method> = self | |
| 180 | + | .routes | |
| 181 | + | .iter() | |
| 182 | + | .filter(|route| route.pattern.match_path(path).is_some()) | |
| 183 | + | .map(|route| route.method) | |
| 184 | + | .collect(); | |
| 185 | + | verbs.sort_unstable(); | |
| 186 | + | verbs.dedup(); | |
| 187 | + | verbs | |
| 188 | + | } | |
| 189 | + | ||
| 163 | 190 | /// Every registered route, most specific first. | |
| 164 | 191 | /// | |
| 165 | 192 | /// For a scaffolder generating a client, a test asserting the table, and an |