From 1e28cbd115c07090faf155462af5abfd82eeafe0 Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Sat, 4 Apr 2026 14:12:43 -0400 Subject: [PATCH 1/9] Add e2e tests for URL parameter isolation between listing and DLP These tests verify that listing-specific query params (page, sortOption, sortDir, showDrafts, showEmpty, search, pos) do not leak into Dandiset Landing Page URLs when navigating from listing/search pages. Expected to FAIL on current code and PASS after the fix in the next commit. Ref: https://github.com/dandi/dandi-archive/issues/1460 Co-Authored-By: Claude Code 2.1.92 / Claude Opus 4.6 --- e2e/tests/urlParams.spec.ts | 140 ++++++++++++++++++++++++++++++++++++ 1 file changed, 140 insertions(+) create mode 100644 e2e/tests/urlParams.spec.ts diff --git a/e2e/tests/urlParams.spec.ts b/e2e/tests/urlParams.spec.ts new file mode 100644 index 000000000..b941d46fb --- /dev/null +++ b/e2e/tests/urlParams.spec.ts @@ -0,0 +1,140 @@ +import { expect, test } from "@playwright/test"; +import { clientUrl, registerNewUser, registerDandiset } from "../utils.ts"; +import { faker } from "@faker-js/faker"; + +/** + * Tests that listing query parameters (page, sortOption, sortDir, showDrafts, + * showEmpty, search, pos) do not leak into Dandiset Landing Page (DLP) URLs, + * and vice-versa. See https://github.com/dandi/dandi-archive/issues/1460 + */ + +const LISTING_PARAMS = ["page", "sortOption", "sortDir", "showDrafts", "showEmpty", "search", "pos"]; + +/** Assert that none of the listing-specific params appear in the current URL. */ +function expectCleanDlpUrl(url: string) { + const parsed = new URL(url); + for (const param of LISTING_PARAMS) { + expect(parsed.searchParams.has(param), `URL should not contain '${param}': ${url}`).toBeFalsy(); + } +} + +test.describe("URL parameter isolation (issue #1460)", () => { + test("listing params do not leak to DLP when clicking a dandiset", async ({ page }) => { + await registerNewUser(page); + const name = faker.lorem.words(); + const description = faker.lorem.sentences(); + await registerDandiset(page, name, description); + + // Go to the public dandisets listing + await page.goto(`${clientUrl}/dandiset`); + await page.waitForLoadState("networkidle"); + + // Click the first dandiset in the list + await page.locator(".v-list-item").first().click(); + await page.waitForLoadState("networkidle"); + + // DLP URL must be clean + expectCleanDlpUrl(page.url()); + // Should be on a dandiset page + expect(page.url()).toMatch(/\/dandiset\/\d+/); + }); + + test("search params do not leak to DLP", async ({ page }) => { + await registerNewUser(page); + const name = `searchtest-${faker.lorem.word()}`; + const description = faker.lorem.sentences(); + await registerDandiset(page, name, description); + + // Navigate to search with a query + await page.goto(`${clientUrl}/dandiset/search?search=${name}`); + await page.waitForLoadState("networkidle"); + + // Verify search results appear + const items = page.locator(".v-list-item"); + await expect(items.first()).toBeVisible(); + + // Click the first result + await items.first().click(); + await page.waitForLoadState("networkidle"); + + // DLP URL must be clean — no search param + expectCleanDlpUrl(page.url()); + }); + + test("direct DLP navigation produces clean URL", async ({ page }) => { + await registerNewUser(page); + const name = faker.lorem.words(); + const description = faker.lorem.sentences(); + const dandisetId = await registerDandiset(page, name, description); + + // Navigate directly to the DLP + await page.goto(`${clientUrl}/dandiset/${dandisetId}`); + await page.waitForLoadState("networkidle"); + + expectCleanDlpUrl(page.url()); + }); + + test("DLP pagination does not add listing params to URL", async ({ page }) => { + test.slow(); + await registerNewUser(page); + + // Create multiple dandisets so pagination has something to page through + for (let i = 0; i < 3; i += 1) { + await registerDandiset(page, faker.lorem.words(), faker.lorem.sentences()); + } + + // Go to listing and click a dandiset + await page.goto(`${clientUrl}/dandiset`); + await page.waitForLoadState("networkidle"); + await page.locator(".v-list-item").first().click(); + await page.waitForLoadState("networkidle"); + + // If pagination is visible, click next and verify URL stays clean + const nextButton = page.locator(".v-pagination__next button"); + if (await nextButton.isVisible()) { + await nextButton.click(); + await page.waitForLoadState("networkidle"); + expectCleanDlpUrl(page.url()); + } + }); + + test("back button from DLP restores listing params", async ({ page }) => { + await registerNewUser(page); + await registerDandiset(page, faker.lorem.words(), faker.lorem.sentences()); + + // Go to listing + await page.goto(`${clientUrl}/dandiset`); + await page.waitForLoadState("networkidle"); + + // Capture listing URL + const listingUrl = page.url(); + + // Click into a dandiset + await page.locator(".v-list-item").first().click(); + await page.waitForLoadState("networkidle"); + + // Go back + await page.goBack(); + await page.waitForLoadState("networkidle"); + + // Should be back on listing with its own params intact + const backUrl = new URL(page.url()); + expect(backUrl.pathname).toBe("/dandiset"); + }); + + test("manually appended listing params are stripped from DLP URL", async ({ page }) => { + await registerNewUser(page); + const name = faker.lorem.words(); + const description = faker.lorem.sentences(); + const dandisetId = await registerDandiset(page, name, description); + + // Navigate to DLP with manually injected listing params + await page.goto( + `${clientUrl}/dandiset/${dandisetId}?page=2&sortOption=1&sortDir=-1&showDrafts=true&showEmpty=false&search=test&pos=5` + ); + await page.waitForLoadState("networkidle"); + + // The router guard should have stripped all listing params + expectCleanDlpUrl(page.url()); + }); +}); From 280c8b16b9b76bfd1c0fbc2ae0a7cc2e7b0e835c Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Sat, 4 Apr 2026 14:54:57 -0400 Subject: [PATCH 2/9] Fix URL parameter leakage from listing pages to DLP Listing query params (page, sortOption, sortDir, showDrafts, showEmpty, search, pos) were spread into DLP URLs via `...$route.query` in DandisetList.vue, producing ugly unshareable URLs like: /dandiset/000027?page=2&sortOption=0&sortDir=NaN&showDrafts=true&... Fix: move listing navigation context into a Pinia store so the DLP "page through dandisets" feature works without polluting the URL. Add a defensive router guard to strip any listing params that reach the DLP route. Fixes: https://github.com/dandi/dandi-archive/issues/1460 Co-Authored-By: Claude Code 2.1.92 / Claude Opus 4.6 --- web/src/components/DandisetList.vue | 7 ++- web/src/components/DandisetsPage.vue | 18 +++++++ web/src/router/index.ts | 26 +++++++++- web/src/stores/listingContext.ts | 52 +++++++++++++++++++ .../DandisetLandingView.vue | 28 +++++----- 5 files changed, 117 insertions(+), 14 deletions(-) create mode 100644 web/src/stores/listingContext.ts diff --git a/web/src/components/DandisetList.vue b/web/src/components/DandisetList.vue index 34153af17..c875db433 100644 --- a/web/src/components/DandisetList.vue +++ b/web/src/components/DandisetList.vue @@ -10,9 +10,9 @@ :to="{ name: 'dandisetLanding', params: { identifier: item.dandiset.identifier }, - query: { ...$route.query, pos: getPos(index) }, }" exact + @click="savePos(index)" > {{ item.name }} @@ -85,6 +85,7 @@ import moment from 'moment'; import { filesize } from 'filesize'; import StarButton from '@/components/StarButton.vue'; import { dandiRest } from '@/rest'; +import { useListingContextStore } from '@/stores/listingContext'; import type { Version } from '@/types'; import { DANDISETS_PER_PAGE } from '@/utils/constants'; @@ -97,6 +98,7 @@ defineProps({ }); const route = useRoute(); +const listingContext = useListingContextStore(); const archiveName = ref(''); onMounted(async () => { @@ -108,6 +110,9 @@ onMounted(async () => { function getPos(index: number) { return (Number(route.query.page || 1) - 1) * DANDISETS_PER_PAGE + (index + 1); } +function savePos(index: number) { + listingContext.setContext({ pos: getPos(index) }); +} function formatDate(date: string) { return moment(date).format('LL'); } diff --git a/web/src/components/DandisetsPage.vue b/web/src/components/DandisetsPage.vue index 1529be604..9d09801bc 100644 --- a/web/src/components/DandisetsPage.vue +++ b/web/src/components/DandisetsPage.vue @@ -187,6 +187,7 @@ import { useRoute } from 'vue-router'; import DandisetList from '@/components/DandisetList.vue'; import DandisetSearchField from '@/components/DandisetSearchField.vue'; import { dandiRest } from '@/rest'; +import { useListingContextStore } from '@/stores/listingContext'; import type { Dandiset, Paginated, Version } from '@/types'; import { sortingOptions, DANDISETS_PER_PAGE } from '@/utils/constants'; import router from '@/router'; @@ -276,6 +277,23 @@ watch(queryParams, (params) => { }, }); }); + +// Keep the listing context store in sync so the DLP can use it for pagination +// without needing URL query params. +const listingContext = useListingContextStore(); +watch( + [sortOption, sortDir, showDrafts, showEmpty, () => route.query.search], + () => { + listingContext.setContext({ + sortOption: sortOption.value, + sortDir: sortDir.value, + showDrafts: showDrafts.value, + showEmpty: showEmpty.value, + search: (route.query.search as string) || null, + }); + }, + { immediate: true }, +);