Skip to content

Commit

Permalink
fix: handle cross-origin redirects in server function redirect hook (#…
Browse files Browse the repository at this point in the history
…2329)

In client-side navigation we now handle redirects returned from
server functions by resolving the location against the current
origin as a base. The base is only relevant if the location
doesn't already include an origin. This fixes cross-origin
redirects.

Note: in order to handle redirects in the same way as the browser
would handle them, we need to use the server function's URL
(typically `<origin>/api/something`) as a base. I leave this as
a TODO for a future leptos version, because it probably
requires changing the signature of the `server_fn` redirect hook.

In order to not be affected by a future breaking change, users
should already start making sure that their redirect locations
either include an origin or at least start with a single slash
(e.g. `Location: /foo`).
  • Loading branch information
haslersn authored Feb 17, 2024
1 parent 1e000af commit 001ca51
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 22 deletions.
16 changes: 10 additions & 6 deletions router/src/components/form.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::{
hooks::has_router, use_navigate, use_resolved_path, NavigateOptions,
ToHref, Url,
hooks::has_router, resolve_redirect_url, use_navigate, use_resolved_path,
NavigateOptions, ToHref, Url,
};
use leptos::{
html::form,
Expand Down Expand Up @@ -447,8 +447,10 @@ where
{
let has_router = has_router();
if !has_router {
_ = server_fn::redirect::set_redirect_hook(|path: &str| {
_ = window().location().set_href(path);
_ = server_fn::redirect::set_redirect_hook(|loc: &str| {
if let Some(url) = resolve_redirect_url(loc) {
_ = window().location().set_href(&url.href());
}
});
}
let action_url = if let Some(url) = action.url() {
Expand Down Expand Up @@ -545,8 +547,10 @@ where
{
let has_router = has_router();
if !has_router {
_ = server_fn::redirect::set_redirect_hook(|path: &str| {
_ = window().location().set_href(path);
_ = server_fn::redirect::set_redirect_hook(|loc: &str| {
if let Some(url) = resolve_redirect_url(loc) {
_ = window().location().set_href(&url.href());
}
});
}
let action_url = if let Some(url) = action.url() {
Expand Down
32 changes: 21 additions & 11 deletions router/src/components/router.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::{
create_location, matching::resolve_path, scroll_to_el, use_location,
use_navigate, Branch, History, Location, LocationChange, RouteContext,
RouterIntegrationContext, State,
create_location, matching::resolve_path, resolve_redirect_url,
scroll_to_el, use_location, use_navigate, Branch, History, Location,
LocationChange, RouteContext, RouterIntegrationContext, State,
};
#[cfg(not(feature = "ssr"))]
use crate::{unescape, Url};
Expand All @@ -24,6 +24,7 @@ use std::{
use thiserror::Error;
#[cfg(not(feature = "ssr"))]
use wasm_bindgen::JsCast;
use wasm_bindgen::UnwrapThrowExt;

static GLOBAL_ROUTERS_COUNT: AtomicUsize = AtomicUsize::new(0);

Expand Down Expand Up @@ -56,15 +57,24 @@ pub fn Router(
// set server function redirect hook
let navigate = use_navigate();
let navigate = SendWrapper::new(navigate);
let router_hook = Box::new(move |path: &str| {
let path = path.to_string();
// delay by a tick here, so that the Action updates *before* the redirect
request_animation_frame({
let router_hook = Box::new(move |loc: &str| {
let Some(url) = resolve_redirect_url(loc) else {
return; // resolve_redirect_url() already logs an error
};
let current_origin =
leptos_dom::helpers::location().origin().unwrap_throw();
if url.origin() == current_origin {
let navigate = navigate.clone();
move || {
navigate(&path, Default::default());
}
});
// delay by a tick here, so that the Action updates *before* the redirect
request_animation_frame(move || {
navigate(&url.pathname(), Default::default());
});
// Use set_href() if the conditions for client-side navigation were not satisfied
} else if let Err(e) =
leptos_dom::helpers::location().set_href(&url.href())
{
leptos::logging::error!("Failed to redirect: {e:#?}");
}
}) as RedirectHook;
_ = server_fn::redirect::set_redirect_hook(router_hook);

Expand Down
29 changes: 27 additions & 2 deletions router/src/hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ use crate::{
RouterContext,
};
use leptos::{
create_memo, request_animation_frame, signal_prelude::*, use_context, Memo,
Oco,
create_memo, request_animation_frame, signal_prelude::*, use_context,
window, Memo, Oco,
};
use std::{rc::Rc, str::FromStr};

Expand Down Expand Up @@ -216,3 +216,28 @@ pub(crate) fn use_is_back_navigation() -> ReadSignal<bool> {
let router = use_router();
router.inner.is_back.read_only()
}

/// Resolves a redirect location to an (absolute) URL.
pub(crate) fn resolve_redirect_url(loc: &str) -> Option<web_sys::Url> {
let origin = match window().location().origin() {
Ok(origin) => origin,
Err(e) => {
leptos::logging::error!("Failed to get origin: {:#?}", e);
return None;
}
};

// TODO: Use server function's URL as base instead.
let base = origin;

match web_sys::Url::new_with_base(loc, &base) {
Ok(url) => Some(url),
Err(e) => {
leptos::logging::error!(
"Invalid redirect location: {}",
e.as_string().unwrap_or_default(),
);
None
}
}
}
6 changes: 3 additions & 3 deletions server_fn/src/redirect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ pub fn set_redirect_hook(
REDIRECT_HOOK.set(Box::new(hook))
}

/// Calls the hook that has been set by [`set_redirect_hook`] to redirect to `path`.
pub fn call_redirect_hook(path: &str) {
/// Calls the hook that has been set by [`set_redirect_hook`] to redirect to `loc`.
pub fn call_redirect_hook(loc: &str) {
if let Some(hook) = REDIRECT_HOOK.get() {
hook(path)
hook(loc)
}
}

0 comments on commit 001ca51

Please sign in to comment.