ITADN

redirect() panics on a redirect path that isn't a valid header value (axum + actix integrations)

#4749OpenAlexeyMatskevich 创建于 2026-05-30
Hi, and thank you for Leptos and the official server integrations — they've been a genuine pleasure to build on. 🙏 While porting the axum/actix adapters to another backend I came across a small robustness wrinkle in the server-side `redirect()` helper, and wanted to flag it gently in case it's useful. It's minor, and I might be missing context — happy to be told it's intended. ## What I noticed Both integration `redirect()` helpers build the `Location` header with `.expect(...)`: - `integrations/axum/src/lib.rs` — `redirect()` (~L227), the `.expect` at ~L234-235 - `integrations/actix/src/lib.rs` — `redirect()` (~L229), the `.expect` at ~L236-237 ```rust res.insert_header( header::LOCATION, header::HeaderValue::from_str(path) .expect("Failed to create HeaderValue"), ); ``` `HeaderValue::from_str` returns `Err` for bytes that aren't legal in a header value (CR `\r`, LF `\n`, NUL, other control bytes). So when `path` happens to contain one of those, the `.expect()` panics and aborts the request rather than degrading. Since `redirect()` is wired up as the server redirect hook via `provide_server_redirect(redirect)`, `<Redirect path=.../>` and server-function redirects flow through it — and the target is often assembled from request-influenced input (e.g. `redirect(&format!("/u/{username}"))`, or a `?next=…` login redirect), so a crafted value can reach this line. To be clear, I don't think this is a header-injection concern — the `from_str` rejection is exactly what *prevents* injection. The only wrinkle is that the rejection currently turns into a panic instead of a graceful no-op. ## The graceful form is already your convention one layer down The same logical redirect in `server_fn` (same repo) handles this softly in every backend: - `server_fn/src/response/http.rs` (~L70-75) - `server_fn/src/response/actix.rs` (~L89-94) - `server_fn/src/response/generic.rs` (~L110-115) ```rust fn redirect(&mut self, path: &str) { if let Ok(path) = HeaderValue::from_str(path) { self.headers_mut().insert(header::LOCATION, path); *self.status_mut() = StatusCode::FOUND; } } ``` So the `if let Ok` form already seems to be the established convention here — it's just the integration-layer helper that still uses `.expect()`. (For what it's worth, #514 *"remove `.unwrap()` from `redirect` in Actix integration"* softened the neighbouring `use_context(...).unwrap()` but happened to leave this particular `from_str(...).expect()` line — very easy to miss.) ## A possible minimal fix Set the header only when `from_str` succeeds, mirroring the `server_fn` impls — roughly: ```rust if let Ok(location) = HeaderValue::from_str(path) { res.insert_header(header::LOCATION, location); // …existing accepts_html / REDIRECT_HEADER branch unchanged… } else { // optional: warn via tracing / eprintln, then return } ``` I'd be glad to open a small PR for both integrations if that'd be welcome — just let me know what you'd prefer. (Line numbers are from `main` as of today and may drift.) Thanks again for all the work on this project! 🙂
3 条评论