redirect() panics on a redirect path that isn't a valid header value (axum + actix integrations)
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 条评论