From 723d277cfbd2b681e772c540f24174ef6e130b85 Mon Sep 17 00:00:00 2001 From: ShaneK Date: Wed, 13 May 2026 14:18:12 -0700 Subject: [PATCH] fix(vue-router): prevent out-of-bounds index when popping across tabs --- packages/vue-router/src/viewStacks.ts | 14 +- .../tests/unit/tabs-single-outlet.spec.ts | 144 ++++++++++++++++++ 2 files changed, 156 insertions(+), 2 deletions(-) create mode 100644 packages/vue/test/base/tests/unit/tabs-single-outlet.spec.ts diff --git a/packages/vue-router/src/viewStacks.ts b/packages/vue-router/src/viewStacks.ts index 8c9f0cbefca..0e00d22de9c 100644 --- a/packages/vue-router/src/viewStacks.ts +++ b/packages/vue-router/src/viewStacks.ts @@ -196,8 +196,15 @@ export const createViewStacks = (router: Router) => { if (!viewStack) return; const startIndex = viewStack.findIndex((v) => v === viewItem); + if (startIndex === -1) return; - for (let i = startIndex + 1; i < startIndex - delta; i++) { + // delta from popstate reflects browser history depth, which can exceed + // the outlet's view stack when tab switches build up history without + // adding new view items. Clamp to the stack length so we never index + // past the end of the array. + const endIndex = Math.min(viewStack.length, startIndex - delta); + + for (let i = startIndex + 1; i < endIndex; i++) { const viewItem = viewStack[i]; viewItem.mount = false; viewItem.ionPageElement = undefined; @@ -233,8 +240,11 @@ export const createViewStacks = (router: Router) => { if (!viewStack) return; const startIndex = viewStack.findIndex((v) => v === viewItem); + if (startIndex === -1) return; + + const endIndex = Math.min(viewStack.length, startIndex + delta); - for (let i = startIndex + 1; i < startIndex + delta; i++) { + for (let i = startIndex + 1; i < endIndex; i++) { viewStack[i].mount = true; } }; diff --git a/packages/vue/test/base/tests/unit/tabs-single-outlet.spec.ts b/packages/vue/test/base/tests/unit/tabs-single-outlet.spec.ts new file mode 100644 index 00000000000..84bcdee9fd9 --- /dev/null +++ b/packages/vue/test/base/tests/unit/tabs-single-outlet.spec.ts @@ -0,0 +1,144 @@ +import { enableAutoUnmount, mount } from '@vue/test-utils'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { createRouter, createWebHistory } from '@ionic/vue-router'; +import { + IonicVue, + IonApp, + IonRouterOutlet, + IonPage, + IonTabs, + IonTabBar, + IonTabButton, + IonLabel, +} from '@ionic/vue'; +import { waitForRouter } from './utils'; + +enableAutoUnmount(afterEach); + +/** + * Reproduces the apps-without-an-outer-outlet shape: mounted at + * the root of the app, with the inner as the ONLY outlet + * in the viewStacks. With this shape, `usingLinearNavigation` (size === 1) is + * true, so unmountLeavingViews / mountIntermediaryViews are invoked when + * router.go(-N) crosses tabs. Several visits across tabs can grow the + * popstate delta beyond the inner outlet's stack length, exposing + * out-of-bounds reads in those helpers. + */ + +const makeTabPage = (id: string) => ({ + template: ``, + components: { IonPage }, +}); + +const Tab1 = makeTabPage('tab1'); +const Tab1Sub = makeTabPage('tab1sub'); +const Tab2 = makeTabPage('tab2'); +const Tab2Sub = makeTabPage('tab2sub'); +const Tab3 = makeTabPage('tab3'); +const Tab3Sub = makeTabPage('tab3sub'); + +const TabsApp = { + components: { + IonApp, + IonTabs, + IonTabBar, + IonTabButton, + IonLabel, + IonRouterOutlet, + }, + template: ` + + + + + Tab 1 + Tab 2 + Tab 3 + + + + `, +}; + +const buildRouter = () => + createRouter({ + history: createWebHistory(process.env.BASE_URL), + routes: [ + { path: '/', redirect: '/tab1' }, + { path: '/tab1', component: Tab1 }, + { path: '/tab1/sub', component: Tab1Sub }, + { path: '/tab2', component: Tab2 }, + { path: '/tab2/sub', component: Tab2Sub }, + { path: '/tab3', component: Tab3 }, + { path: '/tab3/sub', component: Tab3Sub }, + ], + }); + +describe('ion-tabs at root (single outlet)', () => { + it('does not throw when navigating across tabs and sub-pages', async () => { + const router = buildRouter(); + + // Capture both Vue runtime errors and unhandled promise rejections. + // `unmountLeavingViews` runs inside a Vue reactive effect, so a throw + // ends up as a Promise rejection rather than a synchronous error. + const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + const errors: unknown[] = []; + const onWindowUnhandled = (event: { reason?: unknown; preventDefault?: () => void }) => { + errors.push(event.reason ?? event); + event.preventDefault?.(); + }; + const onProcessUnhandled = (reason: unknown) => { + errors.push(reason); + }; + (globalThis as any).addEventListener('unhandledrejection', onWindowUnhandled); + (process as any).on('unhandledRejection', onProcessUnhandled); + + try { + router.push('/tab1'); + await router.isReady(); + + mount(TabsApp, { + global: { + plugins: [router, IonicVue], + config: { + errorHandler: (err: unknown) => { + errors.push(err); + }, + }, + }, + }); + await waitForRouter(); + + // Build up browser history across tabs and sub-pages, the way the + // customer's repro does. Each push adds a history entry while reusing + // any existing view item for the destination route. + router.push('/tab1/sub'); await waitForRouter(); + router.push('/tab2'); await waitForRouter(); + router.push('/tab3'); await waitForRouter(); + router.push('/tab3/sub'); await waitForRouter(); + router.push('/tab2'); await waitForRouter(); + router.push('/tab1/sub'); await waitForRouter(); + + // Now: history position is large (7+), but the inner outlet's view stack + // only ever held [tab1, tab1sub, tab2, tab3, tab3sub] at most. Jump back + // to /tab1 with a delta that exceeds the stack-above-startIndex count. + router.go(-6); + await waitForRouter(); + + // Give any pending microtasks/promise rejections a chance to flush. + await waitForRouter(); + } finally { + consoleErrorSpy.mockRestore(); + (globalThis as any).removeEventListener('unhandledrejection', onWindowUnhandled); + (process as any).off('unhandledRejection', onProcessUnhandled); + } + + // Any error thrown during the navigation above is a regression for this + // scenario; the original bug surfaced as a TypeError from indexing past + // the end of the inner outlet's view stack. + expect( + errors, + `errors: ${errors.map((e) => (e instanceof Error ? e.stack : String(e))).join('\n')}`, + ).toEqual([]); + }); +});