From 66674c72773e7b68dc2c32d06491d6ebc5ad9ad8 Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Thu, 22 Dec 2022 13:09:07 +0000 Subject: [PATCH] Ensure view names are properly encoded to handle certain special characters (#9145) --- .../DataTable/modals/CreateViewModal.svelte | 3 ++- .../TableNavigator/TableNavigator.svelte | 5 ++--- .../popovers/EditViewPopover.svelte | 3 ++- packages/builder/src/helpers/urlStateSync.js | 20 ++++++++++++++++--- .../data/view/[viewName]/_layout.svelte | 1 + .../app/[application]/data/view/index.svelte | 4 ++-- packages/frontend-core/src/api/views.js | 6 +++--- .../src/api/controllers/row/internal.ts | 2 +- .../server/src/api/controllers/view/index.ts | 4 ++-- 9 files changed, 32 insertions(+), 16 deletions(-) diff --git a/packages/builder/src/components/backend/DataTable/modals/CreateViewModal.svelte b/packages/builder/src/components/backend/DataTable/modals/CreateViewModal.svelte index 7e34e347e6..8f2679f874 100644 --- a/packages/builder/src/components/backend/DataTable/modals/CreateViewModal.svelte +++ b/packages/builder/src/components/backend/DataTable/modals/CreateViewModal.svelte @@ -10,6 +10,7 @@ $: views = $tables.list.flatMap(table => Object.keys(table.views || {})) const saveView = async () => { + name = name?.trim() if (views.includes(name)) { notifications.error(`View exists with name ${name}`) return @@ -21,7 +22,7 @@ field, }) notifications.success(`View ${name} created`) - $goto(`../../view/${name}`) + $goto(`../../view/${encodeURIComponent(name)}`) } catch (error) { notifications.error("Error creating view") } diff --git a/packages/builder/src/components/backend/TableNavigator/TableNavigator.svelte b/packages/builder/src/components/backend/TableNavigator/TableNavigator.svelte index 2ad33e9477..182ac49314 100644 --- a/packages/builder/src/components/backend/TableNavigator/TableNavigator.svelte +++ b/packages/builder/src/components/backend/TableNavigator/TableNavigator.svelte @@ -36,9 +36,8 @@ indentLevel={2} icon="Remove" text={viewName} - selected={$isActive("./view/:viewName") && - $views.selected?.name === viewName} - on:click={() => $goto(`./view/${viewName}`)} + selected={$isActive("./view") && $views.selected?.name === viewName} + on:click={() => $goto(`./view/${encodeURIComponent(viewName)}`)} > { store, routify, beforeNavigate, + decode, } = options || {} if ( !urlParam || @@ -29,11 +30,23 @@ export const syncURLToState = options => { return } + // Decodes encoded URL params if required + const decodeParams = urlParams => { + if (!decode) { + return urlParams + } + let decoded = {} + Object.keys(urlParams || {}).forEach(key => { + decoded[key] = decode(urlParams[key]) + }) + return decoded + } + // We can't dynamically fetch the value of stateful routify stores so we need // to just subscribe and cache the latest versions. // We can grab their initial values as this is during component // initialisation. - let cachedParams = get(routify.params) + let cachedParams = decodeParams(get(routify.params)) let cachedGoto = get(routify.goto) let cachedRedirect = get(routify.redirect) let cachedPage = get(routify.page) @@ -77,7 +90,7 @@ export const syncURLToState = options => { // Check if new value is valid if (validate && fallbackUrl) { if (!validate(urlValue)) { - log("Invalid URL param!") + log("Invalid URL param!", urlValue) redirectUrl(fallbackUrl) return } @@ -109,7 +122,7 @@ export const syncURLToState = options => { log(`url.${urlParam} (${urlValue}) <= state.${stateKey} (${stateValue})`) if (validate && fallbackUrl) { if (!validate(stateValue)) { - log("Invalid state param!") + log("Invalid state param!", stateValue) redirectUrl(fallbackUrl) return } @@ -137,6 +150,7 @@ export const syncURLToState = options => { // Subscribe to URL changes and cache them const unsubscribeParams = routify.params.subscribe($urlParams => { + $urlParams = decodeParams($urlParams) cachedParams = $urlParams mapUrlToState($urlParams) }) diff --git a/packages/builder/src/pages/builder/app/[application]/data/view/[viewName]/_layout.svelte b/packages/builder/src/pages/builder/app/[application]/data/view/[viewName]/_layout.svelte index 6b5c8d0fc8..2e38256d88 100644 --- a/packages/builder/src/pages/builder/app/[application]/data/view/[viewName]/_layout.svelte +++ b/packages/builder/src/pages/builder/app/[application]/data/view/[viewName]/_layout.svelte @@ -12,6 +12,7 @@ fallbackUrl: "../", store: views, routify, + decode: decodeURIComponent, }) onDestroy(stopSyncing) diff --git a/packages/builder/src/pages/builder/app/[application]/data/view/index.svelte b/packages/builder/src/pages/builder/app/[application]/data/view/index.svelte index 71ba1a81ac..7c0483fe1a 100644 --- a/packages/builder/src/pages/builder/app/[application]/data/view/index.svelte +++ b/packages/builder/src/pages/builder/app/[application]/data/view/index.svelte @@ -6,9 +6,9 @@ onMount(async () => { const { list, selected } = $views if (selected) { - $redirect(`./${selected?.name}`) + $redirect(`./${encodeURIComponent(selected?.name)}`) } else if (list?.length) { - $redirect(`./${list[0].name}`) + $redirect(`./${encodeURIComponent(list[0].name)}`) } else { $redirect("../") } diff --git a/packages/frontend-core/src/api/views.js b/packages/frontend-core/src/api/views.js index 237a66bc13..1d710c5171 100644 --- a/packages/frontend-core/src/api/views.js +++ b/packages/frontend-core/src/api/views.js @@ -16,8 +16,8 @@ export const buildViewEndpoints = API => ({ params.set("group", groupBy) } const QUERY_VIEW_URL = field - ? `/api/views/${name}?${params}` - : `/api/views/${name}` + ? `/api/views/${encodeURIComponent(name)}?${params}` + : `/api/views/${encodeURIComponent(name)}` return await API.get({ url: QUERY_VIEW_URL }) }, @@ -53,7 +53,7 @@ export const buildViewEndpoints = API => ({ */ deleteView: async viewName => { return await API.delete({ - url: `/api/views/${viewName}`, + url: `/api/views/${encodeURIComponent(viewName)}`, }) }, }) diff --git a/packages/server/src/api/controllers/row/internal.ts b/packages/server/src/api/controllers/row/internal.ts index 2b2b1b8b36..9bfdff24ea 100644 --- a/packages/server/src/api/controllers/row/internal.ts +++ b/packages/server/src/api/controllers/row/internal.ts @@ -187,7 +187,7 @@ export async function save(ctx: UserCtx) { } export async function fetchView(ctx: Ctx) { - const viewName = ctx.params.viewName + const viewName = decodeURIComponent(ctx.params.viewName) // if this is a table view being looked for just transfer to that if (viewName.startsWith(DocumentType.TABLE)) { diff --git a/packages/server/src/api/controllers/view/index.ts b/packages/server/src/api/controllers/view/index.ts index 17c3ee301d..932823d172 100644 --- a/packages/server/src/api/controllers/view/index.ts +++ b/packages/server/src/api/controllers/view/index.ts @@ -113,7 +113,7 @@ async function handleViewEvents(existingView: View, newView: View) { export async function destroy(ctx: BBContext) { const db = context.getAppDB() - const viewName = decodeURI(ctx.params.viewName) + const viewName = decodeURIComponent(ctx.params.viewName) const view = await deleteView(viewName) const table = await db.get(view.meta.tableId) delete table.views[viewName] @@ -124,7 +124,7 @@ export async function destroy(ctx: BBContext) { } export async function exportView(ctx: BBContext) { - const viewName = decodeURI(ctx.query.view as string) + const viewName = decodeURIComponent(ctx.query.view as string) const view = await getView(viewName) const format = ctx.query.format as string