From 69b3c9ec1b89e0bb1f51fe777b11360655e5d9f3 Mon Sep 17 00:00:00 2001 From: Pietro di Caprio Date: Sun, 30 Aug 2026 04:01:17 +0200 Subject: [PATCH 1/6] fix: enforce multi-user lifecycle boundaries Authorize SSC resume against the retained session before reattachment, bind host component subscriptions to mounted ownership, and add declarative route guard outcomes for redirect, replace, and forbidden navigation.\n\nPreserve the legacy resume, host registration, and bool guard APIs while adding migration guidance and native/WASM regressions for cross-user resume, route teardown, late delivery, guard ordering, and browser history. --- CHANGELOG.md | 64 +++ core/hosthook.go | 23 +- core/hosthook_test.go | 217 +++++++- core/html_component.go | 75 ++- docs/articles/guide/routing.md | 50 ++ docs/articles/guide/ssc-security.md | 45 +- docs/articles/guide/ssc.md | 27 + host/resume_authorization_test.go | 293 ++++++++++ host/session.go | 72 ++- host/ssc_protocol_test.go | 96 ++++ host/ws_runtime.go | 62 ++- hostclient/component_binding_wasm_test.go | 543 +++++++++++++++++++ hostclient/coreinit.go | 2 +- hostclient/host_signal_delivery_wasm_test.go | 198 +++++++ hostclient/runtime_foundation.go | 416 ++++++++++++-- router/data.go | 141 +++++ router/router.go | 44 +- router/router_guard_result_test.go | 288 ++++++++++ router/router_history_test.go | 125 +++++ router/router_stub.go | 41 +- ssc/server_test.go | 92 ++++ state/signal.go | 73 ++- 22 files changed, 2857 insertions(+), 130 deletions(-) create mode 100644 host/resume_authorization_test.go create mode 100644 hostclient/component_binding_wasm_test.go create mode 100644 hostclient/host_signal_delivery_wasm_test.go create mode 100644 router/router_guard_result_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e1d787a..600ab9a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,32 @@ version and include migration notes. ## [Unreleased] +### Added + +- `host.WithSSCResumeAuthorizer(func(*http.Request, *host.Session) error)`, run + before a retained session is reattached. An authenticated deployment decides + whether the upgrade request may resume the session its token names; refusing + serves the caller a new session and leaves the retained one untouched. Left + unset, resume keeps its token-only behavior. +- `router.ResultGuard` and `Route.ResultGuards`, guards that return + `router.Allow()`, `router.Forbid()`, `router.RedirectTo(path)` or + `router.ReplaceWith(path)` instead of a bool. A login redirect is now + declarative and replaces the history entry rather than pushing one, so back + does not return to the denied page. `Route.Guards`, `router.Page` and the + `Group` builder are unchanged; a route may declare both, and its bool guards + run first. New errors: `router.ErrNavigationForbidden` and + `router.ErrInvalidGuardResult`. +- `core.SetHostRegistrar`, the cleanup-capable form of `core.SetHostRegister`. + Both remain; the legacy hook registers without a release. +- `hostclient.RegisterComponentOwned`, the cleanup-capable form of + `hostclient.RegisterComponent`. It returns an idempotent release that owns the + binding's lifecycle; `RegisterComponent` keeps its signature and its + register-until-replaced behavior. +- `state.Signal.SetFromHostGated` and `state.Signal.HostWriteBarrier`, the pair + a host delivery uses to make its ownership check and its write one step + against a concurrent revocation. `SetFromHost` is unchanged and is the ungated + form of the same write. + ### Changed - WebAssembly applications can opt into structured `rfw:render-trace` browser @@ -95,6 +121,44 @@ version and include migration notes. ### Fixed +- release a host component binding when its client component unmounts. + `AddHostComponent` registered the binding on every render and never undid it, + so leaving a route kept the `hostclient` binding, its queued reconnect init + and the server subscription alive, and a late push still reached the + unmounted component root. Registration now happens once per mounted + lifecycle, its cleanup is owned by the component, and a remount registers + again; a component that is rendered but never mounted binds nothing, so a + preparatory render cannot leave a binding no unmount would release. No + published signature changes: `hostclient.RegisterComponent` keeps + its own, and the cleanup comes from the new `RegisterComponentOwned`, which + `hostclient` installs into `core.SetHostRegistrar`. +- deliver host frames only to the binding that is still registered, and only to + the root it owns. Delivery snapshotted a binding, released the lock and then + wrote to the DOM, so a frame that overlapped a route exit could still update a + released component; it also resolved its target through `dom.ComponentRoot`, + whose fallback would have written into `#app` once the component's own root + left the document. Delivery is now serialized against registration and + release, revalidates the registration it was snapshotted for, and ignores a + frame whose `[data-component-id]` root is absent. `RegisterComponent` takes + any id, so the id is escaped through `CSS.escape` before it reaches a + selector, and compared attribute by attribute where that API is missing. +- stop a host frame from writing the signals of a released component. Delivery + re-checked the registration and then ran the setters with no lock held, so a + cleanup that returned in between still saw a stale frame set a host signal and + schedule the effects that follow one after the unmount. A registration now + hands every frame it carries a gate the signal evaluates under the same lock + it stores the value with, and the cleanup closes that gate before waiting out + the writes it had already allowed: once the cleanup returns, neither the root + it owned nor its host signals take another frame. No lock covers application + code, so a setter that unmounts or remounts its own component from inside the + frame cannot deadlock against its own registration. +- keep the disconnected message queue bounded across offline route churn. Every + mount and unmount appended its own reconnect control, so a route entered and + left 100 times offline retained 100 unsubscribes. The queue now holds one + control per host component, the latest desired state: a registration replaces + a queued unsubscribe and a release replaces a queued init, in the position the + superseded control held. Other messages, and the controls of other components, + keep their order. - isolate SSC broadcasts behind bounded per-connection outbound queues and a write deadline. A slow socket no longer stalls later broadcast targets; a full queue requests resynchronization and closes that connection so retained diff --git a/core/hosthook.go b/core/hosthook.go index 62583571..e64dfe42 100644 --- a/core/hosthook.go +++ b/core/hosthook.go @@ -2,13 +2,34 @@ package core +// HostRegistrar registers a component with the SSC runtime and returns an +// idempotent cleanup that releases the registration. A nil cleanup means the +// registration owns nothing to release. +type HostRegistrar func(id, name string, vars []string) func() + // hostRegisterComponent registers a component with the SSC runtime. It stays // nil until the hostclient package is linked, so a client-only build never // reaches the websocket and net/http stacks through core. -var hostRegisterComponent func(id, name string, vars []string) +var hostRegisterComponent HostRegistrar // SetHostRegister installs the SSC component registration hook. The hostclient // package calls it from its own init; applications never call it directly. +// Registrations made through it are never released, so prefer +// SetHostRegistrar when the runtime can unbind a component. func SetHostRegister(fn func(id, name string, vars []string)) { + if fn == nil { + hostRegisterComponent = nil + return + } + hostRegisterComponent = func(id, name string, vars []string) func() { + fn(id, name, vars) + return nil + } +} + +// SetHostRegistrar installs a cleanup-capable SSC registration hook, so a +// component that unmounts can release its binding, pending initialization and +// server subscription. +func SetHostRegistrar(fn HostRegistrar) { hostRegisterComponent = fn } diff --git a/core/hosthook_test.go b/core/hosthook_test.go index db5945da..f61d447b 100644 --- a/core/hosthook_test.go +++ b/core/hosthook_test.go @@ -4,27 +4,34 @@ package core import "testing" -// Rendering a component that declares host components goes through the hook -// the hostclient package installs. Without hostclient linked the hook is nil -// and rendering must still succeed, which is what keeps client-only builds -// free of the websocket and net/http stacks. -func TestHostRegisterHook(t *testing.T) { +// hostCall is one registration the hook observed. +type hostCall struct { + id, name string + vars []string +} + +// recordHostCalls installs the legacy hook and collects what it receives. +func recordHostCalls(t *testing.T, calls *[]hostCall) { + t.Helper() prev := hostRegisterComponent t.Cleanup(func() { hostRegisterComponent = prev }) - - type call struct { - id, name string - vars []string - } - var calls []call SetHostRegister(func(id, name string, vars []string) { - calls = append(calls, call{id: id, name: name, vars: vars}) + *calls = append(*calls, hostCall{id: id, name: name, vars: vars}) }) +} + +// Mounting a component that declares host components goes through the hook the +// hostclient package installs, once per declared name and in declaration order. +func TestHostRegisterHook(t *testing.T) { + var calls []hostCall + recordHostCalls(t, &calls) c := NewHTMLComponent("HookHost", []byte(``), nil) c.AddHostComponent("Counter") c.AddHostComponent("Clock") + c.Init(nil) c.Render() + c.Mount() if len(calls) != 2 { t.Fatalf("expected 2 host registrations, got %d: %+v", len(calls), calls) @@ -37,7 +44,70 @@ func TestHostRegisterHook(t *testing.T) { } } -// A nil hook is the client-only case: rendering must not panic. +// Ownership is mounted-bound: a component that is only rendered registers +// nothing, because no Unmount would ever come to release it. +func TestHostRegisterSkipsRenderOnlyComponent(t *testing.T) { + var calls []hostCall + recordHostCalls(t, &calls) + + c := NewHTMLComponent("RenderOnlyHost", []byte(`{h:value}`), nil) + c.AddHostComponent("Counter") + c.Init(nil) + for i := 0; i < 5; i++ { + c.RenderFresh() + } + c.Unmount() + + if len(calls) != 0 { + t.Fatalf("a render-only component registered %d host bindings: %+v", len(calls), calls) + } +} + +// Render before mount is the framework's own order: the render discovers the +// host variables and the mount binds once with them. +func TestHostRegisterAfterRenderThenMount(t *testing.T) { + var calls []hostCall + recordHostCalls(t, &calls) + + c := NewHTMLComponent("RenderThenMountHost", []byte(`{h:value}`), nil) + c.AddHostComponent("Counter") + c.Init(nil) + c.Render() + c.Mount() + c.Render() + + if len(calls) != 1 { + t.Fatalf("host registrations = %d, want 1: %+v", len(calls), calls) + } + if len(calls[0].vars) != 1 || calls[0].vars[0] != "value" { + t.Fatalf("registered vars = %v, want the ones the render discovered", calls[0].vars) + } +} + +// Mount before render binds at mount time, with whatever the template has +// discovered so far, and the render that follows adds no second registration. +func TestHostRegisterAfterMountThenRender(t *testing.T) { + var calls []hostCall + recordHostCalls(t, &calls) + + c := NewHTMLComponent("MountThenRenderHost", []byte(`{h:value}`), nil) + c.AddHostComponent("Counter") + c.Init(nil) + c.Mount() + if len(calls) != 1 { + t.Fatalf("mount registrations = %d, want 1: %+v", len(calls), calls) + } + if len(calls[0].vars) != 0 { + t.Fatalf("registered vars = %v, want none discovered yet", calls[0].vars) + } + c.Render() + c.RenderFresh() + if len(calls) != 1 { + t.Fatalf("host registrations = %d after rendering, want 1: %+v", len(calls), calls) + } +} + +// A nil hook is the client-only case: the lifecycle must not panic. func TestHostRegisterHookNil(t *testing.T) { prev := hostRegisterComponent t.Cleanup(func() { hostRegisterComponent = prev }) @@ -45,7 +115,128 @@ func TestHostRegisterHookNil(t *testing.T) { c := NewHTMLComponent("NoHook", []byte(``), nil) c.AddHostComponent("Counter") + c.Init(nil) if got := c.Render(); got == "" { t.Fatal("expected a rendered template with no host hook installed") } + c.Mount() + c.Unmount() +} + +// The legacy hook returns no cleanup. Unmounting a component that used it must +// still complete instead of calling a nil release. +func TestLegacyHostRegisterUnmountsWithoutCleanup(t *testing.T) { + prev := hostRegisterComponent + t.Cleanup(func() { hostRegisterComponent = prev }) + registrations := 0 + SetHostRegister(func(string, string, []string) { registrations++ }) + + c := NewHTMLComponent("LegacyHook", []byte(``), nil) + c.AddHostComponent("Counter") + c.Init(nil) + c.Mount() + c.Render() + c.Unmount() + + if registrations != 1 { + t.Fatalf("host registrations = %d, want 1", registrations) + } +} + +// A host component is bound once per mounted lifecycle: repeated renders reuse +// the live binding instead of re-initializing the SSC feed. +func TestHostRegistrarBindsOncePerRender(t *testing.T) { + prev := hostRegisterComponent + t.Cleanup(func() { hostRegisterComponent = prev }) + registrations := 0 + releases := 0 + SetHostRegistrar(func(string, string, []string) func() { + registrations++ + return func() { releases++ } + }) + + c := NewHTMLComponent("RenderedHost", []byte(``), nil) + c.AddHostComponent("Counter") + c.Init(nil) + c.Mount() + for i := 0; i < 5; i++ { + c.RenderFresh() + } + + if registrations != 1 || releases != 0 { + t.Fatalf("registrations = %d, releases = %d, want 1 and 0", registrations, releases) + } +} + +// A remount whose render comes straight from the template cache still owns its +// bindings: the markup is identical, the previous registration is not. +func TestHostRegistrarRebindsOnCachedRemount(t *testing.T) { + prev := hostRegisterComponent + t.Cleanup(func() { hostRegisterComponent = prev }) + registrations := 0 + releases := 0 + SetHostRegistrar(func(string, string, []string) func() { + registrations++ + return func() { releases++ } + }) + + c := NewHTMLComponent("CachedHost", []byte(`{h:value}`), nil) + c.AddHostComponent("Counter") + c.Init(nil) + c.Mount() + first := c.Render() + c.Unmount() + + if registrations != 1 || releases != 1 { + t.Fatalf("first cycle: registrations = %d, releases = %d, want 1 each", registrations, releases) + } + + c.Mount() + if second := c.Render(); second != first { + t.Fatalf("the remount re-rendered instead of using the cache:\n%q\n%q", second, first) + } + if registrations != 2 || releases != 1 { + t.Fatalf("cached remount: registrations = %d, releases = %d, want 2 and 1", registrations, releases) + } + c.Unmount() + if releases != 2 { + t.Fatalf("releases = %d after the second unmount, want 2", releases) + } +} + +// Route exit must not retain a host binding. Every mount owns its own +// registration and releases exactly that one on unmount, including when the +// render comes from the template cache. +func TestHostRegistrarReleasedOnEveryUnmount(t *testing.T) { + prev := hostRegisterComponent + t.Cleanup(func() { hostRegisterComponent = prev }) + active := map[string]int{} + registrations := 0 + releases := 0 + SetHostRegistrar(func(_, name string, _ []string) func() { + registrations++ + active[name]++ + return func() { + releases++ + active[name]-- + } + }) + + c := NewHTMLComponent("CycledHost", []byte(``), nil) + c.AddHostComponent("Counter") + c.Init(nil) + for cycle := 0; cycle < 100; cycle++ { + c.Mount() + c.Render() + if active["Counter"] != 1 { + t.Fatalf("cycle %d: active bindings = %d, want 1", cycle, active["Counter"]) + } + c.Unmount() + if active["Counter"] != 0 { + t.Fatalf("cycle %d: binding retained after unmount", cycle) + } + } + if registrations != 100 || releases != 100 { + t.Fatalf("registrations = %d, releases = %d, want 100 each", registrations, releases) + } } diff --git a/core/html_component.go b/core/html_component.go index 3dc02d3a..b14d7e50 100644 --- a/core/html_component.go +++ b/core/html_component.go @@ -60,6 +60,7 @@ type HTMLComponent struct { classExprContents map[string]string hostVars []string hostCmds []string + hostRegistrations []hostRegistration component Component mounted bool onMount func(*HTMLComponent) @@ -174,6 +175,11 @@ func (c *HTMLComponent) Render() (renderedTemplate string) { key := c.cacheKey() if c.cache != nil { if val, ok := c.cache[key]; ok { + // A cache hit returns before the rest of the render, so the host + // bindings are registered here too: Mount bound the names declared + // by then, this covers one declared since. Unmounted, it is a + // no-op. + c.registerHostComponents() renderedTemplate = val return } @@ -248,11 +254,7 @@ func (c *HTMLComponent) Render() (renderedTemplate string) { // Handle constructor decorators like [ref] and [key expr] renderedTemplate = replaceConstructors(renderedTemplate) - if hostRegisterComponent != nil { - for _, name := range c.hostComponentNames() { - hostRegisterComponent(c.ID, name, c.hostVars) - } - } + c.registerHostComponents() c.cache[key] = renderedTemplate c.lastCacheKey = key @@ -351,6 +353,9 @@ func (c *HTMLComponent) Unmount() { } dom.UnmountLifecycleHooks(c.ID) c.releaseDOMHooks() + // Before the signals go: releasing a host binding waits out the writes an + // in-flight frame had already been allowed to make on them. + c.releaseHostComponents() if c.scope != nil { c.scope.Close() } @@ -379,6 +384,9 @@ func (c *HTMLComponent) Mount() { } c.registerHandlers() c.registerDOMHooks() + // The mount owns the host bindings: it opens the lifecycle the matching + // Unmount releases, whether or not this component is ever rendered again. + c.registerHostComponents() for _, dep := range c.Dependencies { dependency := dep c.runLifecycle("dependency mount", dependency.Mount) @@ -603,6 +611,63 @@ func (c *HTMLComponent) AddHostComponent(name string) { } } +// hostRegistration keeps the cleanup returned when a host component was bound +// to the SSC runtime for the current mounted lifecycle. +type hostRegistration struct { + name string + release func() +} + +// registerHostComponents binds every declared host component once per mounted +// lifecycle. Ownership is mount-bound: only Mount opens a lifecycle that +// Unmount closes, so a component that is merely rendered (a preparatory or +// discarded render) never registers a binding nothing would ever release. +// Mount binds every name declared by then; renders call it again to cover a +// name declared afterwards, and otherwise find the registration in place and +// leave it alone, so a re-render never re-initializes or unsubscribes a live +// SSC feed. +// +// A registration carries the host variables known when it was made and is never +// remade mid-lifecycle to pick up ones discovered later, so mounting before the +// first render binds with none: the template discovers them while rendering. +// The framework's own paths render first (the router and MountRoot both do), so +// a component mounted through them binds with its variables. +func (c *HTMLComponent) registerHostComponents() { + if !c.mounted || hostRegisterComponent == nil { + return + } + for _, name := range c.hostComponentNames() { + if c.hostRegistered(name) { + continue + } + release := hostRegisterComponent(c.ID, name, c.hostVars) + c.hostRegistrations = append(c.hostRegistrations, hostRegistration{name: name, release: release}) + } +} + +// releaseHostComponents runs the host registration cleanups in reverse order, +// so unmounting drops the bindings and a later mount registers them again. +func (c *HTMLComponent) releaseHostComponents() { + registrations := c.hostRegistrations + c.hostRegistrations = nil + for index := len(registrations) - 1; index >= 0; index-- { + registration := registrations[index] + if registration.release == nil { + continue + } + c.runLifecycle("host unregister: "+registration.name, registration.release) + } +} + +func (c *HTMLComponent) hostRegistered(name string) bool { + for _, registration := range c.hostRegistrations { + if registration.name == name { + return true + } + } + return false +} + // hostComponentNames returns every host component linked to this component, // including a HostComponent assigned directly to the exported field. func (c *HTMLComponent) hostComponentNames() []string { diff --git a/docs/articles/guide/routing.md b/docs/articles/guide/routing.md index 253bac75..d6085772 100644 --- a/docs/articles/guide/routing.md +++ b/docs/articles/guide/routing.md @@ -34,6 +34,56 @@ router.RegisterRoute(router.Route{ }) ``` +## Guards + +`Route.Guards` holds the original guards: a `func(map[string]string) bool` that +allows or blocks. They keep working exactly as before, including +`router.Page(path, component, guards...)` and the `Group` builder. + +`Route.ResultGuards` adds guards that say what should happen instead of the +route they refuse: + +```go +router.RegisterRoute(router.Route{ + Path: "/account", + Component: NewAccountPage, + ResultGuards: []router.ResultGuard{ + func(params map[string]string) router.GuardResult { + switch { + case !session.Authenticated(): + return router.ReplaceWith("/login") + case !session.Can("account:read"): + return router.Forbid() + default: + return router.Allow() + } + }, + }, +}) +``` + +- `router.Allow()` continues to the next guard and then to the route. +- `router.Forbid()` stops navigation with `router.ErrNavigationForbidden`. +- `router.RedirectTo(path)` navigates elsewhere and keeps a history entry for + the current page. +- `router.ReplaceWith(path)` navigates elsewhere in place of the current entry, + which is what a login redirect wants: back must not return to the page the + user was denied. + +Guards run parent before child, and for each route its `Guards` run before its +`ResultGuards`. The first guard that does not allow decides: the guards behind +it, the route loader, the component factory, the unmount of the current page, +the history entry, and the DOM commit are all skipped. A redirect result +navigates to its destination without ever loading the protected route, bounded +by the same redirect-loop limit as `Route.Redirect`. A destination that is +empty or not a rooted path, and an unknown action, fail closed with +`router.ErrInvalidGuardResult`. + +Do not call `router.Navigate` or `router.Replace` inside a guard: it reenters +navigation while one is in flight. Return the destination instead and let the +router apply it once. Outside browser builds the router keeps no history, so +`RedirectTo` and `ReplaceWith` both navigate to the destination. + ## Data loaders A loader completes before the new component is created and the previous page diff --git a/docs/articles/guide/ssc-security.md b/docs/articles/guide/ssc-security.md index 18769980..28defea8 100644 --- a/docs/articles/guide/ssc-security.md +++ b/docs/articles/guide/ssc-security.md @@ -126,7 +126,35 @@ Apply these controls together: }) ``` -3. **Authorize messages and actions.** `host.WithSSCAuthorizer` can reject + The initializer runs for new sessions only. A resumed session keeps the + identity it was created with, which is the point of the next control. +3. **Authorize resume.** A resume token reattaches the session it names. The + upgrade guard proves the new request is authenticated; it does not prove it + is authenticated *as the user the token belongs to*. Without + `host.WithSSCResumeAuthorizer`, any client that obtains a valid token + reattaches that session, private store contents included: + + ```go + host.WithSSCResumeAuthorizer(func(r *http.Request, session *host.Session) error { + user, ok := session.ContextGet("user") + if !ok || user != authenticatedUser(r) || revoked(r) { + return errors.New("resume denied") + } + return nil + }) + ``` + + The callback runs before the session is attached, its retention timer + cleared or its connection replaced. Refusing changes nothing: the caller is + served a new session and receives the `resume_rejected` control frame that + clears its stale token, while the retained session stays detached and + resumable by its owner until the TTL expires. Two upgrades racing on the + same token cannot cross-attach; the loser gets a new session too. An + authenticated multi-user deployment must configure a resume authorizer or + disable resume with `host.WithoutSSCResume()`. The default remains + token-only for compatibility with single-user and unauthenticated + deployments. +4. **Authorize messages and actions.** `host.WithSSCAuthorizer` can reject every decoded message before dispatch. `host.WithActionAuthorizer` applies a typed policy after an action request is decoded. Legacy component handlers should still perform their own object-level checks. @@ -155,8 +183,21 @@ Apply these controls together: `host.WithSessionTarget(sessionID)` for per-user data; broadcasting a payload that contains one user's data sends it to all users on that component. +- **Client-side delivery scope.** A payload is applied only to the root the + binding names (`[data-component-id]`) and to that component's host signals, + and only while that binding is registered. A frame that arrives after its + component unmounted, including one already in flight when the route changed, + is dropped rather than applied to the page shell, to whatever mounted next, or + to a signal the unmounted component still holds. The id is escaped before it + reaches a selector, so an id carrying CSS metacharacters cannot widen the + match to a root the binding does not own. That bounds the damage a stale + broadcast can do to the DOM; it is not an authorization control, since the + browser still received the payload. Scope the data itself with + `host.WithSessionTarget`. - **Transport security.** A resume token can reattach detached session state. - Treat it as a bearer credential, do not log it, and use `wss://`. + Treat it as a bearer credential, do not log it, use `wss://`, and pair it + with `host.WithSSCResumeAuthorizer` so a stolen token is not enough on its + own. `host.Start` serves HTTP and, on the next port, HTTPS with a self-signed certificate generated at boot. That certificate is a development convenience. In production, terminate TLS with real diff --git a/docs/articles/guide/ssc.md b/docs/articles/guide/ssc.md index 0c46bd3c..87d370a8 100644 --- a/docs/articles/guide/ssc.md +++ b/docs/articles/guide/ssc.md @@ -46,6 +46,26 @@ declare server-synced bindings on the struct; every host field name is registered against the server-side component of the same name. HTML components link to their host explicitly with `AddHostComponent(name)`. +A declared host component is bound once per mounted lifecycle, not once per +render: a component that is rendered but never mounted binds nothing, since no +unmount would come to release it. Unmounting the component, which is what +leaving a route does, drops the binding, discards its queued reconnect +initialization and unsubscribes from the host, so the feed stops and a late push +reaches neither the old component root nor its host signals: a frame that was +already in flight when the cleanup ran is discarded rather than applied, and +delivery only ever addresses the exact root the binding owns, whatever +characters its id carries. A host signal setter may unmount or remount its own +component, which releases and registers the binding from inside the frame being +applied; that ends the frame and never deadlocks. Mounting again binds it +again. + +Code that drives the runtime directly keeps both entry points. +`hostclient.RegisterComponent(id, name, vars)` binds until another registration +replaces it, and `hostclient.RegisterComponentOwned` returns the idempotent +cleanup for a component that can unmount. While the client is disconnected the +queue holds only the latest state per host component, so entering and leaving a +route offline does not pile up reconnect messages. + On the wire, `hostclient.Send(name, payload)` delivers a payload to the host component. Repeated identical messages are delivered as-is; call `hostclient.EnableSendDedup(name)` if a channel should drop identical @@ -160,5 +180,12 @@ reports `connecting`, `connected`, `disconnected`, or `desynced`. Use `host.WithoutSSCResume()` when detached session state must be discarded immediately. +A resume token reattaches whatever session it names, so an authenticated +deployment decides who may present it with +`host.WithSSCResumeAuthorizer(func(r *http.Request, session *host.Session) error)`. +The callback runs before the session is touched and a refusal serves the caller +a new session instead, leaving the retained one available to its owner. See +[SSC security](ssc-security.md). + During development `rfw dev` detects `"type": "ssc"` in `rfw.json`, builds the host binary and restarts it on every rebuild. diff --git a/host/resume_authorization_test.go b/host/resume_authorization_test.go new file mode 100644 index 00000000..b2a8b239 --- /dev/null +++ b/host/resume_authorization_test.go @@ -0,0 +1,293 @@ +package host + +import ( + "errors" + "net/http" + "testing" + "time" +) + +const resumeIdentityHeader = "X-Test-User" + +func resumeRequest(t *testing.T, user string) *http.Request { + t.Helper() + request, err := http.NewRequest(http.MethodGet, "/ws", nil) + if err != nil { + t.Fatalf("build request: %v", err) + } + request.Header.Set(resumeIdentityHeader, user) + return request +} + +func bindResumeIdentity(request *http.Request, session *Session) error { + session.ContextSet("user", request.Header.Get(resumeIdentityHeader)) + return nil +} + +func authorizeSameUserResume(request *http.Request, session *Session) error { + user, ok := session.ContextGet("user") + if !ok || user != request.Header.Get(resumeIdentityHeader) { + return errors.New("resume denied") + } + return nil +} + +func resumableRuntime(opts ...MuxOption) *WSRuntime { + base := []MuxOption{WithSSCLimits(SSCLimits{ReplayMessages: 4, ResumeTTL: time.Minute})} + return NewWSRuntime(append(base, opts...)...) +} + +func detachedSession(t *testing.T, runtime *WSRuntime, user string) *Session { + t.Helper() + session, resumed, err := runtime.OpenSession(resumeRequest(t, user), "") + if err != nil { + t.Fatalf("open session: %v", err) + } + if resumed || session.ResumeToken() == "" { + t.Fatalf("expected a new resumable session, resumed=%v token=%q", resumed, session.ResumeToken()) + } + SuspendSession(session, runtime.ResumeTTL()) + return session +} + +func assertDetached(t *testing.T, session *Session) { + t.Helper() + session.deliveryMu.Lock() + attached := session.attached + expires := session.expires + session.deliveryMu.Unlock() + if attached { + t.Fatal("rejected attempt attached the retained session") + } + if expires.IsZero() { + t.Fatal("rejected attempt cleared the retention deadline") + } +} + +// A request authenticated as another user must not reattach a retained +// session, and the refusal must leave that session resumable by its owner. +func TestResumeAuthorizerRejectsAnotherUser(t *testing.T) { + initializations := 0 + runtime := resumableRuntime( + WithSSCSessionInitializer(func(request *http.Request, session *Session) error { + initializations++ + return bindResumeIdentity(request, session) + }), + WithSSCResumeAuthorizer(authorizeSameUserResume), + ) + owner := detachedSession(t, runtime, "alice") + defer ReleaseSession(owner) + token := owner.ResumeToken() + + intruder, resumed, err := runtime.OpenSession(resumeRequest(t, "mallory"), token) + if err != nil { + t.Fatalf("open session for rejected resume: %v", err) + } + defer ReleaseSession(intruder) + if resumed || intruder == owner { + t.Fatalf("foreign request resumed the retained session: resumed=%v", resumed) + } + if user, _ := owner.ContextGet("user"); user != "alice" { + t.Fatalf("retained session identity changed to %v", user) + } + assertDetached(t, owner) + + restored, resumed, err := runtime.OpenSession(resumeRequest(t, "alice"), token) + if err != nil { + t.Fatalf("open session for owner resume: %v", err) + } + if !resumed || restored != owner { + t.Fatalf("owner could not resume after a rejected attempt: resumed=%v", resumed) + } + if user, _ := restored.ContextGet("user"); user != "alice" { + t.Fatalf("resume rebound the session identity to %v", user) + } + // The two new sessions were initialized; the resume was not. + if initializations != 2 { + t.Fatalf("session initializations = %d, want 2", initializations) + } +} + +// Identity alone is not enough: an authorizer that consults revocation state +// keeps a revoked credential from reattaching its own session. +func TestResumeAuthorizerRejectsRevokedIdentity(t *testing.T) { + revoked := map[string]bool{"alice": true} + runtime := resumableRuntime( + WithSSCSessionInitializer(bindResumeIdentity), + WithSSCResumeAuthorizer(func(request *http.Request, session *Session) error { + if err := authorizeSameUserResume(request, session); err != nil { + return err + } + if revoked[request.Header.Get(resumeIdentityHeader)] { + return errors.New("credential revoked") + } + return nil + }), + ) + owner := detachedSession(t, runtime, "alice") + defer ReleaseSession(owner) + token := owner.ResumeToken() + + replacement, resumed, err := runtime.OpenSession(resumeRequest(t, "alice"), token) + if err != nil { + t.Fatalf("open session for revoked resume: %v", err) + } + defer ReleaseSession(replacement) + if resumed || replacement == owner { + t.Fatalf("revoked identity resumed the session: resumed=%v", resumed) + } + assertDetached(t, owner) + + revoked["alice"] = false + restored, resumed, err := runtime.OpenSession(resumeRequest(t, "alice"), token) + if err != nil { + t.Fatalf("open session after revocation cleared: %v", err) + } + if !resumed || restored != owner { + t.Fatalf("cleared credential could not resume: resumed=%v", resumed) + } +} + +// Two upgrades presenting the same token are authorized against the same +// candidate. Only one may attach it, and the loser must not be handed the +// retained session under the other request's authorization. +func TestConcurrentResumeAttachesOneAuthorizedSession(t *testing.T) { + authorizing := make(chan struct{}, 2) + release := make(chan struct{}) + runtime := resumableRuntime( + WithSSCSessionInitializer(bindResumeIdentity), + WithSSCResumeAuthorizer(func(request *http.Request, session *Session) error { + if err := authorizeSameUserResume(request, session); err != nil { + return err + } + authorizing <- struct{}{} + <-release + return nil + }), + ) + owner := detachedSession(t, runtime, "alice") + defer ReleaseSession(owner) + token := owner.ResumeToken() + + type outcome struct { + session *Session + resumed bool + err error + } + requests := []*http.Request{resumeRequest(t, "alice"), resumeRequest(t, "alice")} + results := make(chan outcome, len(requests)) + for _, request := range requests { + go func(request *http.Request) { + session, resumed, err := runtime.OpenSession(request, token) + results <- outcome{session: session, resumed: resumed, err: err} + }(request) + } + // Both attempts hold an authorized candidate before either commits. + <-authorizing + <-authorizing + close(release) + + attached := 0 + for range requests { + result := <-results + if result.err != nil { + t.Fatalf("concurrent open session: %v", result.err) + } + if result.resumed { + attached++ + if result.session != owner { + t.Fatal("resume attached a session the request was not authorized for") + } + continue + } + if result.session == owner { + t.Fatal("losing attempt received the retained session") + } + ReleaseSession(result.session) + } + if attached != 1 { + t.Fatalf("resumed sessions = %d, want 1", attached) + } +} + +// Deployments that configure no resume authorizer keep the historical +// behavior, where the token alone reattaches the session. +func TestResumeWithoutAuthorizerKeepsTokenBehavior(t *testing.T) { + runtime := resumableRuntime() + owner := detachedSession(t, runtime, "alice") + defer ReleaseSession(owner) + + restored, resumed, err := runtime.OpenSession(resumeRequest(t, "mallory"), owner.ResumeToken()) + if err != nil { + t.Fatalf("open session: %v", err) + } + if !resumed || restored != owner { + t.Fatalf("token resume changed without an authorizer: resumed=%v", resumed) + } +} + +// Releasing a retained session is final. Its token stops naming a candidate, +// so a later upgrade presenting it never reaches the authorizer, the token-only +// path cannot reattach it either, and the caller is served a new session with a +// token of its own. +func TestReleasedSessionTokenStaysUnresumable(t *testing.T) { + handed := 0 + runtime := resumableRuntime( + WithSSCSessionInitializer(bindResumeIdentity), + WithSSCResumeAuthorizer(func(request *http.Request, session *Session) error { + handed++ + return authorizeSameUserResume(request, session) + }), + ) + owner := detachedSession(t, runtime, "alice") + token := owner.ResumeToken() + + ReleaseSession(owner) + ReleaseSession(owner) + + for attempt := 0; attempt < 2; attempt++ { + session, resumed, err := runtime.OpenSession(resumeRequest(t, "alice"), token) + if err != nil { + t.Fatalf("attempt %d: open session: %v", attempt, err) + } + t.Cleanup(func() { ReleaseSession(session) }) + if resumed || session == owner { + t.Fatalf("attempt %d: the released session was resumed: resumed=%v", attempt, resumed) + } + if session.ResumeToken() == "" || session.ResumeToken() == token { + t.Fatalf("attempt %d: the fresh session carries token %q", attempt, session.ResumeToken()) + } + } + if handed != 0 { + t.Fatalf("the authorizer was handed a released session %d times", handed) + } + if _, registered := SessionByID(owner.ID()); registered { + t.Fatal("the released session is still registered") + } + // Detaching it again must not put its token back into circulation. + SuspendSession(owner, runtime.ResumeTTL()) + if resumed, ok := ResumeSession(token); ok { + t.Fatalf("the released token reattached session %q", resumed.ID()) + } +} + +// An unknown token is refused before the authorizer runs: there is no +// candidate to authorize, and the caller gets a fresh session. +func TestResumeAuthorizerSkippedForUnknownToken(t *testing.T) { + calls := 0 + runtime := resumableRuntime( + WithSSCSessionInitializer(bindResumeIdentity), + WithSSCResumeAuthorizer(func(*http.Request, *Session) error { + calls++ + return nil + }), + ) + session, resumed, err := runtime.OpenSession(resumeRequest(t, "alice"), "unknown-token") + if err != nil { + t.Fatalf("open session: %v", err) + } + defer ReleaseSession(session) + if resumed || calls != 0 { + t.Fatalf("unknown token reached the authorizer: resumed=%v calls=%d", resumed, calls) + } +} diff --git a/host/session.go b/host/session.go index 005b8aca..378c0a54 100644 --- a/host/session.go +++ b/host/session.go @@ -164,30 +164,78 @@ func SuspendSession(session *Session, ttl time.Duration) { // ResumeSession attaches a disconnected session by opaque token. // The new socket must call ReplaySession or BindSessionConnection before sends. func ResumeSession(token string) (*Session, bool) { - if token == "" { + session := sessionForToken(token) + if session == nil { return nil, false } - sessionMu.RLock() - session := sessionByToken[token] - sessionMu.RUnlock() + session.deliveryMu.Lock() + defer session.deliveryMu.Unlock() + if !session.resumableLocked() { + return nil, false + } + session.markResumedLocked() + return session, true +} + +// resumeCandidate reports the session a token can currently resume without +// mutating it, so an authorization decision can be taken before the session is +// attached, its expiry cleared or its connection replaced. +func resumeCandidate(token string) (*Session, bool) { + session := sessionForToken(token) if session == nil { return nil, false } session.deliveryMu.Lock() defer session.deliveryMu.Unlock() - if session.released || session.attached || (!session.expires.IsZero() && time.Now().After(session.expires)) { + if !session.resumableLocked() { return nil, false } - session.attached = true - session.resumePending = true - session.expires = time.Time{} - if session.expiryTimer != nil { - session.expiryTimer.Stop() - session.expiryTimer = nil - } return session, true } +// commitResume attaches a candidate that authorization approved. It fails +// closed when the token stopped mapping to that exact session or the session is +// no longer resumable, so a concurrent attempt cannot reattach a session other +// than the one that was authorized. +func commitResume(token string, candidate *Session) bool { + if candidate == nil || sessionForToken(token) != candidate { + return false + } + candidate.deliveryMu.Lock() + defer candidate.deliveryMu.Unlock() + if !candidate.resumableLocked() { + return false + } + candidate.markResumedLocked() + return true +} + +func sessionForToken(token string) *Session { + if token == "" { + return nil + } + sessionMu.RLock() + defer sessionMu.RUnlock() + return sessionByToken[token] +} + +// resumableLocked reports whether a detached session can still be reattached. +func (s *Session) resumableLocked() bool { + return !s.released && !s.attached && + (s.expires.IsZero() || !time.Now().After(s.expires)) +} + +// markResumedLocked attaches the session and cancels its retention timer. +func (s *Session) markResumedLocked() { + s.attached = true + s.resumePending = true + s.expires = time.Time{} + if s.expiryTimer != nil { + s.expiryTimer.Stop() + s.expiryTimer = nil + } +} + // ReleaseSession removes a session from the registry. func ReleaseSession(session *Session) { releaseSession(session, time.Time{}) diff --git a/host/ssc_protocol_test.go b/host/ssc_protocol_test.go index c3119f74..419dc7cc 100644 --- a/host/ssc_protocol_test.go +++ b/host/ssc_protocol_test.go @@ -314,6 +314,102 @@ func TestWSSessionResumeExpires(t *testing.T) { } } +// A detached session is reattached only for a request the resume authorizer +// approves. The refused client is served a new session and the retained one +// stays available to the identity that created it. +func TestWSResumeAuthorizationRejectsForeignToken(t *testing.T) { + type request struct{} + const action = "test.ws.resume.identity" + if err := RegisterAction(action, func(_ context.Context, session *Session, _ request) (string, error) { + user, _ := session.ContextGet("user") + text, _ := user.(string) + return text, nil + }); err != nil { + t.Fatalf("register action: %v", err) + } + + server := httptest.NewServer(NewMux(t.TempDir(), + WithSSCLimits(SSCLimits{ResumeTTL: 5 * time.Second, ReplayMessages: 8}), + WithSSCSessionInitializer(bindResumeIdentity), + WithSSCResumeAuthorizer(authorizeSameUserResume), + )) + defer server.Close() + wsURL := "ws" + strings.TrimPrefix(server.URL, "http") + "/ws" + dial := func(user string) *websocket.Conn { + config, err := websocket.NewConfig(wsURL, server.URL) + if err != nil { + t.Fatalf("websocket config: %v", err) + } + config.Header.Set(resumeIdentityHeader, user) + socket, err := websocket.DialConfig(config) + if err != nil { + t.Fatalf("dial websocket: %v", err) + } + return socket + } + + ownerSocket := dial("alice") + sendProtocolMessage(t, ownerSocket, Inbound{Action: action, ID: "alice", Sequence: 1}) + owner := receiveProtocolMessage(t, ownerSocket) + if owner.Payload != "alice" || owner.ResumeToken == "" { + t.Fatalf("session identity was not bound: %#v", owner) + } + closeTestResource(t, ownerSocket) + + intruderSocket := dial("mallory") + defer closeTestResource(t, intruderSocket) + sendProtocolMessage(t, intruderSocket, Inbound{ + Action: action, + ID: "mallory", + Sequence: 1, + Ack: owner.Sequence, + ResumeToken: owner.ResumeToken, + }) + rejected := receiveProtocolMessage(t, intruderSocket) + if rejected.Control != "resume_rejected" || rejected.Error == nil { + t.Fatalf("foreign token was not refused: %#v", rejected) + } + if rejected.Session == owner.Session { + t.Fatalf("refused client attached the retained session: %#v", rejected) + } + response := receiveProtocolMessage(t, intruderSocket) + if response.Payload != "mallory" || response.Session == owner.Session { + t.Fatalf("refused client did not get its own session: %#v", response) + } + + var ( + resumedSocket *websocket.Conn + restored Outbound + ) + deadline := time.Now().Add(2 * time.Second) + for { + resumedSocket = dial("alice") + sendProtocolMessage(t, resumedSocket, Inbound{ + Action: action, + ID: "resume", + Sequence: 2, + Ack: owner.Sequence, + ResumeToken: owner.ResumeToken, + }) + restored = receiveProtocolMessage(t, resumedSocket) + if restored.Session == owner.Session { + break + } + closeTestResource(t, resumedSocket) + if time.Now().After(deadline) { + t.Fatalf("owner could not resume after the refused attempt: %#v", restored) + } + time.Sleep(10 * time.Millisecond) + } + defer closeTestResource(t, resumedSocket) + if restored.ID != "resume" || restored.Payload != "alice" { + t.Fatalf("resumed session lost its identity: %#v", restored) + } + if session, ok := SessionByID(owner.Session); ok { + ReleaseSession(session) + } +} + func TestWSControlPingIsAnsweredOutOfBand(t *testing.T) { socket, closeSocket := openProtocolSocket(t) defer closeSocket() diff --git a/host/ws_runtime.go b/host/ws_runtime.go index 54eb9ac8..191fecef 100644 --- a/host/ws_runtime.go +++ b/host/ws_runtime.go @@ -43,17 +43,23 @@ type MessageAuthorizer func(context.Context, *Session, Inbound) error // SessionInitializer copies authenticated request state into a new session. type SessionInitializer func(*http.Request, *Session) error +// ResumeAuthorizer decides whether an upgrade request may reattach a retained +// session. It receives the new request and the detached session the presented +// token identifies, and returns a non-nil error to refuse the reattachment. +type ResumeAuthorizer func(*http.Request, *Session) error + // MuxOption configures the WebSocket endpoint created by NewMux. type MuxOption func(*WSRuntime) // WSRuntime holds the guards, limits, and connection count for one endpoint. type WSRuntime struct { - authFunc func(*http.Request) bool - origins []string - authorize MessageAuthorizer - initialize SessionInitializer - limits SSCLimits - connections atomic.Int64 + authFunc func(*http.Request) bool + origins []string + authorize MessageAuthorizer + initialize SessionInitializer + authorizeResume ResumeAuthorizer + limits SSCLimits + connections atomic.Int64 } // NewWSRuntime resolves MuxOptions into an endpoint runtime. @@ -83,10 +89,29 @@ func WithSSCAuthorizer(authorize MessageAuthorizer) MuxOption { } // WithSSCSessionInitializer initializes session identity from the upgrade request. +// It runs for new sessions only; a resumed session keeps the identity bound +// when it was created, so use WithSSCResumeAuthorizer to decide who may +// reattach it. func WithSSCSessionInitializer(initialize SessionInitializer) MuxOption { return func(runtime *WSRuntime) { runtime.initialize = initialize } } +// WithSSCResumeAuthorizer authorizes reattachment of a retained session. +// The callback compares the authenticated upgrade request with the session it +// would resume (identity bound by the session initializer, revocation state, +// anything else the deployment tracks) and returns an error to refuse. +// +// A refusal leaves the retained session untouched and detached, so its owner +// can still resume it while its TTL lasts, and the caller is served a fresh +// session exactly like an unknown token. +// +// Without this option any client presenting a valid token resumes the session, +// which is the historical behavior. An authenticated multi-user deployment must +// configure a resume authorizer or disable resume with WithoutSSCResume. +func WithSSCResumeAuthorizer(authorize ResumeAuthorizer) MuxOption { + return func(runtime *WSRuntime) { runtime.authorizeResume = authorize } +} + // WithSSCLimits overrides non-zero SSC resource limits. func WithSSCLimits(limits SSCLimits) MuxOption { return func(runtime *WSRuntime) { @@ -215,15 +240,36 @@ func (runtime *WSRuntime) NewSession(request *http.Request) (*Session, error) { return session, nil } -// OpenSession resumes a retained session before allocating a new one. +// OpenSession resumes a retained session before allocating a new one. A token +// the request is not allowed to reattach is treated like an unresumable one: +// the caller gets a new session and the retained one stays available to its +// owner. func (runtime *WSRuntime) OpenSession(request *http.Request, resumeToken string) (*Session, bool, error) { - if resumed, ok := ResumeSession(resumeToken); ok { + if resumed, ok := runtime.resumeSession(request, resumeToken); ok { return resumed, true, nil } session, err := runtime.NewSession(request) return session, false, err } +// resumeSession authorizes a reattachment before any session state changes. +func (runtime *WSRuntime) resumeSession(request *http.Request, resumeToken string) (*Session, bool) { + if runtime == nil || runtime.authorizeResume == nil { + return ResumeSession(resumeToken) + } + candidate, ok := resumeCandidate(resumeToken) + if !ok { + return nil, false + } + if err := runtime.authorizeResume(request, candidate); err != nil { + return nil, false + } + if !commitResume(resumeToken, candidate) { + return nil, false + } + return candidate, true +} + // Authorize validates a decoded message. func (runtime *WSRuntime) Authorize(ctx context.Context, session *Session, message Inbound) error { if runtime == nil || runtime.authorize == nil { diff --git a/hostclient/component_binding_wasm_test.go b/hostclient/component_binding_wasm_test.go new file mode 100644 index 00000000..06a22a27 --- /dev/null +++ b/hostclient/component_binding_wasm_test.go @@ -0,0 +1,543 @@ +//go:build js && wasm + +package hostclient + +import ( + "strings" + "testing" + "time" + + "github.com/rfwlab/rfw/v2/core" + dom "github.com/rfwlab/rfw/v2/dom" + js "github.com/rfwlab/rfw/v2/js" +) + +// The plain registrar keeps the signature applications pass to +// core.SetHostRegister as a function value. +var _ func(id, name string, vars []string) = RegisterComponent + +// The owned registrar is what core.SetHostRegistrar takes. +var _ core.HostRegistrar = RegisterComponentOwned + +func bindingSnapshot(name string) (componentBinding, bool) { + mu.RLock() + defer mu.RUnlock() + binding, bound := bindings[name] + return binding, bound +} + +func pendingFor(name string) (inits, unsubscribes int) { + mu.RLock() + defer mu.RUnlock() + for _, queued := range pending { + if queued.name != name { + continue + } + values, _ := queued.payload.(map[string]any) + if values["init"] == true { + inits++ + } + if values["unsubscribe"] == true { + unsubscribes++ + } + } + return inits, unsubscribes +} + +// pendingLabels renders the queued messages for the given names in queue order, +// so a test can assert where a control sits and not only how many there are. +func pendingLabels(names ...string) []string { + wanted := make(map[string]struct{}, len(names)) + for _, name := range names { + wanted[name] = struct{}{} + } + mu.RLock() + defer mu.RUnlock() + labels := make([]string, 0, len(pending)) + for _, queued := range pending { + if _, ok := wanted[queued.name]; !ok { + continue + } + labels = append(labels, queued.name+"/"+pendingKind(queued.payload)) + } + return labels +} + +func pendingKind(payload any) string { + values, ok := payload.(map[string]any) + if !ok { + return "message" + } + switch { + case values["init"] == true: + return "init" + case values["unsubscribe"] == true: + return "unsubscribe" + } + if cmd, ok := values["cmd"].(string); ok { + return "cmd:" + cmd + } + return "message" +} + +// forgetPending drops the messages a test queued, so package state does not +// leak into the next one. +func forgetPending(t *testing.T, name string) { + t.Cleanup(func() { + mu.Lock() + filtered := pending[:0] + for _, queued := range pending { + if queued.name == name { + continue + } + filtered = append(filtered, queued) + } + pending = filtered + mu.Unlock() + }) +} + +// setConnection makes the package treat c as its host connection, the way the +// connection loop does once a handshake completes. A nil c is the offline +// state, where registration controls go to the reconnect queue. +func setConnection(t *testing.T, c *hostConn) { + t.Helper() + mu.Lock() + conn = c + mu.Unlock() + t.Cleanup(func() { + mu.Lock() + conn = nil + mu.Unlock() + }) +} + +// setSnapshotBarrier installs the delivery barrier under the lock the read loop +// reads it with. +func setSnapshotBarrier(fn func(component string)) { + mu.Lock() + afterBindingSnapshot = fn + mu.Unlock() +} + +// A registration made through the plain registrar owns no cleanup: it lives +// until another registration replaces it, which is the behavior code calling +// RegisterComponent as a statement has always had. +func TestRegisterComponentBindsWithoutACleanup(t *testing.T) { + name := "plain-binding" + forgetPending(t, name) + + RegisterComponent("plain-root", name, []string{"value"}) + binding, bound := bindingSnapshot(name) + if !bound || binding.id != "plain-root" { + t.Fatalf("registration bound %#v bound=%v", binding, bound) + } + + release := RegisterComponentOwned("owned-root", name, nil) + if binding, _ := bindingSnapshot(name); binding.id != "owned-root" { + t.Fatalf("the owned registration did not replace the plain one: %#v", binding) + } + release() + if _, bound := bindingSnapshot(name); bound { + t.Fatal("binding remained after the owned cleanup") + } +} + +func TestRegisterComponentCleanupReleasesTheBinding(t *testing.T) { + name := "scoped-binding" + setConnection(t, nil) + forgetPending(t, name) + + release := RegisterComponentOwned("scoped-root", name, []string{"value"}) + if _, bound := bindingSnapshot(name); !bound { + t.Fatal("registration did not bind the component") + } + if inits, _ := pendingFor(name); inits != 1 { + t.Fatalf("queued init messages = %d, want 1", inits) + } + + release() + release() + + if _, bound := bindingSnapshot(name); bound { + t.Fatal("binding remained after cleanup") + } + inits, unsubscribes := pendingFor(name) + if inits != 0 { + t.Fatalf("stale init messages = %d, want 0", inits) + } + if unsubscribes != 1 { + t.Fatalf("queued unsubscribe messages = %d, want 1", unsubscribes) + } +} + +func TestStaleComponentCleanupKeepsTheReplacementBinding(t *testing.T) { + name := "replacement-binding" + forgetPending(t, name) + + first := RegisterComponentOwned("first-root", name, nil) + second := RegisterComponentOwned("second-root", name, nil) + first() + + binding, bound := bindingSnapshot(name) + if !bound || binding.id != "second-root" { + t.Fatalf("stale cleanup removed the replacement binding: %#v bound=%v", binding, bound) + } + second() + if _, bound := bindingSnapshot(name); bound { + t.Fatal("the replacement binding survived its own cleanup") + } +} + +// With a live connection the lifecycle talks to the host directly: one init on +// registration and one unsubscribe on cleanup, however often the cleanup runs, +// and nothing left queued for a reconnect. +func TestActiveConnectionCleanupSendsOneUnsubscribe(t *testing.T) { + c, socket := dialFake(t) + setConnection(t, c) + name := "wired-ticker" + forgetPending(t, name) + + release := RegisterComponentOwned("wired-root", name, nil) + release() + release() + + inits, unsubscribes := 0, 0 + for _, frame := range socket.sent() { + if !strings.Contains(frame, `"component":"`+name+`"`) { + continue + } + if strings.Contains(frame, `"init":true`) { + inits++ + } + if strings.Contains(frame, `"unsubscribe":true`) { + unsubscribes++ + } + } + if inits != 1 || unsubscribes != 1 { + t.Fatalf("frames sent: init=%d unsubscribe=%d, want 1 each", inits, unsubscribes) + } + if _, queued := pendingFor(name); queued != 0 { + t.Fatalf("a live connection queued %d unsubscribes", queued) + } +} + +// Route entry and exit repeated: every mount owns one binding and one init, and +// leaves nothing behind when it unmounts. Offline, the queue keeps only the +// latest desired state for the name, so churn does not retain protocol work +// proportional to the number of cycles. +func TestComponentRegistrationCyclesRetainNothing(t *testing.T) { + name := "cycled-binding" + setConnection(t, nil) + forgetPending(t, name) + + for cycle := 0; cycle < 100; cycle++ { + release := RegisterComponentOwned("cycled-root", name, nil) + if _, bound := bindingSnapshot(name); !bound { + t.Fatalf("cycle %d: registration did not bind the component", cycle) + } + release() + if _, bound := bindingSnapshot(name); bound { + t.Fatalf("cycle %d: binding retained after cleanup", cycle) + } + } + + mu.RLock() + _, tracked := bindingTokens[name] + mu.RUnlock() + if tracked { + t.Fatal("registration bookkeeping retained the released name") + } + inits, unsubscribes := pendingFor(name) + if inits != 0 || unsubscribes != 1 { + t.Fatalf("queued messages: init=%d unsubscribe=%d, want 0 and 1", inits, unsubscribes) + } +} + +// The pending queue is a per-name desired state, not a log: a control is +// superseded where it stands, so the messages that are not registration +// controls, and the controls of other names, keep their exact place. +func TestPendingRegistrationControlKeepsOnlyTheLatestState(t *testing.T) { + name := "queued-binding" + other := "other-binding" + setConnection(t, nil) + forgetPending(t, name) + forgetPending(t, other) + + assertQueue := func(step string, want ...string) { + t.Helper() + got := strings.Join(pendingLabels(name, other), " ") + if expected := strings.Join(want, " "); got != expected { + t.Fatalf("%s: queue = %q, want %q", step, got, expected) + } + } + + release := RegisterComponentOwned("queued-root", name, nil) + Send(name, map[string]any{"cmd": "refresh"}) + otherRelease := RegisterComponentOwned("other-root", other, nil) + assertQueue("after registration", + name+"/init", name+"/cmd:refresh", other+"/init") + + release() + assertQueue("after release", + name+"/unsubscribe", name+"/cmd:refresh", other+"/init") + + second := RegisterComponentOwned("queued-root", name, nil) + assertQueue("after re-registration", + name+"/init", name+"/cmd:refresh", other+"/init") + + otherRelease() + assertQueue("after the other release", + name+"/init", name+"/cmd:refresh", other+"/unsubscribe") + + second() + assertQueue("after the second release", + name+"/unsubscribe", name+"/cmd:refresh", other+"/unsubscribe") +} + +// A control decided with the connection up goes out on the wire, so the queue +// keeps neither it nor the state it superseded, and everything queued around it +// stays where it was. +func TestConnectedRegistrationControlDropsTheQueuedControl(t *testing.T) { + name := "wired-queue-binding" + other := "wired-queue-other" + setConnection(t, nil) + forgetPending(t, name) + forgetPending(t, other) + + release := RegisterComponentOwned("wired-queue-root", name, nil) + Send(name, map[string]any{"cmd": "refresh"}) + otherRelease := RegisterComponentOwned("wired-queue-other-root", other, nil) + t.Cleanup(otherRelease) + + c, _ := dialFake(t) + setConnection(t, c) + release() + + got := strings.Join(pendingLabels(name, other), " ") + want := strings.Join([]string{name + "/cmd:refresh", other + "/init"}, " ") + if got != want { + t.Fatalf("queue = %q, want %q", got, want) + } +} + +// This package installs the core registration hook from its own init, so a +// mounted component binds through the real boundary and unmounting releases it, +// with no test double in between. +func TestCoreComponentLifecycleOwnsItsBinding(t *testing.T) { + setConnection(t, nil) + forgetPending(t, "BoundaryHost") + + component := core.NewHTMLComponent("BoundaryComponent", []byte(``), nil) + component.AddHostComponent("BoundaryHost") + component.Init(nil) + + for cycle := 0; cycle < 100; cycle++ { + component.Mount() + component.Render() + binding, bound := bindingSnapshot("BoundaryHost") + if !bound || binding.id != component.ID { + t.Fatalf("cycle %d: mount bound %#v, want the component id %q", cycle, binding, component.ID) + } + component.Unmount() + if _, bound := bindingSnapshot("BoundaryHost"); bound { + t.Fatalf("cycle %d: unmount left the binding in place", cycle) + } + } + + mu.RLock() + _, tracked := bindingTokens["BoundaryHost"] + mu.RUnlock() + if tracked { + t.Fatal("route churn retained the registration token") + } + inits, unsubscribes := pendingFor("BoundaryHost") + if inits != 0 || unsubscribes != 1 { + t.Fatalf("queued messages: init=%d unsubscribe=%d, want 0 and 1", inits, unsubscribes) + } +} + +// hostVarRoot builds a component root carrying one host variable element. +func hostVarRoot(t *testing.T, id string) dom.Element { + t.Helper() + root := dom.CreateElement("div") + root.SetAttr("data-component-id", id) + variable := dom.CreateElement("span") + variable.SetAttr("data-host-var", "value") + root.AppendChild(variable) + dom.Doc().Body().AppendChild(root) + t.Cleanup(func() { root.Call("remove") }) + return root +} + +// appSentinel puts a host variable under #app, which is where +// dom.ComponentRoot falls back when a component id resolves to nothing. Host +// delivery must never take that fallback, so this element stays untouched. +func appSentinel(t *testing.T) dom.Element { + t.Helper() + app := dom.ByID("app") + if app.IsNull() || app.IsUndefined() { + app = dom.CreateElement("div") + app.SetAttr("id", "app") + dom.Doc().Body().AppendChild(app) + t.Cleanup(func() { app.Call("remove") }) + } + sentinel := dom.CreateElement("span") + sentinel.SetAttr("data-host-var", "value") + sentinel.SetText("untouched") + app.AppendChild(sentinel) + t.Cleanup(func() { sentinel.Call("remove") }) + return sentinel +} + +func hostVarText(root dom.Element) string { + return root.Query(`[data-host-var="value"]`).Text() +} + +func waitForHostVar(t *testing.T, root dom.Element, want string) { + t.Helper() + for i := 0; i < 200; i++ { + if hostVarText(root) == want { + return + } + time.Sleep(5 * time.Millisecond) + } + t.Fatalf("host variable = %q, want %q", hostVarText(root), want) +} + +// A component that unmounts stops receiving host pushes: the frame that arrives +// after its cleanup reaches neither the root it owned, which route exit removed +// from the document, nor the #app shell the generic root lookup would fall back +// to, while a component registered afterwards keeps receiving its own. +func TestReleasedBindingIgnoresLateDelivery(t *testing.T) { + conn, socket := dialFake(t) + forgetPending(t, "released-ticker") + forgetPending(t, "live-ticker") + sentinel := appSentinel(t) + releasedRoot := hostVarRoot(t, "released-root") + liveRoot := hostVarRoot(t, "live-root") + + release := RegisterComponentOwned("released-root", "released-ticker", []string{"value"}) + t.Cleanup(release) + go func() { _ = readOnce(conn, 2*time.Second) }() + + socket.deliver(`{"component":"released-ticker","payload":{"value":"1"},"sequence":1}`) + waitForHostVar(t, releasedRoot, "1") + + // Route exit: the cleanup runs and the root it owned leaves the document. + release() + releasedRoot.Call("remove") + socket.deliver(`{"component":"released-ticker","payload":{"value":"2"},"sequence":2}`) + + // Frames are applied in order, so a later frame reaching a live binding + // proves the released one was already processed. + releaseLive := RegisterComponentOwned("live-root", "live-ticker", []string{"value"}) + t.Cleanup(releaseLive) + socket.deliver(`{"component":"live-ticker","payload":{"value":"3"},"sequence":3}`) + waitForHostVar(t, liveRoot, "3") + + if got := hostVarText(releasedRoot); got != "1" { + t.Fatalf("the released root was updated to %q", got) + } + if got := sentinel.Text(); got != "untouched" { + t.Fatalf("a released frame fell back to #app and wrote %q", got) + } +} + +// withoutCSSEscape hides the CSS object for the duration of a test, which is +// the runtime where the root has to be found by scanning the attribute. +func withoutCSSEscape(t *testing.T) { + t.Helper() + original := js.Get("CSS") + js.Set("CSS", js.Null()) + t.Cleanup(func() { js.Set("CSS", original) }) +} + +// RegisterComponent is public and takes any id, so an application can bind a +// root whose id is not a CSS identifier. Delivery has to resolve exactly that +// root: an id spliced into a selector raw would throw a DOMException out of +// querySelector, or select a root the binding does not own. +func TestDeliveryResolvesRootsWithCSSMetacharacters(t *testing.T) { + cases := []struct { + label string + component string + id string + escape bool + }{ + {label: "css escape", component: "meta-escaped", id: `meta"root'\]:.#[ 1`, escape: true}, + {label: "attribute scan", component: "meta-scanned", id: `meta"root'\]:.#[ 2`, escape: false}, + } + for _, tc := range cases { + t.Run(tc.label, func(t *testing.T) { + if !tc.escape { + withoutCSSEscape(t) + } + conn, socket := dialFake(t) + forgetPending(t, tc.component) + sentinel := appSentinel(t) + decoy := hostVarRoot(t, "meta-decoy") + root := hostVarRoot(t, tc.id) + + release := RegisterComponentOwned(tc.id, tc.component, []string{"value"}) + t.Cleanup(release) + go func() { _ = readOnce(conn, 2*time.Second) }() + + socket.deliver(`{"component":"` + tc.component + `","payload":{"value":"1"},"sequence":1}`) + waitForHostVar(t, root, "1") + + if got := hostVarText(decoy); got != "" { + t.Fatalf("the frame also wrote %q into a root it does not own", got) + } + if got := sentinel.Text(); got != "untouched" { + t.Fatalf("the frame fell back to #app and wrote %q", got) + } + }) + } +} + +// The window a cleanup has to close is the one between the read loop +// snapshotting a binding and the delivery touching the DOM. The barrier runs +// inside exactly that window, on a frame the loop has already accepted, so the +// overlap is reproduced rather than raced for. +func TestCleanupDuringAnInFlightDeliveryWins(t *testing.T) { + conn, socket := dialFake(t) + forgetPending(t, "overlap-ticker") + forgetPending(t, "overlap-live") + sentinel := appSentinel(t) + overlapRoot := hostVarRoot(t, "overlap-root") + liveRoot := hostVarRoot(t, "overlap-live-root") + + release := RegisterComponentOwned("overlap-root", "overlap-ticker", []string{"value"}) + t.Cleanup(release) + releaseLive := RegisterComponentOwned("overlap-live-root", "overlap-live", []string{"value"}) + t.Cleanup(releaseLive) + + barriers := make(chan struct{}, 1) + setSnapshotBarrier(func(component string) { + if component != "overlap-ticker" { + return + } + setSnapshotBarrier(nil) + release() + overlapRoot.Call("remove") + barriers <- struct{}{} + }) + t.Cleanup(func() { setSnapshotBarrier(nil) }) + + go func() { _ = readOnce(conn, 2*time.Second) }() + socket.deliver(`{"component":"overlap-ticker","payload":{"value":"9"},"sequence":1}`) + socket.deliver(`{"component":"overlap-live","payload":{"value":"10"},"sequence":2}`) + waitForHostVar(t, liveRoot, "10") + + select { + case <-barriers: + default: + t.Fatal("the delivery never reached the snapshot barrier") + } + if got := hostVarText(overlapRoot); got != "" { + t.Fatalf("a delivery snapshotted before the cleanup still wrote %q", got) + } + if got := sentinel.Text(); got != "untouched" { + t.Fatalf("the overlapping delivery fell back to #app and wrote %q", got) + } +} diff --git a/hostclient/coreinit.go b/hostclient/coreinit.go index 215722bb..34c6c93f 100644 --- a/hostclient/coreinit.go +++ b/hostclient/coreinit.go @@ -7,4 +7,4 @@ import "github.com/rfwlab/rfw/v2/core" // Linking this package wires SSC component registration into core. Apps that // never use SSC never import hostclient, so the websocket and net/http stacks // stay out of their bundle. -func init() { core.SetHostRegister(RegisterComponent) } +func init() { core.SetHostRegistrar(RegisterComponentOwned) } diff --git a/hostclient/host_signal_delivery_wasm_test.go b/hostclient/host_signal_delivery_wasm_test.go new file mode 100644 index 00000000..493affab --- /dev/null +++ b/hostclient/host_signal_delivery_wasm_test.go @@ -0,0 +1,198 @@ +//go:build js && wasm + +package hostclient + +import ( + "sync" + "testing" + "time" + + dom "github.com/rfwlab/rfw/v2/dom" + "github.com/rfwlab/rfw/v2/state" +) + +// setHostSignalWriteHook installs the delivery hook under the lock the read +// loop reads it with. +func setHostSignalWriteHook(fn func(component, name string)) { + mu.Lock() + beforeHostSignalWrite = fn + mu.Unlock() +} + +// hostSignalRoot builds the root a binding owns and registers one host signal +// per name against it, which is the state a rendered component leaves behind. +func hostSignalRoot(t *testing.T, id string, names ...string) map[string]*state.Signal[string] { + t.Helper() + root := dom.CreateElement("div") + root.SetAttr("data-component-id", id) + dom.Doc().Body().AppendChild(root) + signals := make(map[string]*state.Signal[string], len(names)) + for _, name := range names { + signal := state.NewSignal("initial") + dom.RegisterSignal(id, name, signal) + signals[name] = signal + } + t.Cleanup(func() { + dom.RemoveComponentSignals(id) + root.Call("remove") + }) + return signals +} + +func waitForSignal(t *testing.T, signal *state.Signal[string], want string) { + t.Helper() + for i := 0; i < 200; i++ { + if signal.Get() == want { + return + } + time.Sleep(5 * time.Millisecond) + } + t.Fatalf("host signal = %q, want %q", signal.Get(), want) +} + +// untouched reports the signals of a component that still hold their initial +// value, so a test can count how much of a frame landed. +func untouched(signals map[string]*state.Signal[string]) int { + count := 0 + for _, signal := range signals { + if signal.Get() == "initial" { + count++ + } + } + return count +} + +// The window a cleanup has to close for host signals is the one between the +// ownership check a delivery makes and the write it was about to perform. The +// hook runs inside exactly that window, on a frame already accepted, so the +// overlap is reproduced rather than raced for: the cleanup returns first, and +// the frame it overlapped leaves the released component's signals alone. +func TestCleanupBetweenTheSignalCheckAndTheWriteWins(t *testing.T) { + conn, socket := dialFake(t) + forgetPending(t, "signal-ticker") + forgetPending(t, "signal-live") + released := hostSignalRoot(t, "signal-root", "value") + live := hostSignalRoot(t, "signal-live-root", "value") + + release := RegisterComponentOwned("signal-root", "signal-ticker", []string{"value"}) + t.Cleanup(release) + releaseLive := RegisterComponentOwned("signal-live-root", "signal-live", []string{"value"}) + t.Cleanup(releaseLive) + + cleanupReturned := make(chan struct{}) + setHostSignalWriteHook(func(component, name string) { + if component != "signal-ticker" { + return + } + setHostSignalWriteHook(nil) + // Route exit runs on its own goroutine, as it does in a browser, and + // the delivery waits for the cleanup to return before its write. + go func() { + release() + close(cleanupReturned) + }() + select { + case <-cleanupReturned: + case <-time.After(2 * time.Second): + t.Error("the cleanup did not return while a delivery was mid-frame") + } + }) + t.Cleanup(func() { setHostSignalWriteHook(nil) }) + + go func() { _ = readOnce(conn, 2*time.Second) }() + socket.deliver(`{"component":"signal-ticker","payload":{"value":"stale"},"sequence":1}`) + // Frames are applied in order, so a later frame reaching a live binding + // proves the overlapped one was already processed. + socket.deliver(`{"component":"signal-live","payload":{"value":"fresh"},"sequence":2}`) + waitForSignal(t, live["value"], "fresh") + + select { + case <-cleanupReturned: + default: + t.Fatal("the delivery never reached the signal write hook") + } + if got := released["value"].Get(); got != "initial" { + t.Fatalf("a frame that overlapped the cleanup wrote %q into a released host signal", got) + } +} + +// A host signal setter that unmounts its own component re-enters the release +// from inside the frame being applied. The release must not wait on the +// delivery it is running under, and the rest of that frame belongs to the +// registration the setter just dropped. +func TestSetterReleasingItsOwnBindingEndsTheFrame(t *testing.T) { + conn, socket := dialFake(t) + forgetPending(t, "reentrant-ticker") + forgetPending(t, "reentrant-live") + signals := hostSignalRoot(t, "reentrant-root", "first", "second") + live := hostSignalRoot(t, "reentrant-live-root", "value") + + release := RegisterComponentOwned("reentrant-root", "reentrant-ticker", []string{"first", "second"}) + t.Cleanup(release) + releaseLive := RegisterComponentOwned("reentrant-live-root", "reentrant-live", []string{"value"}) + t.Cleanup(releaseLive) + + var unmount sync.Once + for _, signal := range signals { + signal.OnChange(func(string) { unmount.Do(release) }) + } + + go func() { _ = readOnce(conn, 2*time.Second) }() + socket.deliver(`{"component":"reentrant-ticker","payload":{"first":"one","second":"two"},"sequence":1}`) + // The witness frame is what proves the read loop came back from the + // re-entrant release instead of deadlocking inside it. + socket.deliver(`{"component":"reentrant-live","payload":{"value":"fresh"},"sequence":2}`) + waitForSignal(t, live["value"], "fresh") + + if remaining := untouched(signals); remaining != 1 { + t.Fatalf("host signals left untouched = %d, want 1: the release from a setter did not end the frame", remaining) + } + if _, bound := bindingSnapshot("reentrant-ticker"); bound { + t.Fatal("the binding survived the release its own setter ran") + } +} + +// A setter that mounts the component again re-enters registration from inside +// the frame. The replacement is what stays bound, and the frame the superseded +// registration was carrying ends where it was superseded. +func TestSetterRebindingItsComponentEndsTheFrame(t *testing.T) { + conn, socket := dialFake(t) + forgetPending(t, "rebound-ticker") + forgetPending(t, "rebound-live") + signals := hostSignalRoot(t, "rebound-root", "first", "second") + live := hostSignalRoot(t, "rebound-live-root", "value") + + release := RegisterComponentOwned("rebound-root", "rebound-ticker", []string{"first", "second"}) + t.Cleanup(release) + releaseLive := RegisterComponentOwned("rebound-live-root", "rebound-live", []string{"value"}) + t.Cleanup(releaseLive) + + var remount sync.Once + rebound := make(chan func(), 1) + for _, signal := range signals { + signal.OnChange(func(string) { + remount.Do(func() { + rebound <- RegisterComponentOwned("remounted-root", "rebound-ticker", []string{"first"}) + }) + }) + } + + go func() { _ = readOnce(conn, 2*time.Second) }() + socket.deliver(`{"component":"rebound-ticker","payload":{"first":"one","second":"two"},"sequence":1}`) + socket.deliver(`{"component":"rebound-live","payload":{"value":"fresh"},"sequence":2}`) + waitForSignal(t, live["value"], "fresh") + + select { + case releaseRemounted := <-rebound: + t.Cleanup(releaseRemounted) + default: + t.Fatal("the setter never re-registered the component") + } + if remaining := untouched(signals); remaining != 1 { + t.Fatalf("host signals left untouched = %d, want 1: the registration from a setter did not end the frame", remaining) + } + binding, bound := bindingSnapshot("rebound-ticker") + if !bound || binding.id != "remounted-root" { + t.Fatalf("the replacement registration is not the live one: %#v bound=%v", binding, bound) + } +} diff --git a/hostclient/runtime_foundation.go b/hostclient/runtime_foundation.go index 5685e06a..8656cd11 100644 --- a/hostclient/runtime_foundation.go +++ b/hostclient/runtime_foundation.go @@ -23,11 +23,46 @@ import ( type componentBinding struct { id string vars []string + // gate carries the registration's ownership into the host signal writes a + // frame makes, which happen with no binding lock held. + gate *deliveryGate } +// deliveryGate is the revocation switch a registration hands to every frame +// delivered under it. A release closes it inside the same section that drops +// the registration and then waits out the writes it may already have allowed, +// so a frame the read loop snapshotted before a cleanup cannot update a host +// signal once that cleanup returned. Reading it costs one atomic load and takes +// no lock, so a signal setter that re-enters registration or release cannot +// deadlock against a delivery holding it. +type deliveryGate struct{ closed atomic.Bool } + +// open reports whether the registration this gate belongs to is still the live +// one. The zero binding carries no gate and owns nothing. +func (g *deliveryGate) open() bool { return g != nil && !g.closed.Load() } + +func (g *deliveryGate) close() { + if g != nil { + g.closed.Store(true) + } +} + +// gatedHostSetter is a host signal that can fold the caller's ownership check +// into its own store, so the two are one step against a concurrent release. +type gatedHostSetter interface { + SetFromHostGated(raw any, allow func() bool) bool +} + +// hostSetter is the plain host signal contract, without the gate. +type hostSetter interface{ SetFromHost(any) } + +// hostWriteBarrier waits out a gated write that was already allowed. +type hostWriteBarrier interface{ HostWriteBarrier() } + var ( conn *hostConn bindings = map[string]componentBinding{} + bindingTokens = map[string]uint64{} once sync.Once mu sync.RWMutex pending []message @@ -51,8 +86,28 @@ var ( callSequence atomic.Uint64 handlerSequence atomic.Uint64 + bindingSequence atomic.Uint64 callMu sync.Mutex pendingCalls = map[string]chan actionReply{} + + // bindingMu serializes host binding delivery against registration and + // release. A frame is validated against the live registration token and its + // DOM applied while it is held, so a release that returns has already waited + // out any in-flight DOM update and every later frame fails the token check. + // It is always taken before mu and never while mu is held. Host signal + // writes run application code and cannot be held under it; they carry the + // registration's deliveryGate instead, and a signal lock is never taken + // while either binding lock is held. + bindingMu sync.Mutex + // afterBindingSnapshot is nil outside tests and is read under mu with the + // binding snapshot itself. It opens the window between snapshotting a + // binding in the read loop and taking bindingMu, which is where a + // concurrent release has to win. + afterBindingSnapshot func(component string) + // beforeHostSignalWrite is nil outside tests and is read under mu. It opens + // the window between the ownership check a delivery makes and the write + // itself, the one the gate exists to close. + beforeHostSignalWrite func(component, name string) ) // Host snapshots can legitimately reach several megabytes (for example, a @@ -385,7 +440,9 @@ func readLoop(ctx context.Context, c *hostConn) error { } mu.RLock() h, hasHandler := handlers[msg.Component] - b, hasBinding := bindings[msg.Component] + _, hasBinding := bindings[msg.Component] + token := bindingTokens[msg.Component] + barrier := afterBindingSnapshot mu.RUnlock() if hasHandler { if msg.Session != "" { @@ -395,54 +452,225 @@ func readLoop(ctx context.Context, c *hostConn) error { continue } if hasBinding { + if barrier != nil { + barrier(msg.Component) + } js.Guard("host binding: "+msg.Component, func() { - applyHostBinding(msg.Component, payload, b) + applyHostBinding(msg.Component, payload, token) }) } } } -func applyHostBinding(component string, payload map[string]any, binding componentBinding) { - rootEl := dom.ComponentRoot(binding.id) - if !rootEl.Truthy() { +// hostSignalUpdate is one host variable a delivered frame carries, held until +// the DOM work is done and its signal can be set outside bindingMu. +type hostSignalUpdate struct { + name string + value any +} + +// applyHostBinding delivers one host frame to the binding that owns it: the DOM +// of the root under bindingMu, then the signals and any resync with the lock +// released, since both run code that reacquires it. +func applyHostBinding(component string, payload map[string]any, token uint64) { + updates, mismatches := deliverHostFrame(component, payload, token) + applyHostSignals(component, token, updates) + if len(mismatches) == 0 { return } + for _, mismatch := range mismatches { + log.Printf("hostclient: hydration mismatch component=%s var=%s expected=%s actualHash=%s actual=%q", component, mismatch.VarName, mismatch.Expected, mismatch.ActualHash, mismatch.Actual) + } + resyncErr := hydrateCB.Execute(func() error { + Send(component, buildResyncPayload(mismatches)) + return nil + }) + if resyncErr != nil { + log.Printf("hostclient: hydration circuit open, skipping resync for %s", component) + } +} + +// deliverHostFrame updates the DOM of the root the binding owns and reports the +// signal updates the frame carries. It runs under bindingMu and re-reads the +// registration token there: a frame the read loop snapshotted before a cleanup +// blocks until the release completes and then finds the token gone, so it +// reaches neither the released root nor the replacement registered under the +// same name. mu is taken only for the token read and the snapshot bookkeeping. +func deliverHostFrame(component string, payload map[string]any, token uint64) ([]hostSignalUpdate, []hydrationMismatch) { + bindingMu.Lock() + defer bindingMu.Unlock() + + binding, live := liveBinding(component, token) + if !live { + return nil, nil + } + rootEl := hostComponentRoot(binding.id) + if !rootEl.Truthy() { + return nil, nil + } root := newComponentRoot(rootEl) if snap := decodeInitSnapshotPayload(payload["initSnapshot"]); snap != nil { applyInitSnapshot(root, snap) if len(snap.Vars) > 0 { binding.vars = append([]string(nil), snap.Vars...) mu.Lock() - bindings[component] = binding + // Only the registration this frame was validated against may be + // updated: a snapshot must not reinstate a released binding nor + // overwrite a newer one. + if bindingTokens[component] == token { + bindings[component] = binding + } mu.Unlock() } - return + return nil, nil } + var updates []hostSignalUpdate mismatches := handleHostPayload(root, payload, func(name string, raw any) { - signals := dom.SnapshotComponentSignals(binding.id) - if signals == nil { + updates = append(updates, hostSignalUpdate{name: name, value: raw}) + }) + return updates, mismatches +} + +// applyHostSignals pushes a delivered frame into the component's host signals. +// A setter runs application code, which may register or release a host binding +// in turn, so it must not run under bindingMu. The registration's gate carries +// the ownership check into the write instead: the signal evaluates it under the +// same lock it stores the value with, so a release either refused the write or +// waited it out before returning, and neither side holds a binding lock while +// application code runs. +func applyHostSignals(component string, token uint64, updates []hostSignalUpdate) { + if len(updates) == 0 { + return + } + binding, live := liveBinding(component, token) + if !live { + return + } + signals := dom.SnapshotComponentSignals(binding.id) + if len(signals) == 0 { + return + } + hook := hostSignalWriteHook() + for _, update := range updates { + signal, ok := signals[update.name] + if !ok { + continue + } + if hook != nil { + hook(component, update.name) + } + if !applyHostSignal(signal, binding.gate, update.value) { + // The registration was released mid-frame: the rest of the frame + // was addressed to it too. return } - if signal, ok := signals[name]; ok { - if setter, ok := signal.(interface{ SetFromHost(any) }); ok { - setter.SetFromHost(raw) - } + } +} + +// applyHostSignal writes one update and reports whether the registration was +// still live for it. A signal that predates the gated setter cannot fold the +// check into its store, so it is checked before the write instead, which leaves +// the window the gate exists to close. +func applyHostSignal(signal any, gate *deliveryGate, value any) bool { + if gated, ok := signal.(gatedHostSetter); ok { + if gated.SetFromHostGated(value, gate.open) { + return true } - }) - if len(mismatches) == 0 { + // A refused write is either a revoked registration or a payload the + // signal cannot represent; only the first one ends the frame. + return gate.open() + } + setter, ok := signal.(hostSetter) + if !ok { + return true + } + if !gate.open() { + return false + } + setter.SetFromHost(value) + return true +} + +// fenceHostSignalWrites waits out the host signal writes a now closed gate had +// already allowed. Each barrier takes only that signal's own value lock, which +// never covers application code, so a release re-entered from a setter never +// waits on itself. It covers the signals the component still has registered, +// which is why core unmounts a component by releasing its host bindings before +// it drops its signals. +func fenceHostSignalWrites(id string) { + if id == "" { return } - for _, mismatch := range mismatches { - log.Printf("hostclient: hydration mismatch component=%s var=%s expected=%s actualHash=%s actual=%q", component, mismatch.VarName, mismatch.Expected, mismatch.ActualHash, mismatch.Actual) + for _, signal := range dom.SnapshotComponentSignals(id) { + if barrier, ok := signal.(hostWriteBarrier); ok { + barrier.HostWriteBarrier() + } } - resyncErr := hydrateCB.Execute(func() error { - Send(component, buildResyncPayload(mismatches)) - return nil - }) - if resyncErr != nil { - log.Printf("hostclient: hydration circuit open, skipping resync for %s", component) +} + +func hostSignalWriteHook() func(component, name string) { + mu.RLock() + defer mu.RUnlock() + return beforeHostSignalWrite +} + +// liveBinding returns the binding still registered under token. A token that no +// longer matches means the registration was released or replaced, so the caller +// owns nothing to update. +func liveBinding(component string, token uint64) (componentBinding, bool) { + mu.RLock() + defer mu.RUnlock() + if bindingTokens[component] != token { + return componentBinding{}, false + } + binding, bound := bindings[component] + return binding, bound +} + +// hostComponentRoot resolves the exact root a binding owns. dom.ComponentRoot +// falls back to #app when the id matches nothing, which for host delivery would +// mean a frame addressed to an unmounted component writing into the application +// shell or into whatever mounted after it. A missing root is a frame to ignore. +// RegisterComponent is public and takes any id, so the id never reaches a +// selector unescaped: a quote or a bracket in it would otherwise throw a +// DOMException out of querySelector, or select a root the caller never owned. +func hostComponentRoot(id string) dom.Element { + if id == "" { + return dom.Element{Value: js.Null()} + } + if selector, ok := componentIDSelector(id); ok { + return dom.Doc().Query(selector) + } + return scanComponentRoot(id) +} + +// componentIDSelector builds the attribute selector for id and leaves the +// escaping rules to the browser. CSS.escape returns an identifier, which is +// what the value position of an attribute selector accepts. +func componentIDSelector(id string) (string, bool) { + css := js.Get("CSS") + if !css.Truthy() || css.Get("escape").Type() != js.TypeFunction { + return "", false } + return "[data-component-id=" + css.Call("escape", id).String() + "]", true +} + +// scanComponentRoot is the fallback for a runtime without CSS.escape: the +// candidates are selected on the bare attribute and compared on its value, so +// no part of the id is ever parsed as a selector. +func scanComponentRoot(id string) dom.Element { + roots := dom.Doc().QueryAll("[data-component-id]") + if !roots.Truthy() { + return dom.Element{Value: js.Null()} + } + for index := 0; index < roots.Length(); index++ { + candidate := roots.Index(index) + if candidate.Attr("data-component-id") == id { + return candidate + } + } + return dom.Element{Value: js.Null()} } func prepareInboundDelivery(remoteSession, control string) { @@ -461,19 +689,126 @@ func prepareInboundDelivery(remoteSession, control string) { deliveryMu.Unlock() } -// RegisterComponent binds a client component to a host component name. +// RegisterComponent binds a client component to a host component name. The +// binding lives until another registration replaces it. Use +// RegisterComponentOwned when the caller has to release the binding again, for +// instance because its component can unmount. func RegisterComponent(id, name string, vars []string) { + registerComponent(id, name, vars) +} + +// RegisterComponentOwned binds like RegisterComponent and returns an idempotent +// cleanup that owns the binding's lifecycle. Releasing removes the binding from +// reconnect hydration and tells the active host session to stop broadcasts for +// the component; once the cleanup returns, no frame still in flight and no +// later frame can update the root it owned or write into its host signals. A +// stale cleanup closure never removes a newer binding registered under the same +// name. The cleanup is safe to call from a host signal setter, which is where a +// component that unmounts on an update ends up calling it. +func RegisterComponentOwned(id, name string, vars []string) func() { + token := registerComponent(id, name, vars) + var once sync.Once + return func() { + once.Do(func() { releaseComponent(name, token) }) + } +} + +// registerComponent installs the binding and returns the token identifying this +// registration. Delivery validates against it, so registering also invalidates +// the cleanup of the binding it replaced. +func registerComponent(id, name string, vars []string) uint64 { + token := bindingSequence.Add(1) + bindingMu.Lock() mu.Lock() - bindings[name] = componentBinding{id: id, vars: vars} + previous := bindings[name] + bindings[name] = componentBinding{id: id, vars: vars, gate: &deliveryGate{}} + bindingTokens[name] = token current := conn - if current == nil { - pending = append(pending, message{name: name, payload: map[string]any{"init": true}}) - } + recordPendingControl(name, map[string]any{"init": true}, current) mu.Unlock() + // The registration this one replaces stops being deliverable here. Its + // frames already fail the token check; closing its gate stops one that was + // snapshotted before the replacement from writing a signal on its way out. + previous.gate.close() + bindingMu.Unlock() connect() if current != nil { sendMessage(current, message{name: name, payload: map[string]any{"init": true}}) } + return token +} + +func releaseComponent(name string, token uint64) { + bindingMu.Lock() + mu.Lock() + if bindingTokens[name] != token { + mu.Unlock() + bindingMu.Unlock() + return + } + binding := bindings[name] + delete(bindings, name) + delete(bindingTokens, name) + current := conn + recordPendingControl(name, map[string]any{"unsubscribe": true}, current) + mu.Unlock() + // Closing the gate in the section that dropped the registration means no + // delivery can read it open again afterwards. + binding.gate.close() + // Every delivery for this binding is now either finished or bound to fail + // its token check, so the unsubscribe can go out without the lock. + bindingMu.Unlock() + + // The root is safe by now, the signals are not: a frame that read the gate + // open before it closed may be committing a write. Waiting that write out, + // with no binding lock held, is what makes the cleanup final. + fenceHostSignalWrites(binding.id) + + if current != nil { + sendMessage(current, message{name: name, payload: map[string]any{"unsubscribe": true}}) + } +} + +// recordPendingControl keeps the reconnect queue holding one registration +// control per host component, the latest desired state: a registration +// supersedes a queued unsubscribe and a release supersedes a queued init, so +// route churn while offline cannot retain one message per cycle. The control +// the caller just decided on is queued only when there is no connection to send +// it on; a superseded one is dropped either way, since sending the new state +// makes replaying the old one wrong. +// +// The new state takes the queue position of the control it supersedes rather +// than the tail: everything else in the queue, the controls of other names and +// this name's own messages, keeps the order it was queued in, so a reconnect +// replays the component's init before the command that assumed it. +// It must be called with mu held. +func recordPendingControl(name string, payload map[string]any, current *hostConn) { + replaced := false + filtered := pending[:0] + for _, queued := range pending { + if queued.name != name || !isRegistrationControl(queued.payload) { + filtered = append(filtered, queued) + continue + } + if current != nil || replaced { + continue + } + queued.payload = payload + filtered = append(filtered, queued) + replaced = true + } + pending = filtered + if current == nil && !replaced { + pending = append(pending, message{name: name, payload: payload}) + } +} + +func isRegistrationControl(payload any) bool { + values, ok := payload.(map[string]any) + if !ok { + return false + } + return values["init"] == true || values["unsubscribe"] == true } // EnableSendDedup turns on payload-based deduplication for the named channel: @@ -533,9 +868,7 @@ func RegisterHandler(name string, h func(map[string]any)) func() { handlers[name] = h handlerTokens[name] = token current := conn - if current == nil { - pending = append(pending, message{name: name, payload: map[string]any{"init": true}}) - } + recordPendingControl(name, map[string]any{"init": true}, current) mu.Unlock() connect() if current != nil { @@ -551,34 +884,17 @@ func RegisterHandler(name string, h func(map[string]any)) func() { } delete(handlers, name) delete(handlerTokens, name) - filtered := pending[:0] - for _, queued := range pending { - if queued.name == name && isInitPayload(queued.payload) { - continue - } - filtered = append(filtered, queued) - } - pending = filtered current := conn + recordPendingControl(name, map[string]any{"unsubscribe": true}, current) mu.Unlock() - unsubscribe := message{name: name, payload: map[string]any{"unsubscribe": true}} if current != nil { - sendMessage(current, unsubscribe) - return + sendMessage(current, message{name: name, payload: map[string]any{"unsubscribe": true}}) } - mu.Lock() - pending = append(pending, unsubscribe) - mu.Unlock() }) } } -func isInitPayload(payload any) bool { - values, ok := payload.(map[string]any) - return ok && values["init"] == true -} - // SessionID returns the current SSC session ID. func SessionID() string { sessionMu.RLock() diff --git a/router/data.go b/router/data.go index 25e5896f..a3b8c808 100644 --- a/router/data.go +++ b/router/data.go @@ -21,8 +21,149 @@ var ( ErrRouteComponent = errors.New("router: route has no component") // ErrRedirectLoop reports too many consecutive route redirects. ErrRedirectLoop = errors.New("router: redirect loop") + // ErrNavigationForbidden reports a result guard that refused a destination + // outright, with no alternative route to send the caller to. + ErrNavigationForbidden = errors.New("router: navigation forbidden") + // ErrInvalidGuardResult reports a result guard that returned an unknown + // action or a redirect without a usable destination. + ErrInvalidGuardResult = errors.New("router: invalid guard result") ) +// GuardAction is the outcome a result guard asks the router to apply. +type GuardAction uint8 + +const ( + // GuardAllow continues with the next guard and, eventually, the route. + GuardAllow GuardAction = iota + // GuardForbid stops navigation with ErrNavigationForbidden. + GuardForbid + // GuardRedirect navigates to another path and keeps a history entry for + // the current one. + GuardRedirect + // GuardReplace navigates to another path in place of the current history + // entry, which is what a login redirect wants. + GuardReplace +) + +// GuardResult is the decision a ResultGuard returns. Build it with Allow, +// Forbid, RedirectTo or ReplaceWith rather than by hand. +type GuardResult struct { + Action GuardAction + Path string +} + +// ResultGuard decides whether a route may be entered and, when it may not, +// where navigation goes instead. A result guard must not call Navigate or +// Replace itself: returning the destination lets the router apply it once, in +// the right order, without reentering an in-flight navigation. +type ResultGuard func(map[string]string) GuardResult + +// Allow lets navigation continue. +func Allow() GuardResult { return GuardResult{Action: GuardAllow} } + +// Forbid refuses navigation without a destination to fall back to. +func Forbid() GuardResult { return GuardResult{Action: GuardForbid} } + +// RedirectTo sends navigation to path and keeps the current history entry. +func RedirectTo(path string) GuardResult { + return GuardResult{Action: GuardRedirect, Path: path} +} + +// ReplaceWith sends navigation to path in place of the current history entry. +func ReplaceWith(path string) GuardResult { + return GuardResult{Action: GuardReplace, Path: path} +} + +// guardEntry is one guard in evaluation order. Exactly one of its fields is +// set: legacy guards keep their allow-or-block contract, result guards carry +// the typed one. +type guardEntry struct { + legacy Guard + result ResultGuard +} + +// routeGuardEntries orders the guards of a single route: its legacy guards +// first, then its result guards. Nesting keeps parents before children, so the +// whole chain is deterministic. +func routeGuardEntries(legacy []Guard, results []ResultGuard) []guardEntry { + if len(legacy) == 0 && len(results) == 0 { + return nil + } + entries := make([]guardEntry, 0, len(legacy)+len(results)) + for _, guard := range legacy { + entries = append(entries, guardEntry{legacy: guard}) + } + for _, guard := range results { + entries = append(entries, guardEntry{result: guard}) + } + return entries +} + +// joinGuards concatenates a parent chain with its child chain without writing +// into the parent's own slice. +func joinGuards(parent, child []guardEntry) []guardEntry { + if len(parent) == 0 { + return child + } + if len(child) == 0 { + return parent + } + joined := make([]guardEntry, 0, len(parent)+len(child)) + joined = append(joined, parent...) + return append(joined, child...) +} + +// runGuards evaluates the chain and stops at the first guard that does not +// allow navigation. The returned result is meaningful only when err is nil: a +// GuardRedirect or GuardReplace action carries the validated destination, and +// GuardAllow means the route may be loaded. +func runGuards(entries []guardEntry, params map[string]string) (GuardResult, error) { + for _, entry := range entries { + if entry.legacy != nil { + if entry.legacy(params) { + continue + } + return GuardResult{}, ErrNavigationBlocked + } + if entry.result == nil { + continue + } + result := entry.result(params) + switch result.Action { + case GuardAllow: + continue + case GuardForbid: + return GuardResult{}, ErrNavigationForbidden + case GuardRedirect, GuardReplace: + destination, err := guardRedirectTarget(result.Path) + if err != nil { + return GuardResult{}, err + } + result.Path = destination + return result, nil + default: + return GuardResult{}, ErrInvalidGuardResult + } + } + return GuardResult{}, nil +} + +// guardRedirectTarget validates a guard destination. It must be a rooted +// application path: an empty, relative or scheme-relative target would send +// navigation somewhere the router cannot resolve, so it fails closed. +func guardRedirectTarget(path string) (string, error) { + destination := strings.TrimSpace(path) + if destination == "" || !strings.HasPrefix(destination, "/") { + return "", ErrInvalidGuardResult + } + // "//host" and "/\host" leave the application: browsers read them as + // protocol-relative URLs, which is an open redirect, not a route. + if strings.HasPrefix(destination, "//") || strings.HasPrefix(destination, `/\`) { + return "", ErrInvalidGuardResult + } + return destination, nil +} + // LoadContext describes the destination passed to a route loader. type LoadContext struct { Path string diff --git a/router/router.go b/router/router.go index 57b2a80c..6f6fa33c 100644 --- a/router/router.go +++ b/router/router.go @@ -36,10 +36,13 @@ type Route struct { Name string Component any Guards []Guard - Children []Route - Loader Loader - Redirect string - Meta map[string]any + // ResultGuards run after Guards for this route and can redirect or forbid + // instead of only blocking. + ResultGuards []ResultGuard + Children []Route + Loader Loader + Redirect string + Meta map[string]any } // Singleton wraps a pre-created View into a Route.Component value. @@ -59,7 +62,7 @@ type route struct { loader func() core.Component singleton bool children []route - guards []Guard + guards []guardEntry dataLoader Loader redirect string meta map[string]any @@ -183,7 +186,7 @@ func buildRouteAt(r Route, parent string) route { matchNames: matchNames, loader: loader, singleton: singleton, - guards: r.Guards, + guards: routeGuardEntries(r.Guards, r.ResultGuards), dataLoader: r.Loader, redirect: r.Redirect, meta: cloneMeta(r.Meta), @@ -248,13 +251,13 @@ type routeParamReceiver interface { SetRouteParams(map[string]string) } -func matchRoute(routes []route, path string) (*route, []Guard, map[string]string) { +func matchRoute(routes []route, path string) (*route, []guardEntry, map[string]string) { for i := range routes { r := &routes[i] matches := r.regex.FindStringSubmatch(path) if matches == nil { if child, guards, params := matchRoute(r.children, path); child != nil { - return child, append(r.guards, guards...), params + return child, joinGuards(r.guards, guards), params } continue } @@ -265,7 +268,7 @@ func matchRoute(routes []route, path string) (*route, []Guard, map[string]string } } if child, guards, childParams := matchRoute(r.children, path); child != nil { - return child, append(r.guards, guards...), childParams + return child, joinGuards(r.guards, guards), childParams } matchedPath := strings.TrimSuffix(matches[0], "/") if (r.loader != nil || r.redirect != "") && matchedPath == strings.TrimSuffix(path, "/") { @@ -384,13 +387,30 @@ func navigateImpl(parent context.Context, fullPath string, history historyMode) for key, value := range queryParams { params[key] = value } - for _, g := range guards { - if !g(params) { + // Guards decide before anything commits: no loader, no component, no + // unmount, no history entry runs for a destination they refuse. + guardResult, guardErr := runGuards(guards, params) + if guardErr != nil { + if guardErr == ErrNavigationBlocked { if currentComponent == nil && path != "/" { navigate("/", history) } - return ErrNavigationBlocked + return guardErr } + failNavigation(guardErr) + return guardErr + } + if guardResult.Action != GuardAllow { + redirectCtx, err := nextRedirectContext(parent) + if err != nil { + failNavigation(err) + return err + } + mode := historyPush + if guardResult.Action == GuardReplace { + mode = historyReplace + } + return navigateImpl(redirectCtx, guardResult.Path, mode) } if r.redirect != "" { diff --git a/router/router_guard_result_test.go b/router/router_guard_result_test.go new file mode 100644 index 00000000..f785e9be --- /dev/null +++ b/router/router_guard_result_test.go @@ -0,0 +1,288 @@ +package router + +import ( + "context" + "errors" + "reflect" + "testing" + + "github.com/rfwlab/rfw/v2/core" +) + +// guardedRoute registers a protected route whose loader and component factory +// must never run when a guard refuses the destination. +func guardedRoute(t *testing.T, path string, guards []ResultGuard, legacy ...Guard) *int { + t.Helper() + created := 0 + RegisterRoute(Route{ + Path: path, + Guards: legacy, + ResultGuards: guards, + Component: func() core.Component { + created++ + return &recordComponent{name: "protected"} + }, + Loader: func(context.Context, LoadContext) (any, error) { + t.Error("route loader ran for a guarded destination") + return nil, nil + }, + }) + return &created +} + +func TestResultGuardForbidsWithoutCommitting(t *testing.T) { + resetRouter(t) + created := guardedRoute(t, "/vault", []ResultGuard{func(map[string]string) GuardResult { + return Forbid() + }}) + + err := NavigateContext(context.Background(), "/vault") + + if !errors.Is(err, ErrNavigationForbidden) { + t.Fatalf("expected a forbidden navigation, got %v", err) + } + if *created != 0 { + t.Fatalf("protected component was created %d times", *created) + } + if CurrentComponent() != nil { + t.Fatalf("forbidden navigation committed %#v", CurrentComponent()) + } + if Status().Get() != NavigationError || !errors.Is(Error().Get(), ErrNavigationForbidden) { + t.Fatalf("navigation state: status=%s error=%v", Status().Get(), Error().Get()) + } +} + +// Guards run parent before child, legacy before typed, and stop at the first +// one that does not allow navigation. +func TestResultGuardsComposeParentBeforeChild(t *testing.T) { + resetRouter(t) + var order []string + record := func(name string) { order = append(order, name) } + RegisterRoute(Route{ + Path: "/app", + Guards: []Guard{func(map[string]string) bool { record("parent-legacy"); return true }}, + ResultGuards: []ResultGuard{func(map[string]string) GuardResult { + record("parent-result") + return Allow() + }}, + Children: []Route{{ + Path: "settings", + Guards: []Guard{func(map[string]string) bool { record("child-legacy"); return true }}, + Component: func() core.Component { return &recordComponent{name: "settings"} }, + ResultGuards: []ResultGuard{ + func(map[string]string) GuardResult { record("child-result"); return Forbid() }, + func(map[string]string) GuardResult { record("unreachable"); return Allow() }, + }, + }}, + }) + + if err := NavigateContext(context.Background(), "/app/settings"); !errors.Is(err, ErrNavigationForbidden) { + t.Fatalf("expected a forbidden navigation, got %v", err) + } + + want := []string{"parent-legacy", "parent-result", "child-legacy", "child-result"} + if !reflect.DeepEqual(order, want) { + t.Fatalf("guard order = %v, want %v", order, want) + } +} + +// A parent guard that refuses stops the chain before any child guard runs. +func TestParentResultGuardStopsChildGuards(t *testing.T) { + resetRouter(t) + childRan := false + RegisterRoute(Route{ + Path: "/admin", + ResultGuards: []ResultGuard{func(map[string]string) GuardResult { + return ReplaceWith("/login") + }}, + Children: []Route{{ + Path: "users", + Component: func() core.Component { return &recordComponent{name: "users"} }, + ResultGuards: []ResultGuard{func(map[string]string) GuardResult { + childRan = true + return Allow() + }}, + }}, + }) + RegisterRoute(Route{Path: "/login", Component: func() core.Component { return &recordComponent{name: "login"} }}) + + if err := NavigateContext(context.Background(), "/admin/users"); err != nil { + t.Fatalf("guard redirect: %v", err) + } + if childRan { + t.Fatal("a child guard ran after the parent refused") + } + if got := mustRecord(t, CurrentComponent()).name; got != "login" { + t.Fatalf("current component = %q, want login", got) + } +} + +func TestResultGuardRedirectsToLogin(t *testing.T) { + resetRouter(t) + RegisterRoute(Route{Path: "/login", Component: func() core.Component { return &recordComponent{name: "login"} }}) + created := guardedRoute(t, "/account", []ResultGuard{func(map[string]string) GuardResult { + return ReplaceWith("/login") + }}) + + if err := NavigateContext(context.Background(), "/account"); err != nil { + t.Fatalf("guard redirect: %v", err) + } + if *created != 0 { + t.Fatalf("protected component was created %d times", *created) + } + if got := mustRecord(t, CurrentComponent()).name; got != "login" { + t.Fatalf("current component = %q, want login", got) + } + if ActivePath().Get() != "/login" { + t.Fatalf("active path = %q, want /login", ActivePath().Get()) + } + if Status().Get() != NavigationReady { + t.Fatalf("navigation status = %s, want ready", Status().Get()) + } +} + +func TestResultGuardRedirectKeepsRouteParameters(t *testing.T) { + resetRouter(t) + var seen map[string]string + RegisterRoute(Route{Path: "/denied", Component: func() core.Component { return &recordComponent{name: "denied"} }}) + RegisterRoute(Route{ + Path: "/orders/:id", + Component: func() core.Component { return &recordComponent{name: "order"} }, + ResultGuards: []ResultGuard{func(params map[string]string) GuardResult { + seen = params + return RedirectTo("/denied") + }}, + }) + + if err := NavigateContext(context.Background(), "/orders/17?ref=mail"); err != nil { + t.Fatalf("guard redirect: %v", err) + } + if seen["id"] != "17" || seen["ref"] != "mail" { + t.Fatalf("guard parameters = %#v", seen) + } + if got := mustRecord(t, CurrentComponent()).name; got != "denied" { + t.Fatalf("current component = %q, want denied", got) + } +} + +func TestInvalidGuardResultFailsClosed(t *testing.T) { + cases := map[string]GuardResult{ + "empty": RedirectTo(""), + "blank": ReplaceWith(" "), + "relative": RedirectTo("login"), + "scheme relative": RedirectTo("//evil.example/login"), + "unknown action": {Action: GuardAction(42), Path: "/login"}, + } + for name, result := range cases { + t.Run(name, func(t *testing.T) { + resetRouter(t) + decision := result + created := guardedRoute(t, "/secret", []ResultGuard{func(map[string]string) GuardResult { + return decision + }}) + + err := NavigateContext(context.Background(), "/secret") + + if !errors.Is(err, ErrInvalidGuardResult) { + t.Fatalf("expected an invalid guard result, got %v", err) + } + if *created != 0 || CurrentComponent() != nil { + t.Fatalf("invalid guard result committed a route: created=%d component=%#v", *created, CurrentComponent()) + } + }) + } +} + +func TestResultGuardRedirectLoopFails(t *testing.T) { + resetRouter(t) + RegisterRoute(Route{ + Path: "/ping", + Component: func() core.Component { return &recordComponent{name: "ping"} }, + ResultGuards: []ResultGuard{func(map[string]string) GuardResult { return RedirectTo("/pong") }}, + }) + RegisterRoute(Route{ + Path: "/pong", + Component: func() core.Component { return &recordComponent{name: "pong"} }, + ResultGuards: []ResultGuard{func(map[string]string) GuardResult { return ReplaceWith("/ping") }}, + }) + + if err := NavigateContext(context.Background(), "/ping"); !errors.Is(err, ErrRedirectLoop) { + t.Fatalf("expected a redirect loop error, got %v", err) + } + if CurrentComponent() != nil { + t.Fatalf("redirect loop committed %#v", CurrentComponent()) + } +} + +// Legacy bool guards keep their contract: a false guard blocks with +// ErrNavigationBlocked and never reaches the typed guards behind it. +func TestLegacyGuardStillBlocksBeforeResultGuards(t *testing.T) { + resetRouter(t) + resultRan := false + RegisterRoute(Route{Path: "/", Component: func() core.Component { return &recordComponent{name: "root"} }}) + RegisterRoute(Route{ + Path: "/legacy", + Component: func() core.Component { return &recordComponent{name: "legacy"} }, + Guards: []Guard{func(map[string]string) bool { return false }}, + ResultGuards: []ResultGuard{func(map[string]string) GuardResult { + resultRan = true + return Allow() + }}, + }) + + if err := NavigateContext(context.Background(), "/legacy"); !errors.Is(err, ErrNavigationBlocked) { + t.Fatalf("expected a blocked navigation, got %v", err) + } + if resultRan { + t.Fatal("a result guard ran after a legacy guard blocked") + } +} + +// A guard that allows leaves the route untouched: loader, component and route +// state commit exactly as they do without guards. +func TestAllowingResultGuardCommitsRoute(t *testing.T) { + resetRouter(t) + loaded := false + RegisterRoute(Route{ + Path: "/open", + Component: func() core.Component { return &recordComponent{name: "open"} }, + ResultGuards: []ResultGuard{func(map[string]string) GuardResult { return Allow() }}, + Loader: func(context.Context, LoadContext) (any, error) { + loaded = true + return nil, nil + }, + }) + + if err := NavigateContext(context.Background(), "/open"); err != nil { + t.Fatalf("navigate: %v", err) + } + if !loaded { + t.Fatal("an allowed route did not run its loader") + } + if got := mustRecord(t, CurrentComponent()).name; got != "open" { + t.Fatalf("current component = %q, want open", got) + } +} + +// A cancelled navigation never reaches its guards. +func TestCancelledNavigationSkipsResultGuards(t *testing.T) { + resetRouter(t) + guardRan := false + RegisterRoute(Route{ + Path: "/cancelled-guard", + Component: func() core.Component { return &recordComponent{name: "cancelled"} }, + ResultGuards: []ResultGuard{func(map[string]string) GuardResult { + guardRan = true + return Allow() + }}, + }) + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + if err := NavigateContext(ctx, "/cancelled-guard"); !errors.Is(err, context.Canceled) { + t.Fatalf("expected a cancelled navigation, got %v", err) + } + if guardRan { + t.Fatal("a guard ran for a cancelled navigation") + } +} diff --git a/router/router_history_test.go b/router/router_history_test.go index 3f3f52ea..be970454 100644 --- a/router/router_history_test.go +++ b/router/router_history_test.go @@ -4,8 +4,10 @@ package router import ( "testing" + "time" "github.com/rfwlab/rfw/v2/core" + events "github.com/rfwlab/rfw/v2/events" "github.com/rfwlab/rfw/v2/js" ) @@ -35,6 +37,129 @@ func TestReplaceKeepsHistoryLength(t *testing.T) { } } +// restoreHistoryPath puts the page back on the path the test found, so the +// suite that follows starts where it expects to. +func restoreHistoryPath(t *testing.T) { + t.Helper() + original := js.Location().Get("pathname").String() + + js.Location().Get("search").String() + + js.Location().Get("hash").String() + t.Cleanup(func() { + js.History().Call("replaceState", nil, "", original) + Reset() + }) +} + +// travelHistory triggers a browser history move and waits for the popstate +// that carries the new URL. +func travelHistory(t *testing.T, direction string) string { + t.Helper() + popstate, stop := events.Listen("popstate", js.Window()) + defer stop() + js.History().Call(direction) + select { + case <-popstate: + case <-time.After(2 * time.Second): + t.Fatalf("history %s did not fire popstate", direction) + } + return js.Location().Get("pathname").String() +} + +// A login redirect replaces the entry it was serving, so the protected path +// leaves no history entry to go back to. +func TestGuardReplaceLeavesNoHistoryEntry(t *testing.T) { + Reset() + restoreHistoryPath(t) + + RegisterRoute(Route{Path: "/guard-base", Component: func() core.Component { return routeComponent{} }}) + RegisterRoute(Route{Path: "/guard-entry", Component: func() core.Component { return routeComponent{} }}) + RegisterRoute(Route{Path: "/guard-login", Component: func() core.Component { return routeComponent{} }}) + RegisterRoute(Route{ + Path: "/guard-account", + Component: func() core.Component { return routeComponent{} }, + ResultGuards: []ResultGuard{func(map[string]string) GuardResult { return ReplaceWith("/guard-login") }}, + }) + + Navigate("/guard-base") + Navigate("/guard-entry") + before := js.History().Get("length").Int() + + Navigate("/guard-account") + + if got := js.Location().Get("pathname").String(); got != "/guard-login" { + t.Fatalf("path after the login redirect = %q", got) + } + if got := js.History().Get("length").Int(); got != before { + t.Fatalf("history length changed from %d to %d", before, got) + } + if got := travelHistory(t, "back"); got != "/guard-base" { + t.Fatalf("back landed on %q, want /guard-base", got) + } + if got := travelHistory(t, "forward"); got != "/guard-login" { + t.Fatalf("forward landed on %q, want /guard-login", got) + } +} + +// A push redirect keeps the entry it came from, so back returns to it. +func TestGuardRedirectPushesHistoryEntry(t *testing.T) { + Reset() + restoreHistoryPath(t) + + RegisterRoute(Route{Path: "/guard-origin", Component: func() core.Component { return routeComponent{} }}) + RegisterRoute(Route{Path: "/guard-offers", Component: func() core.Component { return routeComponent{} }}) + RegisterRoute(Route{ + Path: "/guard-promo", + Component: func() core.Component { return routeComponent{} }, + ResultGuards: []ResultGuard{func(map[string]string) GuardResult { return RedirectTo("/guard-offers") }}, + }) + + Navigate("/guard-origin") + before := js.History().Get("length").Int() + + Navigate("/guard-promo") + + if got := js.Location().Get("pathname").String(); got != "/guard-offers" { + t.Fatalf("path after the guard redirect = %q", got) + } + if got := js.History().Get("length").Int(); got != before+1 { + t.Fatalf("history length = %d, want %d", got, before+1) + } + if got := travelHistory(t, "back"); got != "/guard-origin" { + t.Fatalf("back landed on %q, want /guard-origin", got) + } + if got := travelHistory(t, "forward"); got != "/guard-offers" { + t.Fatalf("forward landed on %q, want /guard-offers", got) + } +} + +// Landing directly on a protected path, the way a deep link does, ends on the +// login route without committing the protected one. +func TestGuardReplaceOnDeepLink(t *testing.T) { + Reset() + restoreHistoryPath(t) + + created := 0 + RegisterRoute(Route{Path: "/deep-login", Component: func() core.Component { return routeComponent{} }}) + RegisterRoute(Route{ + Path: "/deep-account", + Component: func() core.Component { + created++ + return routeComponent{} + }, + ResultGuards: []ResultGuard{func(map[string]string) GuardResult { return ReplaceWith("/deep-login") }}, + }) + + js.History().Call("pushState", nil, "", "/deep-account") + navigate("/deep-account", historyNone) + + if got := js.Location().Get("pathname").String(); got != "/deep-login" { + t.Fatalf("deep link path = %q, want /deep-login", got) + } + if created != 0 { + t.Fatalf("protected component was created %d times", created) + } +} + func TestHistoryNoneGuardFallbackDoesNotPush(t *testing.T) { Reset() t.Cleanup(Reset) diff --git a/router/router_stub.go b/router/router_stub.go index eac37373..d2eff63e 100644 --- a/router/router_stub.go +++ b/router/router_stub.go @@ -22,10 +22,13 @@ type Route struct { Name string Component any Guards []Guard - Children []Route - Loader Loader - Redirect string - Meta map[string]any + // ResultGuards run after Guards for this route and can redirect or forbid + // instead of only blocking. + ResultGuards []ResultGuard + Children []Route + Loader Loader + Redirect string + Meta map[string]any } // Singleton marks a view instance for reuse between navigations. @@ -44,7 +47,7 @@ type route struct { loader func() core.Component singleton bool children []route - guards []Guard + guards []guardEntry dataLoader Loader redirect string meta map[string]any @@ -154,7 +157,7 @@ func buildRouteAt(r Route, parent string) route { matchNames: matchNames, loader: loader, singleton: singleton, - guards: r.Guards, + guards: routeGuardEntries(r.Guards, r.ResultGuards), dataLoader: r.Loader, redirect: r.Redirect, meta: cloneMeta(r.Meta), @@ -222,13 +225,13 @@ type routeParamHandler interface { OnParams(map[string]string) } -func matchRoute(routes []route, path string) (*route, []Guard, map[string]string) { +func matchRoute(routes []route, path string) (*route, []guardEntry, map[string]string) { for i := range routes { r := &routes[i] matches := r.regex.FindStringSubmatch(path) if matches == nil { if child, guards, params := matchRoute(r.children, path); child != nil { - return child, append(r.guards, guards...), params + return child, joinGuards(r.guards, guards), params } continue } @@ -239,7 +242,7 @@ func matchRoute(routes []route, path string) (*route, []Guard, map[string]string } } if child, guards, childParams := matchRoute(r.children, path); child != nil { - return child, append(r.guards, guards...), childParams + return child, joinGuards(r.guards, guards), childParams } matchedPath := strings.TrimSuffix(matches[0], "/") if (r.loader != nil || r.redirect != "") && matchedPath == strings.TrimSuffix(path, "/") { @@ -304,13 +307,27 @@ func NavigateContext(parent context.Context, fullPath string) error { for key, value := range queryParams { params[key] = value } - for _, g := range guards { - if !g(params) { + // Guards decide before anything commits: no loader, no component and no + // route state change runs for a destination they refuse. The stub keeps no + // browser history, so a push and a replace redirect both navigate. + guardResult, guardErr := runGuards(guards, params) + if guardErr != nil { + if guardErr == ErrNavigationBlocked { if currentComponent == nil && path != "/" { Navigate("/") } - return ErrNavigationBlocked + return guardErr } + failNavigation(guardErr) + return guardErr + } + if guardResult.Action != GuardAllow { + redirectCtx, err := nextRedirectContext(parent) + if err != nil { + failNavigation(err) + return err + } + return NavigateContext(redirectCtx, guardResult.Path) } if r.redirect != "" { diff --git a/ssc/server_test.go b/ssc/server_test.go index a7a4e7fe..048cabd4 100644 --- a/ssc/server_test.go +++ b/ssc/server_test.go @@ -5,6 +5,7 @@ package ssc import ( "context" "encoding/json" + "errors" "net/http" "net/http/httptest" "os" @@ -258,6 +259,97 @@ func TestSSCServerResumesAtSessionLimit(t *testing.T) { } } +// The ssc handler opens sessions through the same runtime as the bundled host, +// so a resume authorizer configured on the server refuses a token presented by +// another identity there too. +func TestSSCServerRefusesUnauthorizedResume(t *testing.T) { + const identityHeader = "X-Test-User" + type request struct{} + const action = "test.ssc.resume.identity" + if err := host.RegisterAction(action, func(_ context.Context, session *host.Session, _ request) (string, error) { + user, _ := session.ContextGet("user") + text, _ := user.(string) + return text, nil + }); err != nil { + t.Fatalf("register action: %v", err) + } + + server := httptest.NewServer(NewSSCServer(":0", t.TempDir(), + host.WithSSCLimits(host.SSCLimits{ResumeTTL: 5 * time.Second, ReplayMessages: 8}), + host.WithSSCSessionInitializer(func(r *http.Request, session *host.Session) error { + session.ContextSet("user", r.Header.Get(identityHeader)) + return nil + }), + host.WithSSCResumeAuthorizer(func(r *http.Request, session *host.Session) error { + user, _ := session.ContextGet("user") + if user != r.Header.Get(identityHeader) { + return errors.New("resume denied") + } + return nil + }), + ).Mux) + defer server.Close() + + dial := func(user string) *websocket.Conn { + config, err := websocket.NewConfig("ws"+strings.TrimPrefix(server.URL, "http")+"/ws", server.URL) + if err != nil { + t.Fatalf("websocket config: %v", err) + } + config.Header.Set(identityHeader, user) + socket, err := websocket.DialConfig(config) + if err != nil { + t.Fatalf("dial websocket: %v", err) + } + return socket + } + send := func(socket *websocket.Conn, msg host.Inbound) { + t.Helper() + if err := websocket.JSON.Send(socket, msg); err != nil { + t.Fatalf("send %#v: %v", msg, err) + } + } + receive := func(socket *websocket.Conn) host.Outbound { + t.Helper() + if err := socket.SetReadDeadline(time.Now().Add(2 * time.Second)); err != nil { + t.Fatalf("set read deadline: %v", err) + } + var out host.Outbound + if err := websocket.JSON.Receive(socket, &out); err != nil { + t.Fatalf("receive: %v", err) + } + return out + } + + ownerSocket := dial("alice") + send(ownerSocket, host.Inbound{Action: action, ID: "alice", Sequence: 1}) + owner := receive(ownerSocket) + if owner.Payload != "alice" || owner.ResumeToken == "" { + t.Fatalf("session identity was not bound: %#v", owner) + } + closeTestResource(t, ownerSocket) + + intruderSocket := dial("mallory") + defer closeTestResource(t, intruderSocket) + send(intruderSocket, host.Inbound{ + Action: action, + ID: "mallory", + Sequence: 1, + Ack: owner.Sequence, + ResumeToken: owner.ResumeToken, + }) + rejected := receive(intruderSocket) + if rejected.Control != "resume_rejected" || rejected.Session == owner.Session { + t.Fatalf("foreign token was not refused: %#v", rejected) + } + response := receive(intruderSocket) + if response.Payload != "mallory" || response.Session == owner.Session { + t.Fatalf("refused client did not get its own session: %#v", response) + } + if session, ok := host.SessionByID(owner.Session); ok { + host.ReleaseSession(session) + } +} + // A browser client cannot send protocol ping frames, so hostclient probes // liveness with a control message and treats silence as a dead connection. // The ssc handler is a second loop over the same protocol: when only the diff --git a/state/signal.go b/state/signal.go index 66fa80fb..dfaea483 100644 --- a/state/signal.go +++ b/state/signal.go @@ -99,16 +99,50 @@ func (s *Signal[T]) Read() any { // so payloads that do not assert directly to T are converted through a // JSON round-trip into T. Payloads that cannot represent T are ignored. func (s *Signal[T]) SetFromHost(raw any) { + s.SetFromHostGated(raw, nil) +} + +// SetFromHostGated sets the signal value from an untyped host payload like +// SetFromHost, unless allow refuses the write. allow runs under the same lock +// as the store itself, so a caller that revoked the delivery elsewhere and then +// waited on HostWriteBarrier knows the refused frame never reached the value. +// It must therefore only read the caller's own revocation state: it runs on the +// hot path of every host update and must not block or call back into the +// signal. It reports whether the value was stored, which a payload that cannot +// represent T never is. +func (s *Signal[T]) SetFromHostGated(raw any, allow func() bool) bool { + if s == nil { + return false + } + v, ok := hostValue[T](raw) + if !ok { + return false + } + return s.set(v, allow) +} + +// HostWriteBarrier blocks until a gated host write already in flight has +// finished. A caller that closed its gate first can then be sure no write it +// had allowed is still on its way into the value. +func (s *Signal[T]) HostWriteBarrier() { if s == nil { return } + // Taking the value lock is the whole barrier: a gated write holds it across + // its permission check and its store, so once it is free that write has + // landed and every later one reads the revoked permission instead. + s.mu.Lock() + defer s.mu.Unlock() +} + +// hostValue converts an untyped host payload into T. The conversion happens on +// a local value so it never touches a stored one outside the lock. +func hostValue[T any](raw any) (T, bool) { if v, ok := raw.(T); ok { - s.Set(v) - return + return v, true } if val, ok := raw.(float64); ok { - // Fast paths for the common numeric targets. The conversion happens - // on a local value so it never touches s.value outside the lock. + // Fast paths for the common numeric targets. var conv T switch p := any(&conv).(type) { case *int: @@ -136,27 +170,26 @@ func (s *Signal[T]) SetFromHost(raw any) { case *float64: *p = val default: - s.setViaJSON(raw) - return + return valueFromJSON[T](raw) } - s.Set(conv) - return + return conv, true } - s.setViaJSON(raw) + return valueFromJSON[T](raw) } -// setViaJSON converts an arbitrary decoded payload into T by re-encoding it, +// valueFromJSON converts an arbitrary decoded payload into T by re-encoding it, // covering structs, slices and maps that arrive as map[string]any/[]any. -func (s *Signal[T]) setViaJSON(raw any) { +func valueFromJSON[T any](raw any) (T, bool) { + var zero T blob, err := json.Marshal(raw) if err != nil { - return + return zero, false } var v T if err := json.Unmarshal(blob, &v); err != nil { - return + return zero, false } - s.Set(v) + return v, true } // Set updates the signal's value and notifies dependent effects. Effects run @@ -165,7 +198,18 @@ func (s *Signal[T]) Set(v T) { if s == nil { return } + s.set(v, nil) +} + +// set stores v and notifies, unless allow refuses the write. allow is the only +// thing that runs under the lock beside the store, so a refusal and the write +// it beats are one atomic step for whoever else takes that lock. +func (s *Signal[T]) set(v T, allow func() bool) bool { s.mu.Lock() + if allow != nil && !allow() { + s.mu.Unlock() + return false + } s.value = v snapshot := make([]*effect, 0, len(s.subs)) for eff := range s.subs { @@ -176,6 +220,7 @@ func (s *Signal[T]) Set(v T) { scheduleEffect(eff) } s.notifyOnChange(v) + return true } // OnChange registers a callback that fires whenever the signal's value changes. From 90264c0ae87b0c1a2632b2e0f76331e6ea0bd633 Mon Sep 17 00:00:00 2001 From: Pietro di Caprio Date: Sun, 30 Aug 2026 04:08:41 +0200 Subject: [PATCH 2/6] fix: preserve history on guard redirects Treat declarative redirects reached through initial-load or popstate navigation as replacements, avoiding denied entries that bounce under Back/Forward. Clarify the lifecycle test name and document the browser-history exception. --- core/hosthook_test.go | 2 +- docs/articles/guide/routing.md | 4 +++- router/data.go | 6 ++++-- router/router.go | 6 +++++- router/router_history_test.go | 34 ++++++++++++++++++++++++++++++++++ 5 files changed, 47 insertions(+), 5 deletions(-) diff --git a/core/hosthook_test.go b/core/hosthook_test.go index f61d447b..51072634 100644 --- a/core/hosthook_test.go +++ b/core/hosthook_test.go @@ -145,7 +145,7 @@ func TestLegacyHostRegisterUnmountsWithoutCleanup(t *testing.T) { // A host component is bound once per mounted lifecycle: repeated renders reuse // the live binding instead of re-initializing the SSC feed. -func TestHostRegistrarBindsOncePerRender(t *testing.T) { +func TestHostRegistrarBindsOncePerMountedLifecycle(t *testing.T) { prev := hostRegisterComponent t.Cleanup(func() { hostRegisterComponent = prev }) registrations := 0 diff --git a/docs/articles/guide/routing.md b/docs/articles/guide/routing.md index d6085772..ef0b5fad 100644 --- a/docs/articles/guide/routing.md +++ b/docs/articles/guide/routing.md @@ -65,7 +65,9 @@ router.RegisterRoute(router.Route{ - `router.Allow()` continues to the next guard and then to the route. - `router.Forbid()` stops navigation with `router.ErrNavigationForbidden`. - `router.RedirectTo(path)` navigates elsewhere and keeps a history entry for - the current page. + the current page. On an initial load or a browser back/forward, where the + denied path is already the current entry, it replaces that entry instead, so + back does not return to the page the guard refused. - `router.ReplaceWith(path)` navigates elsewhere in place of the current entry, which is what a login redirect wants: back must not return to the page the user was denied. diff --git a/router/data.go b/router/data.go index a3b8c808..efcfc55b 100644 --- a/router/data.go +++ b/router/data.go @@ -38,7 +38,8 @@ const ( // GuardForbid stops navigation with ErrNavigationForbidden. GuardForbid // GuardRedirect navigates to another path and keeps a history entry for - // the current one. + // the current one, except on an initial load or a popstate, where the + // denied entry is replaced instead. GuardRedirect // GuardReplace navigates to another path in place of the current history // entry, which is what a login redirect wants. @@ -64,7 +65,8 @@ func Allow() GuardResult { return GuardResult{Action: GuardAllow} } // Forbid refuses navigation without a destination to fall back to. func Forbid() GuardResult { return GuardResult{Action: GuardForbid} } -// RedirectTo sends navigation to path and keeps the current history entry. +// RedirectTo sends navigation to path and keeps the current history entry, +// unless navigation came from an initial load or a popstate. func RedirectTo(path string) GuardResult { return GuardResult{Action: GuardRedirect, Path: path} } diff --git a/router/router.go b/router/router.go index 6f6fa33c..a666c8f9 100644 --- a/router/router.go +++ b/router/router.go @@ -406,8 +406,12 @@ func navigateImpl(parent context.Context, fullPath string, history historyMode) failNavigation(err) return err } + // A historyNone navigation (initial load or popstate) is already sitting + // on the denied entry, so the redirect replaces it whatever the action + // is: pushing would leave the refused path behind for back to return to, + // where the guard denies it again and bounces forward. mode := historyPush - if guardResult.Action == GuardReplace { + if guardResult.Action == GuardReplace || history == historyNone { mode = historyReplace } return navigateImpl(redirectCtx, guardResult.Path, mode) diff --git a/router/router_history_test.go b/router/router_history_test.go index be970454..588d5880 100644 --- a/router/router_history_test.go +++ b/router/router_history_test.go @@ -132,6 +132,40 @@ func TestGuardRedirectPushesHistoryEntry(t *testing.T) { } } +// A push redirect served on an initial load or a popstate replaces the denied +// entry instead: back and forward must not run through the refused path. +func TestGuardRedirectOnHistoryNoneReplacesEntry(t *testing.T) { + Reset() + restoreHistoryPath(t) + + RegisterRoute(Route{Path: "/none-origin", Component: func() core.Component { return routeComponent{} }}) + RegisterRoute(Route{Path: "/none-offers", Component: func() core.Component { return routeComponent{} }}) + RegisterRoute(Route{ + Path: "/none-promo", + Component: func() core.Component { return routeComponent{} }, + ResultGuards: []ResultGuard{func(map[string]string) GuardResult { return RedirectTo("/none-offers") }}, + }) + + Navigate("/none-origin") + js.History().Call("pushState", nil, "", "/none-promo") + before := js.History().Get("length").Int() + + navigate("/none-promo", historyNone) + + if got := js.Location().Get("pathname").String(); got != "/none-offers" { + t.Fatalf("path after the guard redirect = %q", got) + } + if got := js.History().Get("length").Int(); got != before { + t.Fatalf("history length changed from %d to %d", before, got) + } + if got := travelHistory(t, "back"); got != "/none-origin" { + t.Fatalf("back landed on %q, want /none-origin", got) + } + if got := travelHistory(t, "forward"); got != "/none-offers" { + t.Fatalf("forward landed on %q, want /none-offers", got) + } +} + // Landing directly on a protected path, the way a deep link does, ends on the // login route without committing the protected one. func TestGuardReplaceOnDeepLink(t *testing.T) { From 53f6dda7a14eee2579432cd89007bc2ecda575df Mon Sep 17 00:00:00 2001 From: Pietro di Caprio Date: Sun, 30 Aug 2026 04:11:05 +0200 Subject: [PATCH 3/6] test: satisfy WASM host signal lint --- hostclient/host_signal_delivery_wasm_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hostclient/host_signal_delivery_wasm_test.go b/hostclient/host_signal_delivery_wasm_test.go index 493affab..02c6ed04 100644 --- a/hostclient/host_signal_delivery_wasm_test.go +++ b/hostclient/host_signal_delivery_wasm_test.go @@ -80,7 +80,7 @@ func TestCleanupBetweenTheSignalCheckAndTheWriteWins(t *testing.T) { t.Cleanup(releaseLive) cleanupReturned := make(chan struct{}) - setHostSignalWriteHook(func(component, name string) { + setHostSignalWriteHook(func(component, _ string) { if component != "signal-ticker" { return } From 14918ec1c0c904f9dd611109b4166ac8f0c05106 Mon Sep 17 00:00:00 2001 From: Pietro di Caprio Date: Sun, 30 Aug 2026 06:32:55 +0200 Subject: [PATCH 4/6] fix: hand host-owned guards to the browser --- CHANGELOG.md | 19 +- docs/articles/guide/routing.md | 60 ++++ router/data.go | 97 ++++++- router/router.go | 15 + router/router_host_replace_native_test.go | 175 ++++++++++++ router/router_host_replace_test.go | 183 ++++++++++++ router/router_host_replace_wasm_test.go | 325 ++++++++++++++++++++++ router/router_stub.go | 8 + 8 files changed, 878 insertions(+), 4 deletions(-) create mode 100644 router/router_host_replace_native_test.go create mode 100644 router/router_host_replace_test.go create mode 100644 router/router_host_replace_wasm_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 600ab9a1..f306dc72 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,7 +26,24 @@ version and include migration notes. does not return to the denied page. `Route.Guards`, `router.Page` and the `Group` builder are unchanged; a route may declare both, and its bool guards run first. New errors: `router.ErrNavigationForbidden` and - `router.ErrInvalidGuardResult`. + `router.ErrInvalidGuardResult`. `RedirectTo` and `ReplaceWith` are SPA + navigation: their destination goes through the route matcher and never leaves + the application. +- `router.HostReplace(path)`, the result guard outcome for a page the host owns + rather than the SPA: a server-rendered login or session-expiry page. The + browser loads the path as a document with `location.replace`, so it replaces + the current history entry and back does not return to the refused route. The + guard chain ends there: no loader, component, unmount, history entry, DOM + write or route state commit runs for the protected route, and the page stays + as it is until the browser unloads it. The target must already be a rooted + same-origin path, validated as the guard returned it and never trimmed into + shape, against browser URL normalization (control characters, surrounding + whitespace, backslash authorities, scheme-relative and malformed targets fail + closed with `router.ErrInvalidGuardResult`); cross-origin handoff is + intentionally unsupported. Outside + browser builds `NavigateContext` returns a `*router.HostNavigationError` + carrying the target and wrapping the new `router.ErrHostNavigation`, and + commits nothing. - `core.SetHostRegistrar`, the cleanup-capable form of `core.SetHostRegister`. Both remain; the legacy hook registers without a release. - `hostclient.RegisterComponentOwned`, the cleanup-capable form of diff --git a/docs/articles/guide/routing.md b/docs/articles/guide/routing.md index ef0b5fad..3f7c6612 100644 --- a/docs/articles/guide/routing.md +++ b/docs/articles/guide/routing.md @@ -71,6 +71,13 @@ router.RegisterRoute(router.Route{ - `router.ReplaceWith(path)` navigates elsewhere in place of the current entry, which is what a login redirect wants: back must not return to the page the user was denied. +- `router.HostReplace(path)` leaves the application and lets the host serve the + path as a document. + +`RedirectTo` and `ReplaceWith` are SPA navigation: the destination goes through +the same route matcher as `router.Navigate`, so it must be a registered route, +and an unregistered one lands on the not-found handling. Neither ever leaves +the application. Guards run parent before child, and for each route its `Guards` run before its `ResultGuards`. The first guard that does not allow decides: the guards behind @@ -86,6 +93,59 @@ navigation while one is in flight. Return the destination instead and let the router apply it once. Outside browser builds the router keeps no history, so `RedirectTo` and `ReplaceWith` both navigate to the destination. +### Handing a route to the host + +Some pages belong to the server, not the SPA: a login form, a session-expiry +page, an SSO endpoint. `router.HostReplace(path)` hands the document to one: + +```go +ResultGuards: []router.ResultGuard{ + func(map[string]string) router.GuardResult { + if !session.Valid() { + return router.HostReplace("/login?next=%2Fadmin") + } + return router.Allow() + }, +}, +``` + +In the browser the guard chain ends there and the browser performs a real +navigation with `location.replace`. Nothing about the refused route is +committed first: no loader, no component, no unmount, no history entry, no DOM +write, and no route state change. The page the user is on stays exactly as it +is until the browser unloads it. Because the load replaces the current history +entry, back returns to the page before the protected one rather than to the +path the guard refused. + +The target must already be a rooted path on the same origin, and it is validated +before any handoff, exactly as the guard returned it: nothing is trimmed or +otherwise repaired first, so a string that is not a valid target is refused +instead of being turned into one. Anything else fails closed with +`router.ErrInvalidGuardResult` and nothing reaches the browser: another scheme, +a scheme-relative `//host`, a backslash authority such as `/\host`, any ASCII +control character (browsers strip tabs and newlines from a URL, which turns +`//host` into `//host`), a leading or trailing space, and a malformed +percent escape. A rooted path with a query and a fragment is accepted and is +handed over byte for byte. + +Cross-origin host handoff is intentionally unsupported: a guard cannot send the +document to another origin, and there is no variant of this API that would. A +guard stays a declaration of where navigation may go, with no side effect of its +own, so the router remains the only thing that acts on the decision. + +Outside browser builds there is no document to replace and the host page is not +a route the router can load, so `router.NavigateContext` returns a +`*router.HostNavigationError` carrying the target and wrapping +`router.ErrHostNavigation`. The current component, the active path and route +data are left untouched, and navigation status goes to `error`: + +```go +var host *router.HostNavigationError +if errors.As(err, &host) { + // host.Path is the path the browser build would have loaded +} +``` + ## Data loaders A loader completes before the new component is created and the previous page diff --git a/router/data.go b/router/data.go index efcfc55b..10119895 100644 --- a/router/data.go +++ b/router/data.go @@ -27,8 +27,28 @@ var ( // ErrInvalidGuardResult reports a result guard that returned an unknown // action or a redirect without a usable destination. ErrInvalidGuardResult = errors.New("router: invalid guard result") + // ErrHostNavigation reports a host handoff that the current build cannot + // perform. Browser builds replace the document instead of returning it. + ErrHostNavigation = errors.New("router: host navigation requested") ) +// HostNavigationError carries the host path a guard handed navigation to. +// Outside browser builds there is no document to replace, so the router +// reports the request rather than pretending the host page loaded. It wraps +// ErrHostNavigation, so errors.Is identifies it without a type assertion. +type HostNavigationError struct { + // Path is the validated rooted path the host was asked to serve. + Path string +} + +// Error reports the handoff and the path it targets. +func (e *HostNavigationError) Error() string { + return ErrHostNavigation.Error() + ": " + e.Path +} + +// Unwrap returns ErrHostNavigation, the sentinel callers match on. +func (e *HostNavigationError) Unwrap() error { return ErrHostNavigation } + // GuardAction is the outcome a result guard asks the router to apply. type GuardAction uint8 @@ -44,10 +64,13 @@ const ( // GuardReplace navigates to another path in place of the current history // entry, which is what a login redirect wants. GuardReplace + // GuardHostReplace leaves the application: the browser loads the path from + // the host, so the router matches no route and commits nothing. + GuardHostReplace ) // GuardResult is the decision a ResultGuard returns. Build it with Allow, -// Forbid, RedirectTo or ReplaceWith rather than by hand. +// Forbid, RedirectTo, ReplaceWith or HostReplace rather than by hand. type GuardResult struct { Action GuardAction Path string @@ -76,6 +99,17 @@ func ReplaceWith(path string) GuardResult { return GuardResult{Action: GuardReplace, Path: path} } +// HostReplace hands the document to a host-owned path on the same origin, the +// server-rendered login or session-expiry page an application does not route +// itself. The browser loads it as a real navigation and replaces the current +// history entry; RedirectTo and ReplaceWith stay inside the SPA and only +// target registered routes. The path must already be rooted and same origin as +// written, surrounding whitespace included: an external or ambiguous target +// fails closed with ErrInvalidGuardResult. +func HostReplace(path string) GuardResult { + return GuardResult{Action: GuardHostReplace, Path: path} +} + // guardEntry is one guard in evaluation order. Exactly one of its fields is // set: legacy guards keep their allow-or-block contract, result guards carry // the typed one. @@ -117,8 +151,8 @@ func joinGuards(parent, child []guardEntry) []guardEntry { // runGuards evaluates the chain and stops at the first guard that does not // allow navigation. The returned result is meaningful only when err is nil: a -// GuardRedirect or GuardReplace action carries the validated destination, and -// GuardAllow means the route may be loaded. +// GuardRedirect, GuardReplace or GuardHostReplace action carries the validated +// destination, and GuardAllow means the route may be loaded. func runGuards(entries []guardEntry, params map[string]string) (GuardResult, error) { for _, entry := range entries { if entry.legacy != nil { @@ -143,6 +177,13 @@ func runGuards(entries []guardEntry, params map[string]string) (GuardResult, err } result.Path = destination return result, nil + case GuardHostReplace: + destination, err := guardHostReplaceTarget(result.Path) + if err != nil { + return GuardResult{}, err + } + result.Path = destination + return result, nil default: return GuardResult{}, ErrInvalidGuardResult } @@ -166,6 +207,56 @@ func guardRedirectTarget(path string) (string, error) { return destination, nil } +// guardHostReplaceTarget validates a host handoff destination. This one is not +// matched against a route: the browser loads it, so what matters is the URL the +// browser ends up with after its own normalization. The string a guard returned +// is therefore checked as it is and never repaired first: trimming would accept +// " /login" by turning it into a different string, and the caller would have +// handed the browser something validation never saw. A target that is not +// already a rooted same-origin path fails closed, and an accepted one is +// returned byte for byte. +func guardHostReplaceTarget(path string) (string, error) { + if path == "" || !strings.HasPrefix(path, "/") { + return "", ErrInvalidGuardResult + } + // Browsers remove tabs and newlines from anywhere in a URL, so "//host" + // reaches the network as "//host", and they strip C0 characters and spaces + // from both ends before parsing. Refuse them rather than validate a string + // the browser will rewrite: a leading one already fails the rooted check + // above, and a trailing space is the only stripped byte left to catch. + if strings.ContainsFunc(path, isURLControl) { + return "", ErrInvalidGuardResult + } + if strings.HasSuffix(path, " ") { + return "", ErrInvalidGuardResult + } + // A backslash is read as a slash, which turns "/\host" into a + // protocol-relative URL pointing somewhere else entirely. + if strings.ContainsRune(path, '\\') { + return "", ErrInvalidGuardResult + } + if strings.HasPrefix(path, "//") { + return "", ErrInvalidGuardResult + } + parsed, err := url.Parse(path) + if err != nil { + return "", ErrInvalidGuardResult + } + if parsed.Scheme != "" || parsed.Opaque != "" || parsed.Host != "" || parsed.User != nil { + return "", ErrInvalidGuardResult + } + if !strings.HasPrefix(parsed.Path, "/") { + return "", ErrInvalidGuardResult + } + // Parse keeps the query raw, so a malformed escape there survives it. + if _, err := url.ParseQuery(parsed.RawQuery); err != nil { + return "", ErrInvalidGuardResult + } + return path, nil +} + +func isURLControl(r rune) bool { return r < 0x20 || r == 0x7f } + // LoadContext describes the destination passed to a route loader. type LoadContext struct { Path string diff --git a/router/router.go b/router/router.go index a666c8f9..e6ce5460 100644 --- a/router/router.go +++ b/router/router.go @@ -104,6 +104,13 @@ var NotFoundComponent any // unregistered route. It receives the requested path. var NotFoundCallback func(string) +// hostReplace hands the document to a host-owned path. It is a variable so +// browser tests can watch the handoff without unloading the page running them; +// production always performs the real navigation. +var hostReplace = func(path string) { + js.Location().Call("replace", path) +} + // Reset clears the router's registered routes and current component. // It is primarily intended for use in tests to ensure a clean state. func Reset() { @@ -400,6 +407,14 @@ func navigateImpl(parent context.Context, fullPath string, history historyMode) failNavigation(guardErr) return guardErr } + if guardResult.Action == GuardHostReplace { + // The host owns the destination, so the browser loads it as a document + // instead of the router matching a route for it. Nothing commits: the + // page the user is on stays exactly as it is until the browser unloads + // it, and navigation never reenters here. + hostReplace(guardResult.Path) + return nil + } if guardResult.Action != GuardAllow { redirectCtx, err := nextRedirectContext(parent) if err != nil { diff --git a/router/router_host_replace_native_test.go b/router/router_host_replace_native_test.go new file mode 100644 index 00000000..91f24382 --- /dev/null +++ b/router/router_host_replace_native_test.go @@ -0,0 +1,175 @@ +//go:build !js || !wasm + +package router + +import ( + "context" + "errors" + "testing" + + "github.com/rfwlab/rfw/v2/core" +) + +// hostNavigation asserts that err is the host handoff contract and returns the +// path it carries. +func hostNavigation(t *testing.T, err error) string { + t.Helper() + if !errors.Is(err, ErrHostNavigation) { + t.Fatalf("expected a host navigation, got %v", err) + } + var host *HostNavigationError + if !errors.As(err, &host) { + t.Fatalf("host navigation error does not carry its target: %v", err) + } + return host.Path +} + +// Outside the browser there is no document to replace, so the router reports +// the handoff with its target instead of pretending the host page loaded. +func TestHostReplaceReportsHandoffWithoutCommitting(t *testing.T) { + resetRouter(t) + RegisterRoute(Route{Path: "/", Component: func() core.Component { return &recordComponent{name: "root"} }}) + Navigate("/") + created := 0 + RegisterRoute(Route{ + Path: "/admin", + ResultGuards: []ResultGuard{func(map[string]string) GuardResult { + return HostReplace("/login?next=%2Fadmin") + }}, + Component: func() core.Component { + created++ + return &recordComponent{name: "admin"} + }, + Loader: func(context.Context, LoadContext) (any, error) { + t.Error("the protected route ran its loader before the host handoff") + return nil, nil + }, + Meta: map[string]any{"section": "admin"}, + }) + + err := NavigateContext(context.Background(), "/admin") + + if got := hostNavigation(t, err); got != "/login?next=%2Fadmin" { + t.Fatalf("host path = %q, want /login?next=%%2Fadmin", got) + } + if created != 0 { + t.Fatalf("the protected component was created %d times", created) + } + if got := mustRecord(t, CurrentComponent()).name; got != "root" { + t.Fatalf("current component = %q, want the mounted root", got) + } + if got := ActivePath().Get(); got != "/" { + t.Fatalf("active path = %q, want /", got) + } + if Data().Get() != nil { + t.Fatalf("the handoff committed route data: %#v", Data().Get()) + } + if section, ok := Meta().Get()["section"]; ok { + t.Fatalf("the handoff committed route metadata: %v", section) + } + if Status().Get() != NavigationError || !errors.Is(Error().Get(), ErrHostNavigation) { + t.Fatalf("navigation state: status=%s error=%v", Status().Get(), Error().Get()) + } +} + +func TestHostReplaceReportsAcceptedTargets(t *testing.T) { + for _, target := range hostReplaceAccepts { + t.Run(target, func(t *testing.T) { + resetRouter(t) + destination := target + created := guardedRoute(t, "/host-accepted", []ResultGuard{func(map[string]string) GuardResult { + return HostReplace(destination) + }}) + + err := NavigateContext(context.Background(), "/host-accepted") + + if got := hostNavigation(t, err); got != target { + t.Fatalf("host path = %q, want %q", got, target) + } + if *created != 0 || CurrentComponent() != nil { + t.Fatalf("the handoff committed a route: created=%d component=%#v", *created, CurrentComponent()) + } + }) + } +} + +// A target validation refuses fails closed with ErrInvalidGuardResult and never +// becomes a host handoff. +func TestHostReplaceRejectedTargetsFailClosed(t *testing.T) { + for _, tc := range hostReplaceRejects { + t.Run(tc.name, func(t *testing.T) { + resetRouter(t) + destination := tc.target + created := guardedRoute(t, "/host-rejected", []ResultGuard{func(map[string]string) GuardResult { + return HostReplace(destination) + }}) + + err := NavigateContext(context.Background(), "/host-rejected") + + if !errors.Is(err, ErrInvalidGuardResult) { + t.Fatalf("expected an invalid guard result, got %v", err) + } + if errors.Is(err, ErrHostNavigation) { + t.Fatalf("a rejected target became a host handoff: %v", err) + } + if *created != 0 || CurrentComponent() != nil { + t.Fatalf("a rejected target committed a route: created=%d component=%#v", *created, CurrentComponent()) + } + }) + } +} + +// A parent guard that hands off stops the chain: no child guard, no child +// loader, no component. +func TestParentHostReplaceStopsChildGuardsOnStub(t *testing.T) { + resetRouter(t) + childRan, created := false, 0 + RegisterRoute(Route{ + Path: "/host-parent", + ResultGuards: []ResultGuard{func(map[string]string) GuardResult { return HostReplace("/login") }}, + Children: []Route{{ + Path: "users", + ResultGuards: []ResultGuard{func(map[string]string) GuardResult { + childRan = true + return Allow() + }}, + Component: func() core.Component { + created++ + return &recordComponent{name: "users"} + }, + Loader: func(context.Context, LoadContext) (any, error) { + t.Error("a child loader ran after the parent handed off") + return nil, nil + }, + }}, + }) + + err := NavigateContext(context.Background(), "/host-parent/users") + + if got := hostNavigation(t, err); got != "/login" { + t.Fatalf("host path = %q, want /login", got) + } + if childRan { + t.Fatal("a child guard ran after the parent handed off") + } + if created != 0 { + t.Fatalf("the child component was created %d times", created) + } +} + +// An unauthenticated caller is handed to the host login page by the same guard +// that keeps a permission refusal inside the SPA. +func TestSessionGuardReportsHandoffForUnauthenticated(t *testing.T) { + resetRouter(t) + authenticated, permitted := false, true + created := sessionGuardedRoute(t, &authenticated, &permitted) + + err := NavigateContext(context.Background(), "/composed") + + if got := hostNavigation(t, err); got != "/login" { + t.Fatalf("host path = %q, want /login", got) + } + if *created != 0 || CurrentComponent() != nil { + t.Fatalf("the handoff committed a route: created=%d component=%#v", *created, CurrentComponent()) + } +} diff --git a/router/router_host_replace_test.go b/router/router_host_replace_test.go new file mode 100644 index 00000000..ec5c7212 --- /dev/null +++ b/router/router_host_replace_test.go @@ -0,0 +1,183 @@ +package router + +import ( + "context" + "errors" + "testing" + + "github.com/rfwlab/rfw/v2/core" +) + +// hostReplaceCase names a host handoff target under test. +type hostReplaceCase struct { + name string + target string +} + +// hostReplaceRejects lists targets a guard must never be able to hand the +// browser. Each one either leaves the origin outright or is rewritten into +// something that does by the normalization a browser applies before loading a +// URL, so validation has to refuse the string it was given rather than repair +// it into one it would accept. +var hostReplaceRejects = []hostReplaceCase{ + {"empty", ""}, + {"blank", " "}, + {"leading space", " /login"}, + {"trailing space", "/login "}, + {"surrounding spaces", " /login "}, + {"leading tab", "\t/login"}, + {"trailing tab", "/login\t"}, + {"leading carriage return", "\r/login"}, + {"trailing crlf", "/login\r\n"}, + {"surrounding whitespace", " /login\r\n"}, + {"relative", "login"}, + {"relative dot", "./login"}, + {"absolute scheme", "https://evil.example/login"}, + {"scheme relative", "//evil.example/login"}, + {"newline scheme relative", "\n//evil.example/login"}, + {"backslash authority", `/\evil.example/login`}, + {"double backslash", `/\\evil.example`}, + {"backslash in path", `/login\..\evil`}, + {"backslash in query", `/login?next=/\evil.example`}, + {"tab authority", "/\t/evil.example"}, + {"embedded newline", "/lo\ngin"}, + {"embedded carriage return", "/lo\r\ngin"}, + {"embedded tab", "/log\tin"}, + {"vertical tab", "/log\vin"}, + {"null byte", "/login\x00"}, + {"delete", "/log\x7fin"}, + {"opaque scheme", "mailto:ops@evil.example"}, + {"javascript scheme", "javascript:alert(1)"}, + {"scheme backslash", `https:/\evil.example`}, + {"truncated percent", "/login%"}, + {"malformed percent path", "/log%2in"}, + {"malformed percent query", "/login?next=%zz"}, + {"malformed percent fragment", "/login#%zz"}, +} + +// hostReplaceAccepts lists the ordinary host-owned paths an application hands +// off to: a rooted path, with or without a query and a fragment. +var hostReplaceAccepts = []string{ + "/", + "/login", + "/login?next=%2Fadmin", + "/settings#billing", + "/login?next=%2Fadmin#top", + "/account/session-expired", +} + +// sessionGuardedRoute registers /composed behind the guard an authenticated +// application writes: no session hands the browser to the host login page, a +// session without the permission is refused inside the SPA, and anything else +// enters normally. +func sessionGuardedRoute(t *testing.T, authenticated, permitted *bool) *int { + t.Helper() + created := 0 + RegisterRoute(Route{ + Path: "/composed", + ResultGuards: []ResultGuard{func(map[string]string) GuardResult { + switch { + case !*authenticated: + return HostReplace("/login") + case !*permitted: + return Forbid() + default: + return Allow() + } + }}, + Component: func() core.Component { + created++ + return &recordComponent{name: "composed"} + }, + }) + return &created +} + +func TestHostReplaceTargetRejectsOffOriginTargets(t *testing.T) { + for _, tc := range hostReplaceRejects { + t.Run(tc.name, func(t *testing.T) { + destination, err := guardHostReplaceTarget(tc.target) + if !errors.Is(err, ErrInvalidGuardResult) { + t.Fatalf("guardHostReplaceTarget(%q) = %q, %v", tc.target, destination, err) + } + if destination != "" { + t.Fatalf("rejected target returned a destination: %q", destination) + } + }) + } +} + +func TestHostReplaceTargetAcceptsRootedPaths(t *testing.T) { + for _, target := range hostReplaceAccepts { + t.Run(target, func(t *testing.T) { + destination, err := guardHostReplaceTarget(target) + if err != nil { + t.Fatalf("guardHostReplaceTarget(%q): %v", target, err) + } + if destination != target { + t.Fatalf("destination = %q, want %q", destination, target) + } + }) + } +} + +// A host handoff is its own outcome: it is neither a refusal nor a route the +// router could resolve, and it carries the target it was asked for. +func TestHostNavigationErrorIsDistinct(t *testing.T) { + var err error = &HostNavigationError{Path: "/login"} + if !errors.Is(err, ErrHostNavigation) { + t.Fatalf("host navigation error does not wrap ErrHostNavigation: %v", err) + } + if errors.Is(err, ErrNavigationForbidden) || errors.Is(err, ErrRouteNotFound) || errors.Is(err, ErrInvalidGuardResult) { + t.Fatalf("host navigation error matches another outcome: %v", err) + } + var host *HostNavigationError + if !errors.As(err, &host) || host.Path != "/login" { + t.Fatalf("host navigation error target = %#v", host) + } + if err.Error() != "router: host navigation requested: /login" { + t.Fatalf("error message = %q", err.Error()) + } +} + +// The SPA redirects keep their own validation: a host handoff loosens nothing +// for RedirectTo and ReplaceWith. +func TestGuardRedirectTargetStillRejectsOffOriginTargets(t *testing.T) { + for _, target := range []string{"", " ", "login", "//evil.example", `/\evil.example`} { + if _, err := guardRedirectTarget(target); !errors.Is(err, ErrInvalidGuardResult) { + t.Fatalf("guardRedirectTarget(%q) = %v", target, err) + } + } +} + +// Allow and Forbid are unchanged by the new action: an allowed navigation +// enters the route, and a refused one stays inside the SPA with +// ErrNavigationForbidden instead of leaving for the host. +func TestSessionGuardAllowsAndForbidsInsideSPA(t *testing.T) { + resetRouter(t) + authenticated, permitted := true, true + created := sessionGuardedRoute(t, &authenticated, &permitted) + + if err := NavigateContext(context.Background(), "/composed"); err != nil { + t.Fatalf("navigate: %v", err) + } + if got := mustRecord(t, CurrentComponent()).name; got != "composed" { + t.Fatalf("current component = %q, want composed", got) + } + if *created != 1 { + t.Fatalf("component created %d times, want 1", *created) + } + + permitted = false + err := NavigateContext(context.Background(), "/composed") + + if !errors.Is(err, ErrNavigationForbidden) { + t.Fatalf("expected a forbidden navigation, got %v", err) + } + if errors.Is(err, ErrHostNavigation) { + t.Fatal("an authenticated caller without the permission was sent to the host") + } + if *created != 1 { + t.Fatalf("forbidden navigation created the component again: %d", *created) + } +} diff --git a/router/router_host_replace_wasm_test.go b/router/router_host_replace_wasm_test.go new file mode 100644 index 00000000..54b0082f --- /dev/null +++ b/router/router_host_replace_wasm_test.go @@ -0,0 +1,325 @@ +//go:build js && wasm + +package router + +import ( + "context" + "errors" + "strings" + "testing" + + "github.com/rfwlab/rfw/v2/core" + "github.com/rfwlab/rfw/v2/dom" + "github.com/rfwlab/rfw/v2/js" +) + +// hostGuardPage is a routed page that counts its unmounts, so a test can tell +// a page left in place from one the router tore down. +type hostGuardPage struct { + *core.HTMLComponent + unmounted int +} + +func (p *hostGuardPage) Unmount() { + p.unmounted++ + p.HTMLComponent.Unmount() +} + +func newHostGuardPage(name, marker string) *hostGuardPage { + page := &hostGuardPage{HTMLComponent: core.NewHTMLComponent( + name, + []byte(`
`), + nil, + )} + page.SetComponent(page) + page.Init(nil) + return page +} + +// captureHostReplace swaps the browser handoff for a recorder. The production +// path calls location.replace, which would unload the page running the test: +// this proves the router asked for the handoff and with which path, not that +// the host served the document. +func captureHostReplace(t *testing.T) *[]string { + t.Helper() + previous := hostReplace + calls := []string{} + hostReplace = func(path string) { calls = append(calls, path) } + t.Cleanup(func() { hostReplace = previous }) + return &calls +} + +// mountHostShell renders a shell with a router outlet into #app and returns the +// page mounted on path, with a marker injected after mount the way a fetch +// callback fills a card. A remount would wipe that marker. +func mountHostShell(t *testing.T, path string) *hostGuardPage { + t.Helper() + if dom.ByID("app").IsNull() { + host := dom.CreateElement("div") + host.SetAttr("id", "app") + dom.Doc().Body().AppendChild(host) + } + shell := core.NewHTMLComponent("HostShell", []byte(`@include:outlet`), nil) + shell.SetComponent(shell) + shell.AddDependency("outlet", NewOutlet()) + shell.Init(nil) + + page := newHostGuardPage("HostDashboard", "dashboard") + Page(path, page) + MountRoot(shell) + Navigate(path) + + dom.Query("[data-host-page='dashboard']").SetHTML(`built`) + if !strings.Contains(dom.ByID("app").HTML(), "host-built") { + t.Fatal("the mounted page was not rendered") + } + return page +} + +// A guard that hands the document to a host-owned path performs the browser +// navigation and commits nothing on the way out: the protected route's loader, +// component factory, DOM and route state stay untouched, no history entry is +// written, and the page the user is looking at survives unchanged until the +// browser unloads it. +func TestHostReplaceHandsOffWithoutCommitting(t *testing.T) { + Reset() + restoreHistoryPath(t) + calls := captureHostReplace(t) + + dashboard := mountHostShell(t, "/host-dashboard") + created := 0 + RegisterRoute(Route{ + Path: "/host-admin", + ResultGuards: []ResultGuard{func(map[string]string) GuardResult { return HostReplace("/login") }}, + Component: func() core.Component { + created++ + return newHostGuardPage("HostAdmin", "admin") + }, + Loader: func(context.Context, LoadContext) (any, error) { + t.Error("the protected route ran its loader before the host handoff") + return nil, nil + }, + Meta: map[string]any{"section": "admin"}, + }) + NotFoundCallback = func(string) { t.Error("the host handoff went through the not-found callback") } + NotFoundComponent = func() core.Component { + t.Error("the host handoff rendered the not-found component") + return newHostGuardPage("HostNotFound", "not-found") + } + + historyLength := js.History().Get("length").Int() + location := js.Location().Get("pathname").String() + + if err := NavigateContext(context.Background(), "/host-admin"); err != nil { + t.Fatalf("host handoff: %v", err) + } + + if len(*calls) != 1 || (*calls)[0] != "/login" { + t.Fatalf("host replace calls = %#v, want one call with /login", *calls) + } + if created != 0 { + t.Fatalf("the protected component was created %d times", created) + } + if got := js.History().Get("length").Int(); got != historyLength { + t.Fatalf("history length changed from %d to %d", historyLength, got) + } + if got := js.Location().Get("pathname").String(); got != location { + t.Fatalf("the SPA moved the location to %q, want %q", got, location) + } + if got := ActivePath().Get(); got != "/host-dashboard" { + t.Fatalf("active path = %q, want /host-dashboard", got) + } + if mounted, ok := CurrentComponent().(*hostGuardPage); !ok || mounted != dashboard { + t.Fatalf("current component changed to %#v", CurrentComponent()) + } + if dashboard.unmounted != 0 { + t.Fatalf("the mounted page was unmounted %d times before the handoff", dashboard.unmounted) + } + html := dom.ByID("app").HTML() + if !strings.Contains(html, "host-built") { + t.Fatalf("the handoff tore down the mounted page: %s", html) + } + if strings.Contains(html, `data-host-page="admin"`) { + t.Fatalf("the protected page reached the DOM: %s", html) + } + if Status().Get() != NavigationReady { + t.Fatalf("navigation status = %s, want the mounted route's ready", Status().Get()) + } + if Error().Get() != nil { + t.Fatalf("navigation error = %v, want none", Error().Get()) + } + if section, ok := Meta().Get()["section"]; ok { + t.Fatalf("the protected route committed its metadata: %v", section) + } +} + +// A parent guard that hands off stops there: no child guard, no child loader, +// no component. +func TestParentHostReplaceStopsChildGuards(t *testing.T) { + Reset() + restoreHistoryPath(t) + calls := captureHostReplace(t) + + childRan, created := false, 0 + RegisterRoute(Route{ + Path: "/host-parent", + ResultGuards: []ResultGuard{func(map[string]string) GuardResult { return HostReplace("/login") }}, + Children: []Route{{ + Path: "users", + ResultGuards: []ResultGuard{func(map[string]string) GuardResult { + childRan = true + return Allow() + }}, + Component: func() core.Component { + created++ + return routeComponent{} + }, + Loader: func(context.Context, LoadContext) (any, error) { + t.Error("a child loader ran after the parent handed off") + return nil, nil + }, + }}, + }) + + if err := NavigateContext(context.Background(), "/host-parent/users"); err != nil { + t.Fatalf("host handoff: %v", err) + } + + if len(*calls) != 1 || (*calls)[0] != "/login" { + t.Fatalf("host replace calls = %#v, want one call with /login", *calls) + } + if childRan { + t.Fatal("a child guard ran after the parent handed off") + } + if created != 0 { + t.Fatalf("the child component was created %d times", created) + } +} + +// An unauthenticated caller leaves for the host login page while the same guard +// keeps a permission refusal inside the SPA. +func TestSessionGuardHandsOffUnauthenticated(t *testing.T) { + Reset() + t.Cleanup(Reset) + calls := captureHostReplace(t) + + authenticated, permitted := false, true + created := sessionGuardedRoute(t, &authenticated, &permitted) + + if err := NavigateContext(context.Background(), "/composed"); err != nil { + t.Fatalf("host handoff: %v", err) + } + + if len(*calls) != 1 || (*calls)[0] != "/login" { + t.Fatalf("host replace calls = %#v, want one call with /login", *calls) + } + if *created != 0 || CurrentComponent() != nil { + t.Fatalf("the handoff committed a route: created=%d component=%#v", *created, CurrentComponent()) + } +} + +// The accepted targets reach the handoff exactly as validation returned them. +func TestHostReplaceHandsOffAcceptedTargets(t *testing.T) { + for _, target := range hostReplaceAccepts { + t.Run(target, func(t *testing.T) { + Reset() + t.Cleanup(Reset) + calls := captureHostReplace(t) + destination := target + created := guardedRoute(t, "/host-accepted", []ResultGuard{func(map[string]string) GuardResult { + return HostReplace(destination) + }}) + + if err := NavigateContext(context.Background(), "/host-accepted"); err != nil { + t.Fatalf("host handoff: %v", err) + } + + if len(*calls) != 1 || (*calls)[0] != target { + t.Fatalf("host replace calls = %#v, want one call with %q", *calls, target) + } + if *created != 0 { + t.Fatalf("the protected component was created %d times", *created) + } + }) + } +} + +// A target validation refuses never reaches the browser: navigation fails +// closed and the document stays where it is. +func TestHostReplaceRejectedTargetsNeverHandOff(t *testing.T) { + for _, tc := range hostReplaceRejects { + t.Run(tc.name, func(t *testing.T) { + Reset() + restoreHistoryPath(t) + calls := captureHostReplace(t) + destination := tc.target + created := guardedRoute(t, "/host-rejected", []ResultGuard{func(map[string]string) GuardResult { + return HostReplace(destination) + }}) + location := js.Location().Get("pathname").String() + + err := NavigateContext(context.Background(), "/host-rejected") + + if !errors.Is(err, ErrInvalidGuardResult) { + t.Fatalf("expected an invalid guard result, got %v", err) + } + if len(*calls) != 0 { + t.Fatalf("a rejected target reached the browser: %#v", *calls) + } + if *created != 0 || CurrentComponent() != nil { + t.Fatalf("a rejected target committed a route: created=%d component=%#v", *created, CurrentComponent()) + } + if got := js.Location().Get("pathname").String(); got != location { + t.Fatalf("location moved to %q, want %q", got, location) + } + }) + } +} + +// The SPA redirects are untouched by the new action: they keep matching routes +// and writing history entries, and never reach the host handoff. +func TestGuardRedirectsStayInsideTheSPA(t *testing.T) { + Reset() + restoreHistoryPath(t) + calls := captureHostReplace(t) + + RegisterRoute(Route{Path: "/spa-origin", Component: func() core.Component { return routeComponent{} }}) + RegisterRoute(Route{Path: "/spa-offers", Component: func() core.Component { return routeComponent{} }}) + RegisterRoute(Route{Path: "/spa-login", Component: func() core.Component { return routeComponent{} }}) + RegisterRoute(Route{ + Path: "/spa-promo", + Component: func() core.Component { return routeComponent{} }, + ResultGuards: []ResultGuard{func(map[string]string) GuardResult { return RedirectTo("/spa-offers") }}, + }) + RegisterRoute(Route{ + Path: "/spa-account", + Component: func() core.Component { return routeComponent{} }, + ResultGuards: []ResultGuard{func(map[string]string) GuardResult { return ReplaceWith("/spa-login") }}, + }) + + Navigate("/spa-origin") + before := js.History().Get("length").Int() + + Navigate("/spa-promo") + + if got := js.Location().Get("pathname").String(); got != "/spa-offers" { + t.Fatalf("path after the guard redirect = %q", got) + } + if got := js.History().Get("length").Int(); got != before+1 { + t.Fatalf("history length = %d, want %d", got, before+1) + } + + before = js.History().Get("length").Int() + + Navigate("/spa-account") + + if got := js.Location().Get("pathname").String(); got != "/spa-login" { + t.Fatalf("path after the guard replace = %q", got) + } + if got := js.History().Get("length").Int(); got != before { + t.Fatalf("history length changed from %d to %d", before, got) + } + if len(*calls) != 0 { + t.Fatalf("an SPA redirect reached the host handoff: %#v", *calls) + } +} diff --git a/router/router_stub.go b/router/router_stub.go index d2eff63e..11d42938 100644 --- a/router/router_stub.go +++ b/router/router_stub.go @@ -321,6 +321,14 @@ func NavigateContext(parent context.Context, fullPath string) error { failNavigation(guardErr) return guardErr } + if guardResult.Action == GuardHostReplace { + // There is no document to replace here, and the host page is not a + // route this router could load in its place. Report the handoff with + // its target and leave the current component and route state alone. + err := &HostNavigationError{Path: guardResult.Path} + failNavigation(err) + return err + } if guardResult.Action != GuardAllow { redirectCtx, err := nextRedirectContext(parent) if err != nil { From 50cf5d24f58b676938e471f59dc64143ef347014 Mon Sep 17 00:00:00 2001 From: Pietro di Caprio Date: Sun, 30 Aug 2026 07:47:59 +0200 Subject: [PATCH 5/6] feat(router): revalidate mounted route guards Re-run the committed route guard chain after authority changes, release revoked routed ownership without disturbing MountRoot shells, and preserve declarative redirect and host handoff semantics. Clear revoked route data and cover cancellation, cleanup, history, and browser ownership invariants. --- CHANGELOG.md | 13 + docs/articles/guide/routing.md | 50 ++ router/data.go | 57 ++ router/revalidate.go | 171 ++++++ router/revalidate_stub.go | 146 +++++ router/router_revalidate_native_test.go | 98 ++++ router/router_revalidate_test.go | 703 ++++++++++++++++++++++++ router/router_revalidate_wasm_test.go | 448 +++++++++++++++ 8 files changed, 1686 insertions(+) create mode 100644 router/revalidate.go create mode 100644 router/revalidate_stub.go create mode 100644 router/router_revalidate_native_test.go create mode 100644 router/router_revalidate_test.go create mode 100644 router/router_revalidate_wasm_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index f306dc72..72775a0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,19 @@ version and include migration notes. browser builds `NavigateContext` returns a `*router.HostNavigationError` carrying the target and wrapping the new `router.ErrHostNavigation`, and commits nothing. +- `router.Revalidate()` and `router.RevalidateContext(ctx)`, which re-run the + guards of the route that is already mounted against the committed path and + apply their decision to it. An application whose session authority changes + installs the newer snapshot and then revalidates, instead of teaching each + page to police itself: the guards stay declarative and keep their existing + order and parameters. An allowed route is a true no-op; `Forbid()` and a + `Route.Guards` guard that now blocks unmount the routed component with its + scope and host registrations, clear its subtree and drop the data and + metadata it committed, leaving the `MountRoot` shell and the outlet mounted + and the committed path in place; `RedirectTo` and `ReplaceWith` replace the + refused history entry and route normally; `HostReplace` keeps its handoff + contract. With nothing mounted for the + committed path it does nothing, so repeating it after a refusal is a no-op. - `core.SetHostRegistrar`, the cleanup-capable form of `core.SetHostRegister`. Both remain; the legacy hook registers without a release. - `hostclient.RegisterComponentOwned`, the cleanup-capable form of diff --git a/docs/articles/guide/routing.md b/docs/articles/guide/routing.md index 3f7c6612..2fc7c308 100644 --- a/docs/articles/guide/routing.md +++ b/docs/articles/guide/routing.md @@ -146,6 +146,56 @@ if errors.As(err, &host) { } ``` +## Revalidating the mounted route + +Guards run on the way into a route. When the authority they read changes while +the user is already on the page, `router.Revalidate()` runs them again against +the committed path and applies their decision to the route that is mounted: + +```go +func onSessionChanged(snapshot Session) { + session.Install(snapshot) // the authority the guards read + router.Revalidate() // the guards decide what happens to the page +} +``` + +The session authority installs the newer snapshot first and revalidates +afterwards; the guards stay the only place that decides access, and they stay +declarative. Nothing about them changes: the same parent-before-child chain runs +with the same parameters, `Guards` before `ResultGuards`. + +- A route the guards still allow is a true no-op: no loader, no component, no + render, no history entry, no route data or metadata change. A loader that a + revalidation cancelled leaves the status back on `ready`. +- `router.Forbid()`, and a `Route.Guards` guard that now returns false, unmount + the routed component, run its scope cleanup and its host registrations, and + remove its subtree. The shell mounted with `MountRoot` and the outlet inside + it stay where they are. What the route loaded goes with it: `router.Data()` + returns `nil` and `router.Meta()` an empty map, so nothing renders the revoked + page's data after the refusal. The status goes to `error` with + `router.ErrNavigationForbidden` or `router.ErrNavigationBlocked`, and the + committed path is kept, so an explicit navigation can enter the route again + once the guards allow it. +- `router.RedirectTo(path)` and `router.ReplaceWith(path)` both replace the + current history entry, since it is the one the guard just refused, and then + route normally to the destination. +- `router.HostReplace(path)` hands the document to the host with the same + contract it has during navigation: nothing is unmounted, written or committed + before the browser unloads the page. + +With no routed component mounted, or none matching the committed path, nothing +happens, so calling it again after a refusal is a no-op, and so is calling it +during a navigation that has not committed yet (that navigation still has its +own guards ahead of it). + +`router.RevalidateContext(ctx)` returns the outcome, and answers a cancelled +context before running any guard. Do not call either from a guard, for the same +reason `router.Navigate` must not be called from one. + +Outside browser builds there is no DOM or history to keep aligned: a refused +route is unmounted and dropped, a redirect navigates, and a host handoff returns +the `*router.HostNavigationError` described above. + ## Data loaders A loader completes before the new component is created and the previous page diff --git a/router/data.go b/router/data.go index 10119895..e3f7ea53 100644 --- a/router/data.go +++ b/router/data.go @@ -372,6 +372,37 @@ func failNavigation(err error) { }) } +// revokeCommittedRoute fails navigation for a route that was committed and has +// just been taken back. What it loaded goes with it: the data and the metadata +// belong to a page the guards no longer allow, so they are dropped in the same +// batch that reports the refusal, and Data and Meta never keep serving what the +// revocation removed from the screen. A denied navigation still uses +// failNavigation, where the route the user is on stays allowed and keeps +// everything it committed. +func revokeCommittedRoute(err error) { + state.Batch(func() { + currentRouteData.Set(nil) + currentRouteMeta.Set(map[string]any{}) + navigationError.Set(err) + navigationStatus.Set(NavigationError) + }) +} + +// settleRevalidatedRoute leaves a route its guards still allow exactly as it +// is. The one thing it repairs is the loading status of the navigation the +// revalidation cancelled: that navigation can no longer reach a status of its +// own, and the route the user is looking at is ready, with the data and the +// metadata it committed when it was entered. +func settleRevalidatedRoute() { + if navigationStatus.Get() != NavigationLoading { + return + } + state.Batch(func() { + navigationError.Set(nil) + navigationStatus.Set(NavigationReady) + }) +} + func cloneMeta(meta map[string]any) map[string]any { if meta == nil { return map[string]any{} @@ -453,6 +484,32 @@ func routeQuery(raw string) (map[string]string, url.Values) { return params, values } +// matchCurrentRoute resolves a committed full path the way navigation resolves +// a destination: the same matcher, the same guard chain, and the same +// parameters, route and query merged in the same order. Revalidation reuses it +// so a guard cannot see one set of parameters when it decides on entry and +// another when it decides whether the route may stay. +func matchCurrentRoute(fullPath string) (*route, []guardEntry, map[string]string) { + path := fullPath + query := "" + if idx := strings.Index(fullPath, "?"); idx != -1 { + path = fullPath[:idx] + query = fullPath[idx+1:] + } + r, guards, params := matchRoute(routes, path) + if r == nil { + return nil, nil, nil + } + if params == nil { + params = map[string]string{} + } + queryParams, _ := routeQuery(query) + for key, value := range queryParams { + params[key] = value + } + return r, guards, params +} + type redirectDepthKey struct{} func nextRedirectContext(parent context.Context) (context.Context, error) { diff --git a/router/revalidate.go b/router/revalidate.go new file mode 100644 index 00000000..15379d0f --- /dev/null +++ b/router/revalidate.go @@ -0,0 +1,171 @@ +//go:build js && wasm + +package router + +import ( + "context" + + "github.com/rfwlab/rfw/v2/core" + "github.com/rfwlab/rfw/v2/dom" + js "github.com/rfwlab/rfw/v2/js" +) + +// Revalidate re-runs the guards of the route that is currently mounted against +// the committed path, and applies their decision to it. It is what an +// application calls after the authority a guard reads changes under a page the +// user is already on: install the new snapshot first, then revalidate, and the +// guards stay the single declarative place where access is decided. +// +// A route its guards still allow is left untouched. One they now refuse is torn +// down like the navigation that would have been refused: the routed component +// unmounts, its subtree leaves the DOM, and the shell around the outlet stays +// mounted. A guard that redirects sends navigation to its destination, and one +// that hands off to the host loads the host page. With no routed component +// mounted, or none for the committed path, nothing happens, so calling it again +// after a refusal is a no-op. +// +// It must not be called from a guard: a guard returns a decision, and the +// router is what acts on it. +func Revalidate() { + _ = RevalidateContext(context.Background()) +} + +// RevalidateContext revalidates the mounted route like Revalidate and returns +// the outcome: nil when the route may stay, when a redirect committed, when the +// browser was handed to the host, or when there was nothing to revalidate, and +// otherwise the error the guards decided on (ErrNavigationForbidden, +// ErrNavigationBlocked, ErrInvalidGuardResult) or the context error. +func RevalidateContext(parent context.Context) error { + // A caller with no context to give passes none, the way Revalidate does. + // Normalized here, at the one entry point: a redirect derives its context + // from this parent, and deriving from a nil one panics. + if parent == nil { + parent = context.Background() + } + var revalidationErr error + core.TryNavigate(activePathSig.Get(), func() { + revalidationErr = revalidateImpl(parent) + }) + return revalidationErr +} + +func revalidateImpl(parent context.Context) error { + // Nothing mounted is nothing to revalidate: a navigation that has not + // committed yet still has its own guards ahead of it, and a refusal already + // cleared the owner it tore down. + if currentComponent == nil { + return nil + } + r, guards, params := matchCurrentRoute(activePathSig.Get()) + if r == nil { + return nil + } + // The match only speaks for the route that actually committed what is + // mounted. It does not when a route is registered for the path the + // not-found component is sitting on: those guards never ran to put anything + // there, so they have no say over a component that is not theirs. + if r.component != currentComponent { + return nil + } + // Taking the navigation generation cancels whatever is in flight: it was + // authorized under the snapshot this call replaces, so its loader must not + // come back and commit over the decision made here. + ctx, _ := beginNavigation(parent) + if err := ctx.Err(); err != nil { + return err + } + + guardResult, guardErr := runGuards(guards, params) + if guardErr != nil { + // Every refusal fails closed the same way, a legacy guard that blocks + // included: the route is mounted, so leaving it there is the one outcome + // a refusal cannot have. Navigation to a refused destination keeps its + // own behavior, where nothing was committed to take back. + revokeMountedRoute(guardErr) + return guardErr + } + switch guardResult.Action { + case GuardAllow: + settleRevalidatedRoute() + return nil + case GuardHostReplace: + // The host owns the destination and the browser loads it as a document. + // Nothing commits and nothing is torn down first: the page the user is + // on stays exactly as it is until the browser unloads it. + hostReplace(guardResult.Path) + return nil + } + + redirectCtx, err := nextRedirectContext(parent) + if err != nil { + failNavigation(err) + return err + } + // The entry the browser is sitting on is the one the guard just refused, so + // both redirect actions replace it: pushing would leave the refused path + // behind for back to return to, where the guard refuses it again. From + // there it is ordinary routing, which unmounts the current owner once. + return navigateImpl(redirectCtx, guardResult.Path, historyReplace) +} + +// revokeMountedRoute tears down the routed owner its guards just refused. Only +// the route goes: the persistent root mounted with MountRoot and the outlet +// inside it stay where they are, so the next allowed navigation renders into +// the shell the user is still looking at. What the route loaded goes with it, +// so nothing reads its data or its metadata after the refusal. The committed +// path is kept, so a repeated revalidation finds nothing mounted and does +// nothing, while an explicit navigation can enter the route again once its +// guards allow it. +func revokeMountedRoute(err error) { + owner := currentComponent + // Dropped before the teardown, not after: the outlet repaints whatever + // currentComponent points at whenever a component around it paints, and + // unmounting paints. + currentComponent = nil + core.Log().Debug("Unmounting revoked component: %s", owner.GetName()) + core.TriggerUnmount(owner) + // Unmount first, so the component's own cleanup, its scope and its host + // registrations still see the tree they own. + owner.Unmount() + clearRoutedSubtree(owner.GetID()) + revokeCommittedRoute(err) +} + +// clearRoutedSubtree removes the DOM the routed owner had, and nothing else. +// With a live outlet the marker div is the router's anchor inside the shell, so +// its content goes and the marker itself stays where a later navigation and a +// shell re-render expect to find it. Without one the router rendered the page +// into #app itself and the owner's own root is removed. +// +// dom.ComponentRoot answers #app for an id it cannot resolve, which here would +// mean erasing the shell instead of the route, so a resolved root is used only +// when it really carries the id it was asked for. +func clearRoutedSubtree(id string) { + if content, ok := routedOutletContent(); ok { + content.SetHTML("") + return + } + root := dom.ComponentRoot(id) + if root.IsNull() || root.IsUndefined() || root.Attr("data-component-id") != id { + return + } + root.Call("remove") +} + +// routedOutletContent resolves the element a live outlet renders routed +// components into: the marker div, or the outlet's own root when the template +// carries no marker. It reports false when there is no outlet or its root is +// not in the document, the case renderChild falls back to #app for. +func routedOutletContent() (dom.Element, bool) { + if liveOutlet == nil { + return dom.Element{Value: js.Null()}, false + } + root := dom.ComponentRoot(liveOutlet.GetID()) + if root.IsNull() || root.IsUndefined() || root.Attr("data-component-id") != liveOutlet.GetID() { + return dom.Element{Value: js.Null()}, false + } + if target := root.Query("[data-router-outlet]"); !target.IsNull() && !target.IsUndefined() { + return target, true + } + return root, true +} diff --git a/router/revalidate_stub.go b/router/revalidate_stub.go new file mode 100644 index 00000000..f66e659c --- /dev/null +++ b/router/revalidate_stub.go @@ -0,0 +1,146 @@ +//go:build !js || !wasm + +package router + +import ( + "context" + + "github.com/rfwlab/rfw/v2/core" +) + +// routeUnmounter is a routed component that owns lifecycle cleanup. The +// Component interface outside browser builds is render-only, so a revoked route +// is unmounted through this one when the component provides it. +type routeUnmounter interface{ Unmount() } + +// Revalidate re-runs the guards of the route that is currently mounted against +// the committed path, and applies their decision to it. It is what an +// application calls after the authority a guard reads changes under a page the +// user is already on: install the new snapshot first, then revalidate, and the +// guards stay the single declarative place where access is decided. +// +// A route its guards still allow is left untouched. One they now refuse is torn +// down like the navigation that would have been refused: the routed component +// is unmounted and dropped. A guard that redirects navigates to its +// destination. With no routed component mounted, or none for the committed +// path, nothing happens, so calling it again after a refusal is a no-op. +// +// It must not be called from a guard: a guard returns a decision, and the +// router is what acts on it. +func Revalidate() { + _ = RevalidateContext(context.Background()) +} + +// RevalidateContext revalidates the mounted route like Revalidate and returns +// the outcome: nil when the route may stay, when a redirect committed, or when +// there was nothing to revalidate, and otherwise the error the guards decided +// on (ErrNavigationForbidden, ErrNavigationBlocked, ErrInvalidGuardResult), the +// *HostNavigationError of a host handoff, or the context error. +func RevalidateContext(parent context.Context) error { + // A caller with no context to give passes none, the way Revalidate does. + // Normalized here, at the one entry point: a redirect derives its context + // from this parent, and deriving from a nil one panics. + if parent == nil { + parent = context.Background() + } + // Nothing mounted is nothing to revalidate: a navigation that has not + // committed yet still has its own guards ahead of it, and a refusal already + // cleared the owner it dropped. + if currentComponent == nil { + return nil + } + r, guards, params := matchCurrentRoute(activePathSig.Get()) + if r == nil { + return nil + } + // The match only speaks for the route that actually committed what is + // mounted. It does not when a route is registered for the path the + // not-found component is sitting on: those guards never ran to put anything + // there, so they have no say over a component that is not theirs. + if r.component != currentComponent { + return nil + } + // Taking the navigation generation cancels whatever is in flight: it was + // authorized under the snapshot this call replaces, so its loader must not + // come back and commit over the decision made here. + ctx, _ := beginNavigation(parent) + if err := ctx.Err(); err != nil { + return err + } + + guardResult, guardErr := runGuards(guards, params) + if guardErr != nil { + // Every refusal fails closed the same way, a legacy guard that blocks + // included: the route is mounted, so leaving it there is the one outcome + // a refusal cannot have. Navigation to a refused destination keeps its + // own behavior, where nothing was committed to take back. + revokeMountedRoute(guardErr) + return guardErr + } + switch guardResult.Action { + case GuardAllow: + settleRevalidatedRoute() + return nil + case GuardHostReplace: + // There is no document to replace here and the host page is not a route + // this router could load in its place, so the handoff is reported with + // its target and the mounted route is left alone, exactly as + // NavigateContext reports it. + hostErr := &HostNavigationError{Path: guardResult.Path} + failNavigation(hostErr) + return hostErr + } + + redirectCtx, err := nextRedirectContext(parent) + if err != nil { + failNavigation(err) + return err + } + // Taken before the redirect, released after it commits: the destination is + // what decides whether this route is still the one the user owns. + owner := currentComponent + // The stub keeps no browser history, so a push and a replace redirect both + // navigate, as they do for a refused destination. + if err := NavigateContext(redirectCtx, guardResult.Path); err != nil { + // The destination refused, failed to load or was cancelled: nothing + // committed over the route the user is on, so it stays current and keeps + // everything it holds. + return err + } + releaseReplacedOwner(owner) + return nil +} + +// releaseReplacedOwner runs the lifecycle of the routed owner a committed +// redirect left behind. Navigation outside the browser has no DOM to tear down +// and only moves the pointer to the routed owner, so the one it replaced is +// released here, exactly once, where the browser gets the same teardown from +// navigateImpl. An owner the destination resolved back to is still current and +// is left running. +func releaseReplacedOwner(owner core.Component) { + if owner == nil || owner == currentComponent { + return + } + tearDownRoutedOwner(owner) +} + +// revokeMountedRoute drops the routed owner its guards just refused, and what +// it loaded with it, so nothing reads its data or its metadata after the +// refusal. The committed path is kept, so a repeated revalidation finds nothing +// mounted and does nothing, while an explicit navigation can enter the route +// again once its guards allow it. +func revokeMountedRoute(err error) { + owner := currentComponent + currentComponent = nil + tearDownRoutedOwner(owner) + revokeCommittedRoute(err) +} + +// tearDownRoutedOwner runs the lifecycle of a routed owner that is no longer +// current, exactly once. +func tearDownRoutedOwner(owner core.Component) { + core.TriggerUnmount(owner) + if unmounter, ok := owner.(routeUnmounter); ok { + unmounter.Unmount() + } +} diff --git a/router/router_revalidate_native_test.go b/router/router_revalidate_native_test.go new file mode 100644 index 00000000..1543e7c1 --- /dev/null +++ b/router/router_revalidate_native_test.go @@ -0,0 +1,98 @@ +//go:build !js || !wasm + +package router + +import ( + "context" + "errors" + "testing" +) + +// Outside the browser there is no document to hand to the host, so a +// revalidation that hands off reports the target and leaves the mounted route +// exactly where it is, the contract NavigateContext already keeps. +func TestRevalidateHostReplaceReportsHandoff(t *testing.T) { + resetRouter(t) + valid := true + session := registerSessionRoute(t, "/reval-host", func(map[string]string) GuardResult { + if !valid { + return HostReplace("/login?next=%2Freval-host") + } + return Allow() + }) + + if err := NavigateContext(context.Background(), "/reval-host"); err != nil { + t.Fatalf("navigate: %v", err) + } + page := mustRevalidatePage(t, CurrentComponent()) + created, loaded := session.created, session.loaded + + valid = false + err := RevalidateContext(context.Background()) + + if got := hostNavigation(t, err); got != "/login?next=%2Freval-host" { + t.Fatalf("host path = %q", got) + } + if page.unmounted != 0 || page.cleanups != 0 { + t.Fatalf("the handoff tore the mounted page down: unmounted=%d cleanups=%d", page.unmounted, page.cleanups) + } + if CurrentComponent() != page { + t.Fatalf("current component changed to %#v", CurrentComponent()) + } + if session.created != created || session.loaded != loaded { + t.Fatalf("the handoff re-ran the route: created=%d loaded=%d", session.created, session.loaded) + } + if got := ActivePath().Get(); got != "/reval-host" { + t.Fatalf("active path = %q", got) + } + if Data().Get() != "payload" || Meta().Get()["section"] != "session" { + t.Fatalf("the handoff changed route state: data=%#v meta=%#v", Data().Get(), Meta().Get()) + } + if Status().Get() != NavigationError || !errors.Is(Error().Get(), ErrHostNavigation) { + t.Fatalf("navigation state: status=%s error=%v", Status().Get(), Error().Get()) + } +} + +// A guard result validation refuses is a refusal like any other: the route the +// user is on is mounted, so it fails closed and is torn down rather than left +// in place on a decision the router could not apply. +func TestRevalidateInvalidGuardResultFailsClosed(t *testing.T) { + for name, decision := range map[string]GuardResult{ + "scheme relative": RedirectTo("//evil.example/login"), + "relative": ReplaceWith("login"), + "off origin host": HostReplace("https://evil.example/login"), + "unknown action": {Action: GuardAction(42), Path: "/login"}, + } { + t.Run(name, func(t *testing.T) { + resetRouter(t) + result := decision + valid := true + registerSessionRoute(t, "/reval-invalid", func(map[string]string) GuardResult { + if !valid { + return result + } + return Allow() + }) + + if err := NavigateContext(context.Background(), "/reval-invalid"); err != nil { + t.Fatalf("navigate: %v", err) + } + page := mustRevalidatePage(t, CurrentComponent()) + + valid = false + if err := RevalidateContext(context.Background()); !errors.Is(err, ErrInvalidGuardResult) { + t.Fatalf("expected an invalid guard result, got %v", err) + } + if page.unmounted != 1 || page.cleanups != 1 { + t.Fatalf("teardown ran %d times (cleanups %d), want 1", page.unmounted, page.cleanups) + } + if CurrentComponent() != nil { + t.Fatalf("the route stayed current: %#v", CurrentComponent()) + } + // A refusal the router could not even apply revokes like any other. + if Data().Get() != nil || len(Meta().Get()) != 0 { + t.Fatalf("the revoked route's state survived: data=%#v meta=%#v", Data().Get(), Meta().Get()) + } + }) + } +} diff --git a/router/router_revalidate_test.go b/router/router_revalidate_test.go new file mode 100644 index 00000000..8ebb0c5a --- /dev/null +++ b/router/router_revalidate_test.go @@ -0,0 +1,703 @@ +package router + +import ( + "context" + "errors" + "testing" + + "github.com/rfwlab/rfw/v2/core" +) + +// revalidatePage is a routed component that counts the lifecycle a revocation +// has to run exactly once, and owns a scope the way a page with running work +// does. +type revalidatePage struct { + name string + scope *core.Scope + unmounted int + paramRuns int + cleanups int +} + +func newRevalidatePage(name string) *revalidatePage { + page := &revalidatePage{name: name} + page.scope = core.NewScope() + page.scope.Defer(func() { page.cleanups++ }) + return page +} + +func (p *revalidatePage) Render() string { return "" } +func (p *revalidatePage) Mount() {} +func (p *revalidatePage) OnMount() {} +func (p *revalidatePage) OnUnmount() {} +func (p *revalidatePage) GetName() string { return p.name } +func (p *revalidatePage) GetID() string { return p.name } +func (p *revalidatePage) SetSlots(map[string]any) {} +func (p *revalidatePage) IsMounted() bool { return p.unmounted == 0 } + +func (p *revalidatePage) Unmount() { + p.unmounted++ + p.scope.Close() +} + +func (p *revalidatePage) OnParams(map[string]string) { p.paramRuns++ } + +func mustRevalidatePage(t *testing.T, c core.Component) *revalidatePage { + t.Helper() + page, ok := c.(*revalidatePage) + if !ok { + t.Fatalf("expected *revalidatePage, got %T", c) + } + return page +} + +// sessionRoute registers a route whose guard reads a decision the test changes +// between navigations, the shape of an application that installs a new +// authorization snapshot and then revalidates. It counts every guard run, every +// component creation and every loader run. +type sessionRoute struct { + decide func(map[string]string) GuardResult + guards int + created int + loaded int +} + +func registerSessionRoute(t *testing.T, path string, decide func(map[string]string) GuardResult) *sessionRoute { + t.Helper() + sr := &sessionRoute{decide: decide} + RegisterRoute(Route{ + Path: path, + ResultGuards: []ResultGuard{func(params map[string]string) GuardResult { + sr.guards++ + return sr.decide(params) + }}, + Component: func() core.Component { + sr.created++ + return newRevalidatePage("session") + }, + Loader: func(context.Context, LoadContext) (any, error) { + sr.loaded++ + return "payload", nil + }, + Meta: map[string]any{"section": "session"}, + }) + return sr +} + +// A route its guards still allow is left exactly as it is: the guards run +// again, and nothing else does. +func TestRevalidateAllowedRouteIsANoOp(t *testing.T) { + resetRouter(t) + allowed := true + session := registerSessionRoute(t, "/reval-open", func(map[string]string) GuardResult { + if !allowed { + return Forbid() + } + return Allow() + }) + + if err := NavigateContext(context.Background(), "/reval-open?tab=usage"); err != nil { + t.Fatalf("navigate: %v", err) + } + page := mustRevalidatePage(t, CurrentComponent()) + guards, created, loaded, paramRuns := session.guards, session.created, session.loaded, page.paramRuns + + if err := RevalidateContext(context.Background()); err != nil { + t.Fatalf("revalidate: %v", err) + } + + if session.guards != guards+1 { + t.Fatalf("guard ran %d times, want %d", session.guards, guards+1) + } + if session.created != created || session.loaded != loaded { + t.Fatalf("revalidation re-ran the route: created=%d loaded=%d", session.created, session.loaded) + } + if page.paramRuns != paramRuns { + t.Fatalf("revalidation re-ran OnParams %d times, want %d", page.paramRuns, paramRuns) + } + if page.unmounted != 0 || page.cleanups != 0 { + t.Fatalf("revalidation tore the page down: unmounted=%d cleanups=%d", page.unmounted, page.cleanups) + } + if CurrentComponent() != core.Component(page) { + t.Fatalf("current component changed to %#v", CurrentComponent()) + } + if got := ActivePath().Get(); got != "/reval-open?tab=usage" { + t.Fatalf("active path = %q", got) + } + if Status().Get() != NavigationReady || Error().Get() != nil { + t.Fatalf("navigation state: status=%s error=%v", Status().Get(), Error().Get()) + } + if Data().Get() != "payload" { + t.Fatalf("route data = %#v, want payload", Data().Get()) + } + if Meta().Get()["section"] != "session" { + t.Fatalf("route metadata = %#v", Meta().Get()) + } +} + +// A guard that refuses a route the user is already on unmounts it once, and +// keeps the committed path so a later navigation can retry it. +func TestRevalidateForbidTearsDownTheOwnerOnce(t *testing.T) { + resetRouter(t) + allowed := true + session := registerSessionRoute(t, "/reval-vault", func(map[string]string) GuardResult { + if !allowed { + return Forbid() + } + return Allow() + }) + + if err := NavigateContext(context.Background(), "/reval-vault"); err != nil { + t.Fatalf("navigate: %v", err) + } + page := mustRevalidatePage(t, CurrentComponent()) + if Data().Get() != "payload" || Meta().Get()["section"] != "session" { + t.Fatalf("the route committed no state to revoke: data=%#v meta=%#v", Data().Get(), Meta().Get()) + } + + allowed = false + err := RevalidateContext(context.Background()) + + if !errors.Is(err, ErrNavigationForbidden) { + t.Fatalf("expected a forbidden revalidation, got %v", err) + } + // The refused route's loader data and metadata went with it: they described a + // page the guards no longer allow. + if Data().Get() != nil { + t.Fatalf("the revoked route's data is still readable: %#v", Data().Get()) + } + if meta := Meta().Get(); len(meta) != 0 { + t.Fatalf("the revoked route's metadata is still readable: %#v", meta) + } + if page.unmounted != 1 { + t.Fatalf("page unmounted %d times, want 1", page.unmounted) + } + if page.cleanups != 1 { + t.Fatalf("scope cleanup ran %d times, want 1", page.cleanups) + } + if CurrentComponent() != nil { + t.Fatalf("the revoked route stayed current: %#v", CurrentComponent()) + } + if Status().Get() != NavigationError || !errors.Is(Error().Get(), ErrNavigationForbidden) { + t.Fatalf("navigation state: status=%s error=%v", Status().Get(), Error().Get()) + } + if got := ActivePath().Get(); got != "/reval-vault" { + t.Fatalf("active path = %q, want the committed path", got) + } + + guards := session.guards + if err := RevalidateContext(context.Background()); err != nil { + t.Fatalf("repeated revalidation: %v", err) + } + if session.guards != guards { + t.Fatalf("a repeated revalidation re-ran the guards: %d, want %d", session.guards, guards) + } + if page.unmounted != 1 || page.cleanups != 1 { + t.Fatalf("a repeated revalidation tore the page down again: unmounted=%d cleanups=%d", page.unmounted, page.cleanups) + } + + allowed = true + if err := NavigateContext(context.Background(), "/reval-vault"); err != nil { + t.Fatalf("navigating back into the route: %v", err) + } + if mustRevalidatePage(t, CurrentComponent()) == page { + t.Fatal("the revoked page was remounted instead of a fresh one") + } +} + +// A legacy bool guard that now blocks tears the mounted route down the same +// way, while a blocked navigation to another route keeps its own behavior: +// nothing was committed there, so nothing is taken back. +func TestRevalidateLegacyBlockTearsDownAndNavigationIsUnchanged(t *testing.T) { + resetRouter(t) + allowed := true + RegisterRoute(Route{ + Path: "/reval-legacy", + Guards: []Guard{func(map[string]string) bool { return allowed }}, + Component: func() core.Component { return newRevalidatePage("legacy") }, + Loader: func(context.Context, LoadContext) (any, error) { return "payload", nil }, + Meta: map[string]any{"section": "legacy"}, + }) + blocked := 0 + RegisterRoute(Route{ + Path: "/reval-legacy-other", + Guards: []Guard{func(map[string]string) bool { blocked++; return false }}, + Component: func() core.Component { return newRevalidatePage("other") }, + }) + RegisterRoute(Route{ + Path: "/reval-legacy-forbidden", + ResultGuards: []ResultGuard{func(map[string]string) GuardResult { return Forbid() }}, + Component: func() core.Component { return newRevalidatePage("forbidden") }, + }) + + if err := NavigateContext(context.Background(), "/reval-legacy"); err != nil { + t.Fatalf("navigate: %v", err) + } + page := mustRevalidatePage(t, CurrentComponent()) + + // An ordinary denied navigation leaves the mounted page and the navigation + // state exactly as they were. + if err := NavigateContext(context.Background(), "/reval-legacy-other"); !errors.Is(err, ErrNavigationBlocked) { + t.Fatalf("expected a blocked navigation, got %v", err) + } + if blocked != 1 { + t.Fatalf("the blocking guard ran %d times, want 1", blocked) + } + if CurrentComponent() != core.Component(page) || page.unmounted != 0 { + t.Fatalf("a blocked navigation disturbed the mounted page: current=%#v unmounted=%d", CurrentComponent(), page.unmounted) + } + if Status().Get() != NavigationReady || Error().Get() != nil { + t.Fatalf("a blocked navigation changed navigation state: status=%s error=%v", Status().Get(), Error().Get()) + } + if Data().Get() != "payload" || Meta().Get()["section"] != "legacy" { + t.Fatalf("a blocked navigation dropped the mounted route's state: data=%#v meta=%#v", Data().Get(), Meta().Get()) + } + // A refused destination is denied the same way: nothing committed there, so + // the page the user is on keeps everything it loaded. + if err := NavigateContext(context.Background(), "/reval-legacy-forbidden"); !errors.Is(err, ErrNavigationForbidden) { + t.Fatalf("expected a forbidden navigation, got %v", err) + } + if CurrentComponent() != core.Component(page) || page.unmounted != 0 { + t.Fatalf("a forbidden navigation disturbed the mounted page: current=%#v unmounted=%d", CurrentComponent(), page.unmounted) + } + if Data().Get() != "payload" || Meta().Get()["section"] != "legacy" { + t.Fatalf("a forbidden navigation dropped the mounted route's state: data=%#v meta=%#v", Data().Get(), Meta().Get()) + } + + allowed = false + err := RevalidateContext(context.Background()) + + if !errors.Is(err, ErrNavigationBlocked) { + t.Fatalf("expected a blocked revalidation, got %v", err) + } + if page.unmounted != 1 || page.cleanups != 1 { + t.Fatalf("teardown ran wrong: unmounted=%d cleanups=%d", page.unmounted, page.cleanups) + } + if CurrentComponent() != nil { + t.Fatalf("the blocked route stayed current: %#v", CurrentComponent()) + } + if Status().Get() != NavigationError || !errors.Is(Error().Get(), ErrNavigationBlocked) { + t.Fatalf("navigation state: status=%s error=%v", Status().Get(), Error().Get()) + } + if Data().Get() != nil { + t.Fatalf("the revoked route's data is still readable: %#v", Data().Get()) + } + if meta := Meta().Get(); len(meta) != 0 { + t.Fatalf("the revoked route's metadata is still readable: %#v", meta) + } + if got := ActivePath().Get(); got != "/reval-legacy" { + t.Fatalf("active path = %q, want the committed path", got) + } +} + +// A guard that redirects during revalidation routes to its destination the way +// navigation does, for both redirect actions. +func TestRevalidateRedirectRoutesToTheDestination(t *testing.T) { + for name, decision := range map[string]GuardResult{ + "redirect": RedirectTo("/reval-login"), + "replace": ReplaceWith("/reval-login"), + } { + t.Run(name, func(t *testing.T) { + resetRouter(t) + result := decision + allowed := true + RegisterRoute(Route{Path: "/reval-login", Component: func() core.Component { return newRevalidatePage("login") }}) + session := registerSessionRoute(t, "/reval-account", func(map[string]string) GuardResult { + if !allowed { + return result + } + return Allow() + }) + + if err := NavigateContext(context.Background(), "/reval-account"); err != nil { + t.Fatalf("navigate: %v", err) + } + page := mustRevalidatePage(t, CurrentComponent()) + created := session.created + + allowed = false + if err := RevalidateContext(context.Background()); err != nil { + t.Fatalf("revalidate: %v", err) + } + + if got := mustRevalidatePage(t, CurrentComponent()).name; got != "login" { + t.Fatalf("current component = %q, want login", got) + } + // The redirect committed over the refused route, so the owner it + // replaced is released with it, once. + if page.unmounted != 1 || page.cleanups != 1 { + t.Fatalf("teardown ran %d times (cleanups %d), want 1", page.unmounted, page.cleanups) + } + if got := ActivePath().Get(); got != "/reval-login" { + t.Fatalf("active path = %q, want /reval-login", got) + } + if session.created != created { + t.Fatalf("the refused route was created again: %d", session.created) + } + if Status().Get() != NavigationReady { + t.Fatalf("navigation status = %s, want ready", Status().Get()) + } + }) + } +} + +// A redirect that never commits leaves the route the user is on exactly as it +// is: the destination is what failed, so the owner still holds the page, its +// scope and everything hanging off it, and a later revalidation can decide +// again. Nothing may be released on the way to a destination that was refused. +func TestRevalidateFailedRedirectKeepsTheOwner(t *testing.T) { + for name, register := range map[string]func(){ + "forbidden destination": func() { + RegisterRoute(Route{ + Path: "/reval-dead-login", + Component: func() core.Component { return newRevalidatePage("login") }, + ResultGuards: []ResultGuard{func(map[string]string) GuardResult { return Forbid() }}, + }) + }, + "failing loader": func() { + RegisterRoute(Route{ + Path: "/reval-dead-login", + Component: func() core.Component { return newRevalidatePage("login") }, + Loader: func(context.Context, LoadContext) (any, error) { + return nil, errors.New("login unavailable") + }, + }) + }, + } { + t.Run(name, func(t *testing.T) { + resetRouter(t) + register() + allowed := true + registerSessionRoute(t, "/reval-dead", func(map[string]string) GuardResult { + if !allowed { + return ReplaceWith("/reval-dead-login") + } + return Allow() + }) + + if err := NavigateContext(context.Background(), "/reval-dead"); err != nil { + t.Fatalf("navigate: %v", err) + } + page := mustRevalidatePage(t, CurrentComponent()) + + allowed = false + if err := RevalidateContext(context.Background()); err == nil { + t.Fatal("a redirect to a destination that never commits reported success") + } + + if CurrentComponent() != core.Component(page) { + t.Fatalf("the owner was replaced by a redirect that failed: %#v", CurrentComponent()) + } + if page.unmounted != 0 || page.cleanups != 0 { + t.Fatalf("a failed redirect tore the owner down: unmounted=%d cleanups=%d", page.unmounted, page.cleanups) + } + if got := ActivePath().Get(); got != "/reval-dead" { + t.Fatalf("active path = %q, want the committed path", got) + } + }) + } +} + +// noContext returns the nil context a caller with none of its own passes. It +// goes through a function so the "do not pass a nil Context" analysis does not +// rewrite the very call under test. +func noContext() context.Context { return nil } + +// A caller that passes no context gets the one Revalidate builds for itself, +// redirects included: a redirect derives its context from the parent, and +// deriving from a nil one is what the boundary normalizes away. +func TestRevalidateNilContextRedirects(t *testing.T) { + resetRouter(t) + allowed := true + RegisterRoute(Route{Path: "/reval-nil-login", Component: func() core.Component { return newRevalidatePage("login") }}) + registerSessionRoute(t, "/reval-nil", func(map[string]string) GuardResult { + if !allowed { + return ReplaceWith("/reval-nil-login") + } + return Allow() + }) + + if err := NavigateContext(context.Background(), "/reval-nil"); err != nil { + t.Fatalf("navigate: %v", err) + } + page := mustRevalidatePage(t, CurrentComponent()) + + allowed = false + if err := RevalidateContext(noContext()); err != nil { + t.Fatalf("revalidate: %v", err) + } + + if got := mustRevalidatePage(t, CurrentComponent()).name; got != "login" { + t.Fatalf("current component = %q, want login", got) + } + if page.unmounted != 1 { + t.Fatalf("the refused page unmounted %d times, want 1", page.unmounted) + } + if got := ActivePath().Get(); got != "/reval-nil-login" { + t.Fatalf("active path = %q, want /reval-nil-login", got) + } +} + +// Two guards redirecting at each other during revalidation hit the same +// redirect-loop limit navigation does. +func TestRevalidateRedirectLoopFails(t *testing.T) { + resetRouter(t) + revalidating := false + RegisterRoute(Route{ + Path: "/reval-ping", + Component: func() core.Component { return newRevalidatePage("ping") }, + ResultGuards: []ResultGuard{func(map[string]string) GuardResult { + if revalidating { + return RedirectTo("/reval-pong") + } + return Allow() + }}, + }) + RegisterRoute(Route{ + Path: "/reval-pong", + Component: func() core.Component { return newRevalidatePage("pong") }, + ResultGuards: []ResultGuard{func(map[string]string) GuardResult { return ReplaceWith("/reval-ping") }}, + }) + + if err := NavigateContext(context.Background(), "/reval-ping"); err != nil { + t.Fatalf("navigate: %v", err) + } + + revalidating = true + if err := RevalidateContext(context.Background()); !errors.Is(err, ErrRedirectLoop) { + t.Fatalf("expected a redirect loop error, got %v", err) + } +} + +// With no routed component mounted there is nothing to revalidate. +func TestRevalidateWithoutAMountedRouteDoesNothing(t *testing.T) { + resetRouter(t) + session := registerSessionRoute(t, "/reval-idle", func(map[string]string) GuardResult { return Forbid() }) + + if err := RevalidateContext(context.Background()); err != nil { + t.Fatalf("revalidate: %v", err) + } + if session.guards != 0 { + t.Fatalf("guards ran with nothing mounted: %d", session.guards) + } + if Status().Get() != NavigationIdle || Error().Get() != nil { + t.Fatalf("navigation state: status=%s error=%v", Status().Get(), Error().Get()) + } +} + +// A committed path that matches no route (the not-found component) has no +// guards to re-run, so revalidation leaves it alone. +func TestRevalidateWithoutAMatchingRouteIsANoOp(t *testing.T) { + resetRouter(t) + session := registerSessionRoute(t, "/reval-present", func(map[string]string) GuardResult { return Forbid() }) + NotFoundComponent = func() core.Component { return newRevalidatePage("not-found") } + + if err := NavigateContext(context.Background(), "/reval-absent"); err != nil { + t.Fatalf("navigate: %v", err) + } + page := mustRevalidatePage(t, CurrentComponent()) + + if err := RevalidateContext(context.Background()); err != nil { + t.Fatalf("revalidate: %v", err) + } + if session.guards != 0 { + t.Fatalf("guards of another route ran: %d", session.guards) + } + if CurrentComponent() != core.Component(page) || page.unmounted != 0 { + t.Fatalf("the not-found page was disturbed: current=%#v unmounted=%d", CurrentComponent(), page.unmounted) + } +} + +// A route registered for the path the not-found component is sitting on matches +// it without owning what is mounted: its guards never ran to commit anything +// there, so revalidation leaves the component alone instead of letting them +// decide the fate of a page that is not theirs. +func TestRevalidateIgnoresARouteThatDoesNotOwnTheMountedComponent(t *testing.T) { + resetRouter(t) + NotFoundComponent = func() core.Component { return newRevalidatePage("not-found") } + + if err := NavigateContext(context.Background(), "/reval-late"); err != nil { + t.Fatalf("navigate: %v", err) + } + page := mustRevalidatePage(t, CurrentComponent()) + + // The route arrives afterwards, the shape of a feature that registers its + // routes when it is installed rather than at startup. + session := registerSessionRoute(t, "/reval-late", func(map[string]string) GuardResult { return Forbid() }) + + if err := RevalidateContext(context.Background()); err != nil { + t.Fatalf("revalidate: %v", err) + } + if session.guards != 0 { + t.Fatalf("the guards of a route that owns nothing ran %d times", session.guards) + } + if CurrentComponent() != core.Component(page) || page.unmounted != 0 { + t.Fatalf("the mounted component was disturbed: current=%#v unmounted=%d", CurrentComponent(), page.unmounted) + } + if got := ActivePath().Get(); got != "/reval-late" { + t.Fatalf("active path = %q, want the committed path", got) + } +} + +// A cancelled context is answered before the guards run, and the mounted route +// stays where it is. +func TestRevalidateCancelledContextSkipsGuards(t *testing.T) { + resetRouter(t) + allowed := true + session := registerSessionRoute(t, "/reval-cancelled", func(map[string]string) GuardResult { + if !allowed { + return Forbid() + } + return Allow() + }) + + if err := NavigateContext(context.Background(), "/reval-cancelled"); err != nil { + t.Fatalf("navigate: %v", err) + } + page := mustRevalidatePage(t, CurrentComponent()) + guards := session.guards + + allowed = false + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + if err := RevalidateContext(ctx); !errors.Is(err, context.Canceled) { + t.Fatalf("expected a cancelled revalidation, got %v", err) + } + if session.guards != guards { + t.Fatalf("guards ran for a cancelled revalidation: %d", session.guards) + } + if CurrentComponent() != core.Component(page) || page.unmounted != 0 { + t.Fatalf("a cancelled revalidation tore the page down: current=%#v unmounted=%d", CurrentComponent(), page.unmounted) + } +} + +// A loader still running was authorized under the snapshot revalidation +// replaces: it must not commit afterwards, whether the mounted route survives +// the revalidation or not. +func TestRevalidateCancelsAnInFlightLoader(t *testing.T) { + resetRouter(t) + allowed := true + registerSessionRoute(t, "/reval-current", func(map[string]string) GuardResult { + if !allowed { + return Forbid() + } + return Allow() + }) + entered, release := make(chan struct{}, 2), make(chan struct{}) + pending := 0 + RegisterRoute(Route{ + Path: "/reval-pending", + Component: func() core.Component { + pending++ + return newRevalidatePage("pending") + }, + Loader: func(ctx context.Context, _ LoadContext) (any, error) { + entered <- struct{}{} + <-release + return "late", nil + }, + }) + + if err := NavigateContext(context.Background(), "/reval-current"); err != nil { + t.Fatalf("navigate: %v", err) + } + page := mustRevalidatePage(t, CurrentComponent()) + + // The route is still allowed: the in-flight navigation is cancelled and the + // mounted page stays. + navigation := make(chan error, 1) + go func() { navigation <- NavigateContext(context.Background(), "/reval-pending") }() + <-entered + + if err := RevalidateContext(context.Background()); err != nil { + t.Fatalf("revalidate: %v", err) + } + release <- struct{}{} + if err := <-navigation; err == nil { + t.Fatal("the cancelled navigation reported success") + } + if CurrentComponent() != core.Component(page) { + t.Fatalf("a cancelled loader committed over the mounted route: %#v", CurrentComponent()) + } + if pending != 0 { + t.Fatalf("the cancelled destination was created %d times", pending) + } + if Status().Get() != NavigationReady || Error().Get() != nil { + t.Fatalf("navigation state: status=%s error=%v", Status().Get(), Error().Get()) + } + if Data().Get() != "payload" { + t.Fatalf("route data = %#v, want the mounted route's", Data().Get()) + } + + // Now the guard refuses: the same cancelled loader must not resurrect the + // route the revalidation tore down. + go func() { navigation <- NavigateContext(context.Background(), "/reval-pending") }() + <-entered + allowed = false + + if err := RevalidateContext(context.Background()); !errors.Is(err, ErrNavigationForbidden) { + t.Fatalf("expected a forbidden revalidation, got %v", err) + } + close(release) + if err := <-navigation; err == nil { + t.Fatal("the cancelled navigation reported success") + } + if CurrentComponent() != nil { + t.Fatalf("a cancelled loader committed after the revocation: %#v", CurrentComponent()) + } + if page.unmounted != 1 || page.cleanups != 1 { + t.Fatalf("teardown ran %d times (cleanups %d), want 1", page.unmounted, page.cleanups) + } + if pending != 0 { + t.Fatalf("the cancelled destination was created %d times", pending) + } +} + +// Revalidation is repeatable: an allowed route survives any number of them, and +// a refused one is torn down exactly once however often it is asked again. +func TestRevalidateIsIdempotentUnderRepetition(t *testing.T) { + resetRouter(t) + allowed := true + session := registerSessionRoute(t, "/reval-stress", func(map[string]string) GuardResult { + if !allowed { + return Forbid() + } + return Allow() + }) + + if err := NavigateContext(context.Background(), "/reval-stress"); err != nil { + t.Fatalf("navigate: %v", err) + } + page := mustRevalidatePage(t, CurrentComponent()) + + const rounds = 50 + for i := 0; i < rounds; i++ { + if err := RevalidateContext(context.Background()); err != nil { + t.Fatalf("revalidation %d: %v", i, err) + } + } + if session.guards != rounds+1 { + t.Fatalf("guard ran %d times, want %d", session.guards, rounds+1) + } + if session.created != 1 || session.loaded != 1 || page.unmounted != 0 { + t.Fatalf("repeated revalidation re-ran the route: created=%d loaded=%d unmounted=%d", session.created, session.loaded, page.unmounted) + } + + allowed = false + refusals := 0 + for i := 0; i < rounds; i++ { + if err := RevalidateContext(context.Background()); err != nil { + refusals++ + } + } + if refusals != 1 { + t.Fatalf("%d revalidations refused, want 1", refusals) + } + if page.unmounted != 1 || page.cleanups != 1 { + t.Fatalf("teardown ran %d times (cleanups %d), want 1", page.unmounted, page.cleanups) + } + if CurrentComponent() != nil { + t.Fatalf("the revoked route stayed current: %#v", CurrentComponent()) + } +} diff --git a/router/router_revalidate_wasm_test.go b/router/router_revalidate_wasm_test.go new file mode 100644 index 00000000..91984fb7 --- /dev/null +++ b/router/router_revalidate_wasm_test.go @@ -0,0 +1,448 @@ +//go:build js && wasm + +package router + +import ( + "context" + "errors" + "strings" + "testing" + + "github.com/rfwlab/rfw/v2/core" + "github.com/rfwlab/rfw/v2/dom" + "github.com/rfwlab/rfw/v2/js" + "github.com/rfwlab/rfw/v2/state" +) + +// mountRevalidateShell renders a shell with a router outlet into #app and +// mounts page behind guard on path. page must carry the "reval" marker, which +// is filled after mount the way a fetch callback fills a card: a remount or a +// stray re-render would wipe it. It returns the shell so a caller can hold on +// to the root MountRoot mounted. +func mountRevalidateShell(t *testing.T, path string, page *hostGuardPage, guard ResultGuard) *core.HTMLComponent { + t.Helper() + if dom.ByID("app").IsNull() { + host := dom.CreateElement("div") + host.SetAttr("id", "app") + dom.Doc().Body().AppendChild(host) + } + shell := core.NewHTMLComponent("RevalShell", []byte(`@include:outlet`), nil) + shell.SetComponent(shell) + shell.AddDependency("outlet", NewOutlet()) + shell.Init(nil) + + RegisterRoute(Route{ + Path: path, + Component: page, + ResultGuards: []ResultGuard{guard}, + Loader: func(context.Context, LoadContext) (any, error) { return "payload", nil }, + Meta: map[string]any{"section": "session"}, + }) + MountRoot(shell) + Navigate(path) + + dom.Query("[data-host-page='reval']").SetHTML(`built`) + if !strings.Contains(dom.ByID("app").HTML(), "reval-built") { + t.Fatal("the mounted page was not rendered") + } + return shell +} + +// mustComponentRoot resolves a component's own root node. dom.ComponentRoot +// answers #app for an id it cannot resolve, so the id it carries is checked: +// asserting on the fallback would compare the shell to the container around it. +func mustComponentRoot(t *testing.T, id string) dom.Element { + t.Helper() + root := dom.ComponentRoot(id) + if root.IsNull() || root.IsUndefined() || root.Attr("data-component-id") != id { + t.Fatalf("component %s has no root of its own: %s", id, dom.ByID("app").HTML()) + } + return root +} + +// An allowed revalidation touches no DOM at all, and a refused one removes the +// routed subtree while the shell and the outlet marker around it stay the very +// nodes they were: the next navigation renders into the page the user is still +// looking at. +func TestRevalidateClearsTheRoutedSubtreeAndKeepsTheShell(t *testing.T) { + Reset() + restoreHistoryPath(t) + + allowed := true + page := newHostGuardPage("RevalPage", "reval") + shell := mountRevalidateShell(t, "/reval-dom", page, func(map[string]string) GuardResult { + if !allowed { + return Forbid() + } + return Allow() + }) + if Data().Get() != "payload" || Meta().Get()["section"] != "session" { + t.Fatalf("the route committed no state to revoke: data=%#v meta=%#v", Data().Get(), Meta().Get()) + } + appNode := dom.ByID("app") + shellNode := mustComponentRoot(t, shell.GetID()) + outletNode := dom.Query("[data-router-outlet]") + historyLength := js.History().Get("length").Int() + location := js.Location().Get("pathname").String() + + if err := RevalidateContext(context.Background()); err != nil { + t.Fatalf("revalidate: %v", err) + } + if !strings.Contains(dom.ByID("app").HTML(), "reval-built") { + t.Fatalf("an allowed revalidation re-rendered the page: %s", dom.ByID("app").HTML()) + } + if page.unmounted != 0 { + t.Fatalf("an allowed revalidation unmounted the page %d times", page.unmounted) + } + if got := mustComponentRoot(t, shell.GetID()); !got.Value.Equal(shellNode.Value) { + t.Fatal("an allowed revalidation replaced the shell root") + } + + allowed = false + if err := RevalidateContext(context.Background()); !errors.Is(err, ErrNavigationForbidden) { + t.Fatalf("expected a forbidden revalidation, got %v", err) + } + + if page.unmounted != 1 { + t.Fatalf("the page unmounted %d times, want 1", page.unmounted) + } + if CurrentComponent() != nil { + t.Fatalf("the revoked route stayed current: %#v", CurrentComponent()) + } + if root := dom.Query("[data-host-page='reval']"); !root.IsNull() { + t.Fatalf("the protected subtree is still in the document: %s", dom.ByID("app").HTML()) + } + if strings.Contains(dom.ByID("app").HTML(), "reval-built") { + t.Fatalf("the protected DOM survived the revocation: %s", dom.ByID("app").HTML()) + } + if got := dom.ByID("app"); !got.Value.Equal(appNode.Value) { + t.Fatal("the revocation replaced the application container") + } + if got := mustComponentRoot(t, shell.GetID()); !got.Value.Equal(shellNode.Value) { + t.Fatal("the revocation replaced the shell root MountRoot mounted") + } + if !strings.Contains(shellNode.HTML(), "data-router-outlet") { + t.Fatalf("the revocation emptied the shell instead of the route: %s", shellNode.HTML()) + } + after := dom.Query("[data-router-outlet]") + if after.IsNull() || !after.Value.Equal(outletNode.Value) { + t.Fatal("the revocation replaced the outlet marker instead of emptying it") + } + if html := after.HTML(); html != "" { + t.Fatalf("the outlet still carries the routed subtree: %s", html) + } + if got := js.History().Get("length").Int(); got != historyLength { + t.Fatalf("history length changed from %d to %d", historyLength, got) + } + if got := js.Location().Get("pathname").String(); got != location { + t.Fatalf("the revocation moved the location to %q", got) + } + if got := ActivePath().Get(); got != "/reval-dom" { + t.Fatalf("active path = %q, want the committed path", got) + } + if Status().Get() != NavigationError || !errors.Is(Error().Get(), ErrNavigationForbidden) { + t.Fatalf("navigation state: status=%s error=%v", Status().Get(), Error().Get()) + } + // The DOM is not the only place the route left something behind: what it + // loaded goes with the subtree it was rendered into. + if Data().Get() != nil { + t.Fatalf("the revoked route's data is still readable: %#v", Data().Get()) + } + if meta := Meta().Get(); len(meta) != 0 { + t.Fatalf("the revoked route's metadata is still readable: %#v", meta) + } +} + +// Without an outlet the router owns #app itself: the revocation removes the +// route's own root and leaves the container it was rendered into. #app is what +// dom.ComponentRoot falls back to for an id it cannot resolve, and that +// fallback must never be cleared in the route's place. +func TestRevalidateWithoutAnOutletClearsOnlyTheRouteRoot(t *testing.T) { + Reset() + restoreHistoryPath(t) + previousOutlet := liveOutlet + liveOutlet = nil + t.Cleanup(func() { liveOutlet = previousOutlet }) + + if dom.ByID("app").IsNull() { + host := dom.CreateElement("div") + host.SetAttr("id", "app") + dom.Doc().Body().AppendChild(host) + } + allowed := true + page := newHostGuardPage("RevalBarePage", "bare") + RegisterRoute(Route{ + Path: "/reval-bare", + Component: page, + ResultGuards: []ResultGuard{func(map[string]string) GuardResult { + if !allowed { + return Forbid() + } + return Allow() + }}, + }) + Navigate("/reval-bare") + if dom.Query("[data-host-page='bare']").IsNull() { + t.Fatalf("the page was not rendered: %s", dom.ByID("app").HTML()) + } + appNode := dom.ByID("app") + + allowed = false + if err := RevalidateContext(context.Background()); !errors.Is(err, ErrNavigationForbidden) { + t.Fatalf("expected a forbidden revalidation, got %v", err) + } + + if page.unmounted != 1 { + t.Fatalf("the page unmounted %d times, want 1", page.unmounted) + } + if root := dom.Query("[data-host-page='bare']"); !root.IsNull() { + t.Fatalf("the protected subtree is still in the document: %s", dom.ByID("app").HTML()) + } + if got := dom.ByID("app"); got.IsNull() || !got.Value.Equal(appNode.Value) { + t.Fatal("the revocation removed the application container instead of the route") + } +} + +// A guard that hands off during revalidation performs the browser navigation +// and commits nothing on the way out: the page the user is on survives +// untouched until the browser unloads it. +func TestRevalidateHostReplaceHandsOffWithoutTearingDown(t *testing.T) { + Reset() + restoreHistoryPath(t) + calls := captureHostReplace(t) + + valid := true + page := newHostGuardPage("RevalHostPage", "reval") + mountRevalidateShell(t, "/reval-host-dom", page, func(map[string]string) GuardResult { + if !valid { + return HostReplace("/login?next=%2Freval-host-dom") + } + return Allow() + }) + historyLength := js.History().Get("length").Int() + location := js.Location().Get("pathname").String() + + valid = false + if err := RevalidateContext(context.Background()); err != nil { + t.Fatalf("host handoff: %v", err) + } + + if len(*calls) != 1 || (*calls)[0] != "/login?next=%2Freval-host-dom" { + t.Fatalf("host replace calls = %#v, want one handoff", *calls) + } + if page.unmounted != 0 { + t.Fatalf("the mounted page was unmounted %d times before the handoff", page.unmounted) + } + if CurrentComponent() != core.Component(page) { + t.Fatalf("current component changed to %#v", CurrentComponent()) + } + if !strings.Contains(dom.ByID("app").HTML(), "reval-built") { + t.Fatalf("the handoff tore down the mounted page: %s", dom.ByID("app").HTML()) + } + if got := js.History().Get("length").Int(); got != historyLength { + t.Fatalf("history length changed from %d to %d", historyLength, got) + } + if got := js.Location().Get("pathname").String(); got != location { + t.Fatalf("the SPA moved the location to %q", got) + } + if got := ActivePath().Get(); got != "/reval-host-dom" { + t.Fatalf("active path = %q", got) + } + if Status().Get() != NavigationReady || Error().Get() != nil { + t.Fatalf("navigation state: status=%s error=%v", Status().Get(), Error().Get()) + } + // Nothing was taken back, so the page still holds what it loaded until the + // browser unloads it. + if Data().Get() != "payload" || Meta().Get()["section"] != "session" { + t.Fatalf("the handoff changed route state: data=%#v meta=%#v", Data().Get(), Meta().Get()) + } +} + +// Both redirect actions replace the entry the guard refused: back returns to +// the page before the protected one, never to the protected one itself. +func TestRevalidateRedirectReplacesTheRefusedEntry(t *testing.T) { + for name, decision := range map[string]GuardResult{ + "redirect": RedirectTo("/reval-hist-login"), + "replace": ReplaceWith("/reval-hist-login"), + } { + t.Run(name, func(t *testing.T) { + Reset() + restoreHistoryPath(t) + result := decision + allowed := true + + RegisterRoute(Route{Path: "/reval-hist-base", Component: func() core.Component { return routeComponent{} }}) + RegisterRoute(Route{Path: "/reval-hist-login", Component: func() core.Component { return routeComponent{} }}) + page := newHostGuardPage("RevalHistPage", "hist") + RegisterRoute(Route{ + Path: "/reval-hist-account", + Component: page, + ResultGuards: []ResultGuard{func(map[string]string) GuardResult { + if !allowed { + return result + } + return Allow() + }}, + }) + + Navigate("/reval-hist-base") + Navigate("/reval-hist-account") + before := js.History().Get("length").Int() + + allowed = false + if err := RevalidateContext(context.Background()); err != nil { + t.Fatalf("revalidate: %v", err) + } + + if got := js.Location().Get("pathname").String(); got != "/reval-hist-login" { + t.Fatalf("path after the redirect = %q", got) + } + if got := js.History().Get("length").Int(); got != before { + t.Fatalf("history length changed from %d to %d", before, got) + } + if page.unmounted != 1 { + t.Fatalf("the refused page unmounted %d times, want 1", page.unmounted) + } + if got := ActivePath().Get(); got != "/reval-hist-login" { + t.Fatalf("active path = %q", got) + } + if got := travelHistory(t, "back"); got != "/reval-hist-base" { + t.Fatalf("back landed on %q, want /reval-hist-base", got) + } + }) + } +} + +// The revocation leaves the refused path in the address bar, so the browser can +// come back to it. It does, through the guards: a back and a forward cannot put +// the protected page back on screen. +func TestRevalidateRevocationSurvivesBackAndForward(t *testing.T) { + Reset() + restoreHistoryPath(t) + + allowed, guards := true, 0 + RegisterRoute(Route{Path: "/reval-back-base", Component: func() core.Component { return routeComponent{} }}) + page := newHostGuardPage("RevalBackPage", "back") + RegisterRoute(Route{ + Path: "/reval-back-guarded", + Component: page, + ResultGuards: []ResultGuard{func(map[string]string) GuardResult { + guards++ + if !allowed { + return Forbid() + } + return Allow() + }}, + }) + + Navigate("/reval-back-base") + Navigate("/reval-back-guarded") + + allowed = false + if err := RevalidateContext(context.Background()); !errors.Is(err, ErrNavigationForbidden) { + t.Fatalf("expected a forbidden revalidation, got %v", err) + } + if root := dom.Query("[data-host-page='back']"); !root.IsNull() { + t.Fatal("the protected subtree survived the revocation") + } + + back := travelHistory(t, "back") + if back != "/reval-back-base" { + t.Fatalf("back landed on %q, want /reval-back-base", back) + } + // What the popstate listener InitRouter installs does with the new URL. + navigate(back, historyNone) + before := guards + + forward := travelHistory(t, "forward") + if forward != "/reval-back-guarded" { + t.Fatalf("forward landed on %q, want /reval-back-guarded", forward) + } + navigate(forward, historyNone) + + if guards <= before { + t.Fatal("forward restored the refused path without running its guards") + } + if root := dom.Query("[data-host-page='back']"); !root.IsNull() { + t.Fatalf("forward put the protected page back on screen: %s", dom.ByID("app").HTML()) + } + if CurrentComponent() == core.Component(page) { + t.Fatal("forward remounted the revoked page") + } + if page.unmounted != 1 { + t.Fatalf("the page unmounted %d times, want 1", page.unmounted) + } +} + +// revalidateGate stands in for the ownership gate a host registration hands to +// the frames delivered under it: releasing the registration closes it. +type revalidateGate struct{ closed bool } + +func (g *revalidateGate) open() bool { return !g.closed } + +// The revocation runs the component's own cleanup: the scope closes (the +// disconnect a long-lived session such as a noVNC client hangs off it), the +// owned host registration is released, and the signals go with it. A frame +// still in flight for that component then finds no root, no signals, and a +// closed gate. +func TestRevalidateReleasesOwnedHostWorkAndIgnoresLateFrames(t *testing.T) { + Reset() + restoreHistoryPath(t) + + gate := &revalidateGate{} + releases := 0 + core.SetHostRegistrar(func(string, string, []string) func() { + return func() { + releases++ + gate.closed = true + } + }) + t.Cleanup(func() { core.SetHostRegistrar(nil) }) + + allowed := true + page := newHostGuardPage("RevalOwnedPage", "reval") + // Declared before the mount, the way a page that talks to a host component + // declares it in its constructor: the mount is what opens the registration. + page.AddHostComponent("RevalHostComponent") + mountRevalidateShell(t, "/reval-host-owned", page, func(map[string]string) GuardResult { + if !allowed { + return Forbid() + } + return Allow() + }) + + disconnects := 0 + page.Scope().Defer(func() { disconnects++ }) + signal := state.NewSignal("connected") + dom.RegisterSignal(page.GetID(), "status", signal) + + if releases != 0 { + t.Fatalf("the registration was released %d times before the revocation", releases) + } + + allowed = false + if err := RevalidateContext(context.Background()); !errors.Is(err, ErrNavigationForbidden) { + t.Fatalf("expected a forbidden revalidation, got %v", err) + } + + if releases != 1 { + t.Fatalf("the host registration was released %d times, want 1", releases) + } + if disconnects != 1 { + t.Fatalf("scope cleanup ran %d times, want 1", disconnects) + } + if signals := dom.SnapshotComponentSignals(page.GetID()); len(signals) != 0 { + t.Fatalf("the revoked component still exposes host signals: %#v", signals) + } + // A late frame resolves its target the way host delivery does, on the exact + // id and never through the #app fallback. + if root := dom.Doc().Query("[data-component-id='" + page.GetID() + "']"); !root.IsNull() { + t.Fatal("a late frame would still find the revoked component's root") + } + if signal.SetFromHostGated("hijacked", gate.open) { + t.Fatal("a late host frame wrote into the revoked component's signal") + } + if got := signal.Get(); got != "connected" { + t.Fatalf("the revoked component's signal changed to %q", got) + } +} From 07566500ba03a7fdbc0143bcda886059ad48bfd1 Mon Sep 17 00:00:00 2001 From: Pietro di Caprio Date: Sun, 30 Aug 2026 07:51:10 +0200 Subject: [PATCH 6/6] test(router): satisfy revalidation lint --- router/router_revalidate_test.go | 2 +- router/router_revalidate_wasm_test.go | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/router/router_revalidate_test.go b/router/router_revalidate_test.go index 8ebb0c5a..4099b62c 100644 --- a/router/router_revalidate_test.go +++ b/router/router_revalidate_test.go @@ -592,7 +592,7 @@ func TestRevalidateCancelsAnInFlightLoader(t *testing.T) { pending++ return newRevalidatePage("pending") }, - Loader: func(ctx context.Context, _ LoadContext) (any, error) { + Loader: func(_ context.Context, _ LoadContext) (any, error) { entered <- struct{}{} <-release return "late", nil diff --git a/router/router_revalidate_wasm_test.go b/router/router_revalidate_wasm_test.go index 91984fb7..8105b94f 100644 --- a/router/router_revalidate_wasm_test.go +++ b/router/router_revalidate_wasm_test.go @@ -94,7 +94,7 @@ func TestRevalidateClearsTheRoutedSubtreeAndKeepsTheShell(t *testing.T) { if page.unmounted != 0 { t.Fatalf("an allowed revalidation unmounted the page %d times", page.unmounted) } - if got := mustComponentRoot(t, shell.GetID()); !got.Value.Equal(shellNode.Value) { + if got := mustComponentRoot(t, shell.GetID()); !got.Equal(shellNode.Value) { t.Fatal("an allowed revalidation replaced the shell root") } @@ -115,17 +115,17 @@ func TestRevalidateClearsTheRoutedSubtreeAndKeepsTheShell(t *testing.T) { if strings.Contains(dom.ByID("app").HTML(), "reval-built") { t.Fatalf("the protected DOM survived the revocation: %s", dom.ByID("app").HTML()) } - if got := dom.ByID("app"); !got.Value.Equal(appNode.Value) { + if got := dom.ByID("app"); !got.Equal(appNode.Value) { t.Fatal("the revocation replaced the application container") } - if got := mustComponentRoot(t, shell.GetID()); !got.Value.Equal(shellNode.Value) { + if got := mustComponentRoot(t, shell.GetID()); !got.Equal(shellNode.Value) { t.Fatal("the revocation replaced the shell root MountRoot mounted") } if !strings.Contains(shellNode.HTML(), "data-router-outlet") { t.Fatalf("the revocation emptied the shell instead of the route: %s", shellNode.HTML()) } after := dom.Query("[data-router-outlet]") - if after.IsNull() || !after.Value.Equal(outletNode.Value) { + if after.IsNull() || !after.Equal(outletNode.Value) { t.Fatal("the revocation replaced the outlet marker instead of emptying it") } if html := after.HTML(); html != "" { @@ -198,7 +198,7 @@ func TestRevalidateWithoutAnOutletClearsOnlyTheRouteRoot(t *testing.T) { if root := dom.Query("[data-host-page='bare']"); !root.IsNull() { t.Fatalf("the protected subtree is still in the document: %s", dom.ByID("app").HTML()) } - if got := dom.ByID("app"); got.IsNull() || !got.Value.Equal(appNode.Value) { + if got := dom.ByID("app"); got.IsNull() || !got.Equal(appNode.Value) { t.Fatal("the revocation removed the application container instead of the route") } }