diff --git a/blocks/browse/da-browse/da-browse.js b/blocks/browse/da-browse/da-browse.js index 44dd05f4..486e10d7 100644 --- a/blocks/browse/da-browse/da-browse.js +++ b/blocks/browse/da-browse/da-browse.js @@ -1,7 +1,7 @@ import { LitElement, html, nothing } from 'da-lit'; import { DA_ORIGIN } from '../../shared/constants.js'; import { daFetch, getFirstSheet } from '../../shared/utils.js'; -import { getNx } from '../../../scripts/utils.js'; +import { getNx, sanitizePathParts } from '../../../scripts/utils.js'; // Components import '../da-breadcrumbs/da-breadcrumbs.js'; @@ -52,7 +52,7 @@ export default class DaBrowse extends LitElement { if ((e.metaKey || e.ctrlKey) && e.altKey && e.code === 'KeyT') { e.preventDefault(); const { fullpath } = this.details; - const [, ...split] = fullpath.split('/'); + const [...split] = sanitizePathParts(fullpath); if (split.length < 2) return; if (split[2] === '.trash') { diff --git a/blocks/browse/da-list/da-list.js b/blocks/browse/da-list/da-list.js index 410d77ad..d595ca45 100644 --- a/blocks/browse/da-list/da-list.js +++ b/blocks/browse/da-list/da-list.js @@ -1,6 +1,6 @@ import { LitElement, html, repeat, nothing } from 'da-lit'; import { DA_ORIGIN } from '../../shared/constants.js'; -import { getNx } from '../../../scripts/utils.js'; +import { getNx, sanitizePathParts } from '../../../scripts/utils.js'; import { daFetch, aemAdmin } from '../../shared/utils.js'; import '../da-list-item/da-list-item.js'; @@ -307,7 +307,7 @@ export default class DaList extends LitElement { this._itemsRemaining = this._selectedItems.length; const callback = async (item) => { - const [, org, site, ...rest] = item.path.split('/'); + const [org, site, ...rest] = sanitizePathParts(item.path); // If already in trash or not in a site, its a direct delete const directDelete = item.path.includes('/.trash/') || rest.length === 0; diff --git a/blocks/browse/da-list/helpers/utils.js b/blocks/browse/da-list/helpers/utils.js index eee2f9ad..6a8f59e1 100644 --- a/blocks/browse/da-list/helpers/utils.js +++ b/blocks/browse/da-list/helpers/utils.js @@ -1,4 +1,5 @@ import { SUPPORTED_FILES, DA_ORIGIN } from '../../../shared/constants.js'; +import { sanitizePath, sanitizePathParts } from '../../../../scripts/utils.js'; import { daFetch } from '../../../shared/utils.js'; const MAX_DEPTH = 1000; @@ -81,12 +82,6 @@ export async function getFullEntryList(entries) { return files.filter((file) => file); } -export function sanitizePath(path) { - const pathArray = path.split('/'); - const sanitizedArray = pathArray.map((element) => element.replaceAll(/[^a-zA-Z0-9.]/g, '-').toLowerCase()); - return [...sanitizedArray].join('/'); -} - export async function handleUpload(list, fullpath, file) { const { data, path } = file; const formData = new FormData(); @@ -122,8 +117,7 @@ export async function handleUpload(list, fullpath, file) { export function items2Clipboard(items) { const aemUrls = items.reduce((acc, item) => { if (item.ext) { - const path = item.path.replace('.html', ''); - const [org, repo, ...pathParts] = path.substring(1).split('/'); + const [org, repo, ...pathParts] = sanitizePathParts(item.path.replace('.html', '')); const pageName = pathParts.pop(); pathParts.push(pageName === 'index' ? '' : pageName); diff --git a/blocks/browse/da-sites/da-sites.js b/blocks/browse/da-sites/da-sites.js index 03868005..f4a33e99 100644 --- a/blocks/browse/da-sites/da-sites.js +++ b/blocks/browse/da-sites/da-sites.js @@ -1,5 +1,6 @@ import { LitElement, html, nothing } from 'da-lit'; import getSheet from '../../shared/sheet.js'; +import { sanitizeName } from '../../../scripts/utils.js'; const sheet = await getSheet('/blocks/browse/da-sites/da-sites.css'); @@ -94,7 +95,7 @@ export default class DaSites extends LitElement { // eslint-disable-next-line no-unused-vars const [_, repo, org] = helixString.split('--'); if (!repo || !org) return null; - return `#/${org}/${repo}`; + return `#/${sanitizeName(org, false)}/${sanitizeName(repo, false)}`; } catch (_) { return null; } diff --git a/blocks/edit/da-library/da-library.js b/blocks/edit/da-library/da-library.js index 7c4ef9c1..7da209e5 100644 --- a/blocks/edit/da-library/da-library.js +++ b/blocks/edit/da-library/da-library.js @@ -9,7 +9,7 @@ import { ref, nothing, } from 'da-lit'; -import { getNx } from '../../../scripts/utils.js'; +import { getNx, sanitizePathParts } from '../../../scripts/utils.js'; import { getBlocks, getBlockVariants } from './helpers/index.js'; import getSheet from '../../shared/sheet.js'; import inlinesvg from '../../shared/inlinesvg.js'; @@ -291,7 +291,7 @@ class DaLibrary extends LitElement { getParts() { const view = 'edit'; - const [org, repo, ...path] = window.location.hash.replace('#/', '').split('/'); + const [org, repo, ...path] = sanitizePathParts(window.location.hash.substring(1)); return { view, org, repo, ref: 'main', path: `/${path.join('/')}` }; } diff --git a/blocks/edit/da-library/helpers/helpers.js b/blocks/edit/da-library/helpers/helpers.js index c8ff484a..0f98d176 100644 --- a/blocks/edit/da-library/helpers/helpers.js +++ b/blocks/edit/da-library/helpers/helpers.js @@ -5,7 +5,7 @@ import getPathDetails from '../../../shared/pathDetails.js'; import { daFetch, getFirstSheet } from '../../../shared/utils.js'; import { getConfKey, openAssets } from '../../da-assets/da-assets.js'; import { fetchKeyAutocompleteData } from '../../prose/plugins/slashMenu/keyAutocomplete.js'; -import { sanitiseRef } from '../../../../scripts/utils.js'; +import { sanitizeName } from '../../../../scripts/utils.js'; const DA_ORIGIN = getDaAdmin(); const REPLACE_CONTENT = ''; @@ -18,7 +18,7 @@ const DA_PLUGINS = [ 'placeholders', ]; -const ref = sanitiseRef(new URLSearchParams(window.location.search).get('ref')) || 'main'; +const ref = sanitizeName(new URLSearchParams(window.location.search).get('ref'), false) || 'main'; export function parseDom(dom) { const { schema } = window.view.state; diff --git a/blocks/edit/utils/helpers.js b/blocks/edit/utils/helpers.js index a42b568c..0bd2a8e7 100644 --- a/blocks/edit/utils/helpers.js +++ b/blocks/edit/utils/helpers.js @@ -1,4 +1,5 @@ import { AEM_ORIGIN, DA_ORIGIN } from '../../shared/constants.js'; +import { sanitizePathParts } from '../../../../scripts/utils.js'; import prose2aem from '../../shared/prose2aem.js'; import { daFetch } from '../../shared/utils.js'; @@ -154,7 +155,7 @@ function parseAemError(xError) { } export async function getCdnConfig(path) { - const [org, site] = path.slice(1).toLowerCase().split('/'); + const [org, site] = sanitizePathParts(path); const resp = await daFetch(`${AEM_ORIGIN}/config/${org}/sites/${site}.json`); if (!resp.ok) { // eslint-disable-next-line no-console diff --git a/blocks/shared/pathDetails.js b/blocks/shared/pathDetails.js index 9f2708cf..a95193f3 100644 --- a/blocks/shared/pathDetails.js +++ b/blocks/shared/pathDetails.js @@ -1,4 +1,5 @@ import { CON_ORIGIN, DA_ORIGIN } from './constants.js'; +import { sanitizePathParts } from '../../scripts/utils.js'; let currpath; let currhash; @@ -119,7 +120,7 @@ export default function getPathDetails(loc) { if (!fullpath || fullpath.startsWith('old_hash') || fullpath.startsWith('access_token')) return null; // Split everything up so it can be later used for both DA & AEM - const pathParts = fullpath.slice(1).toLowerCase().split('/'); + const pathParts = sanitizePathParts(fullpath); // Redirect JSON files from edit view to sheet view if (editor === 'edit' && fullpath.endsWith('.json')) { diff --git a/blocks/start/index.js b/blocks/start/index.js index cabd3ee2..dc8edf78 100644 --- a/blocks/start/index.js +++ b/blocks/start/index.js @@ -1,4 +1,4 @@ -import { getNx } from '../../scripts/utils.js'; +import { getNx, sanitizePathParts } from '../../scripts/utils.js'; import { DA_ORIGIN } from '../shared/constants.js'; import { daFetch } from '../shared/utils.js'; @@ -32,7 +32,7 @@ async function getBlob(path) { async function bulkAemAdmin(org, site, files) { const paths = files.map((file) => { - const [, , ...parts] = file.path.slice(1).split('/'); + const [, , ...parts] = sanitizePathParts(file.path); return `/${parts.join('/')}`.replace('.html', ''); }); diff --git a/blocks/start/start.js b/blocks/start/start.js index bb2fc18e..810d1000 100644 --- a/blocks/start/start.js +++ b/blocks/start/start.js @@ -3,6 +3,7 @@ import { getDaAdmin } from '../shared/constants.js'; import getSheet from '../shared/sheet.js'; import { daFetch } from '../shared/utils.js'; import { copyConfig, copyContent, previewContent } from './index.js'; +import { sanitizePathParts } from '../../scripts/utils.js'; const sheet = await getSheet('/blocks/start/start.css'); @@ -124,7 +125,7 @@ class DaStart extends LitElement { try { const { origin, pathname } = new URL(e.target.value); if (origin !== 'https://github.com') throw Error('Not github'); - const [, org, site] = pathname.toLowerCase().trim().split('/'); + const [org, site] = sanitizePathParts(pathname); if (!(org && site)) throw Error('No org or site'); this.org = org; this.site = site; diff --git a/scripts/utils.js b/scripts/utils.js index 677b1704..1a529bed 100644 --- a/scripts/utils.js +++ b/scripts/utils.js @@ -11,21 +11,36 @@ */ export const codeBase = `${import.meta.url.replace('/scripts/utils.js', '')}`; -export function sanitiseRef(ref) { - if (!ref) return null; +export function sanitizeName(name, preserveDots = true) { + if (!name) return null; - return ref.toLowerCase() + if (preserveDots && name.indexOf('.') !== -1) { + return name + .split('.') + .map((part) => sanitizeName(part)) + .join('.'); + } + + return name + .toLowerCase() .normalize('NFD') .replace(/[\u0300-\u036f]/g, '') - .replace(/[^a-z0-9]+/g, '-') - .replace(/^-|-$/g, ''); + .replace(/[^a-z0-9_]+/g, '-') + .replace(/-$/g, ''); } -export function sanitizeName() {} - -export function sanitizePathParts() {} +export function sanitizePathParts(path) { + return path.slice(1) + .toLowerCase() + .split('/') + .map((name) => (name ? sanitizeName(name) : '')) + // remove path traversal parts, and empty strings unless at the end + .filter((name, i, parts) => !/^[.]{1,2}$/.test(name) && (name !== '' || i === parts.length - 1)); +} -export function sanitizePath() {} +export function sanitizePath(path) { + return `/${sanitizePathParts(path).join('/')}`; +} export const [setNx, getNx] = (() => { let nx; @@ -34,7 +49,7 @@ export const [setNx, getNx] = (() => { nx = (() => { const { hostname, search } = location || window.location; if (!(hostname.includes('.hlx.') || hostname.includes('.aem.') || hostname.includes('local'))) return nxBase; - const branch = sanitiseRef(new URLSearchParams(search).get('nx')) || 'main'; + const branch = sanitizeName(new URLSearchParams(search).get('nx'), false) || 'main'; if (branch === 'local') return 'http://localhost:6456/nx'; return `https://${branch}--da-nx--adobe.aem.live/nx`; })(); diff --git a/test/e2e/tests/authenticated/collab.spec.js b/test/e2e/tests/authenticated/collab.spec.js index bd9550df..c9f43814 100644 --- a/test/e2e/tests/authenticated/collab.spec.js +++ b/test/e2e/tests/authenticated/collab.spec.js @@ -27,6 +27,7 @@ test('Collab cursors in multiple editors', async ({ browser, page }, workerInfo) await expect(page.locator('div.ProseMirror')).toBeVisible(); await expect(page.locator('div.ProseMirror')).toHaveAttribute('contenteditable', 'true'); + await page.waitForTimeout(1000); await page.locator('div.ProseMirror').fill('Entered by user 1'); // Right now there should not be any collab indicators yet diff --git a/test/e2e/tests/edit.spec.js b/test/e2e/tests/edit.spec.js index 821580b1..3934850f 100644 --- a/test/e2e/tests/edit.spec.js +++ b/test/e2e/tests/edit.spec.js @@ -106,13 +106,14 @@ test('Change document by switching anchors', async ({ page }, workerInfo) => { await expect(page.locator('div.ProseMirror')).toBeVisible(); await page.waitForTimeout(3000); await expect(page.locator('div.ProseMirror')).toHaveAttribute('contenteditable', 'true'); - + await page.waitForTimeout(1000); await page.locator('div.ProseMirror').fill('page B'); await page.waitForTimeout(3000); await page.goto(urlA); await page.waitForTimeout(3000); await expect(page.locator('div.ProseMirror')).toBeVisible(); + await page.waitForTimeout(1000); await expect(page.locator('div.ProseMirror')).toContainText('mytable'); await page.waitForTimeout(2000); await expect(page.locator('div.ProseMirror')).toContainText('k 2'); @@ -121,6 +122,7 @@ test('Change document by switching anchors', async ({ page }, workerInfo) => { await page.goto(urlB); await page.waitForTimeout(3000); await expect(page.locator('div.ProseMirror')).toBeVisible(); + await page.waitForTimeout(1000); await expect(page.locator('div.ProseMirror')).toContainText('page B'); }); @@ -131,6 +133,7 @@ test('Add code mark', async ({ page }, workerInfo) => { await expect(page.locator('div.ProseMirror')).toBeVisible(); await page.waitForTimeout(3000); await expect(page.locator('div.ProseMirror')).toHaveAttribute('contenteditable', 'true'); + await page.waitForTimeout(1000); await page.locator('div.ProseMirror').fill('This is a line that will contain a code mark.'); // Forward diff --git a/test/unit/blocks/browse/helpers/helpers.test.js b/test/unit/blocks/browse/helpers/helpers.test.js index 316640f2..8d780276 100644 --- a/test/unit/blocks/browse/helpers/helpers.test.js +++ b/test/unit/blocks/browse/helpers/helpers.test.js @@ -1,6 +1,6 @@ import { expect } from '@esm-bundle/chai'; import { stub } from 'sinon'; -import { getFullEntryList, handleUpload, sanitizePath } from '../../../../../blocks/browse/da-list/helpers/utils.js'; +import { getFullEntryList, handleUpload } from '../../../../../blocks/browse/da-list/helpers/utils.js'; const goodEntry = { isDirectory: false, @@ -81,10 +81,4 @@ describe('Upload and format', () => { const item = await handleUpload(list, fullpath, packagedFile); expect(item).to.exist; }); - - it('Returns sanitize file path', async () => { - const path = '/new folder/geo_metrixx.jpg'; - const item = sanitizePath(path); - expect(item).to.equal('/new-folder/geo-metrixx.jpg'); - }); }); diff --git a/test/unit/scripts/utils.test.js b/test/unit/scripts/utils.test.js index 56b50bbd..8d490fd5 100644 --- a/test/unit/scripts/utils.test.js +++ b/test/unit/scripts/utils.test.js @@ -1,5 +1,5 @@ import { expect } from '@esm-bundle/chai'; -import { setNx } from '../../../scripts/utils.js'; +import { setNx, sanitizePath, sanitizePathParts, sanitizeName } from '../../../scripts/utils.js'; describe('Libs', () => { it('Default Libs', () => { @@ -33,4 +33,112 @@ describe('Libs', () => { const libs = setNx('/nx', location); expect(libs).to.equal('http://localhost:6456/nx'); }); + + describe('sanitizeName', () => { + it('Sanitizes name', async () => { + expect(sanitizeName(undefined)).to.equal(null); + expect(sanitizeName('')).to.equal(null); + expect(sanitizeName('Geö métrixX')).to.equal('geo-metrixx'); + expect(sanitizeName('Geö métrixX.jpg')).to.equal('geo-metrixx.jpg'); + expect(sanitizeName('.da')).to.equal('.da'); + }); + + it('Sanitizes name without preserving dots', async () => { + expect(sanitizeName('branch.name', false)).to.equal('branch-name'); + }); + }); + + describe('sanitizePath', () => { + it('Handles root path', () => { + expect(sanitizePath('/file.txt')).to.equal('/file.txt'); + }); + + it('Sanitizes path with spaces', () => { + expect(sanitizePath('/new folder/my file.txt')).to.equal('/new-folder/my-file.txt'); + }); + + it('Sanitizes path with special or uppercase characters', () => { + expect(sanitizePath('/café/résumé')).to.equal('/cafe/resume'); + expect(sanitizePath('/hello@world/file#1!')).to.equal('/hello-world/file-1'); + expect(sanitizePath('/my---folder/file!!!.json')).to.equal('/my-folder/file.json'); + expect(sanitizePath('/My Folder/File.JSON')).to.equal('/my-folder/file.json'); + }); + + it('Removes empty path parts except if at the end', () => { + expect(sanitizePath('/!!!/geometrixx')).to.equal('/geometrixx'); + expect(sanitizePath('//adobe/geometrixx/')).to.equal('/adobe/geometrixx/'); + }); + + it('Preserves dots in path parts', () => { + const path = '/.da/config.json'; + expect(sanitizePath(path)).to.equal('/.da/config.json'); + }); + + it('Protects against path traversal attacks', () => { + const path = './../../../etc/password'; + expect(sanitizePath(path)).to.equal('/etc/password'); + }); + }); + + describe('sanitizePathParts', () => { + it('Returns array of sanitized path parts', () => { + const path = '/New folder/Geo metrixx.jpg'; + const parts = sanitizePathParts(path); + expect(parts).to.deep.equal(['new-folder', 'geo-metrixx.jpg']); + }); + + it('Handles single file path', () => { + const path = '/file.html'; + const parts = sanitizePathParts(path); + expect(parts).to.deep.equal(['file.html']); + }); + + it('Handles paths with special characters in each part', () => { + const path = '/hello@world/test#folder/file!.doc'; + const parts = sanitizePathParts(path); + expect(parts).to.deep.equal(['hello-world', 'test-folder', 'file.doc']); + }); + + it('Handles path with no extension', () => { + const path = '/folder/README'; + const parts = sanitizePathParts(path); + expect(parts).to.deep.equal(['folder', 'readme']); + }); + + it('Sanitizes basename and extension separately', () => { + const path = '/folder/HELLO WORLD.TXT'; + const parts = sanitizePathParts(path); + expect(parts).to.deep.equal(['folder', 'hello-world.txt']); + }); + + it('Handles accented characters in path parts', () => { + const path = '/café/naïve/résumé.pdf'; + const parts = sanitizePathParts(path); + expect(parts).to.deep.equal(['cafe', 'naive', 'resume.pdf']); + }); + + it('Removes consecutive dashes', () => { + const path = '/my---folder/file!!!name.txt'; + const parts = sanitizePathParts(path); + expect(parts).to.deep.equal(['my-folder', 'file-name.txt']); + }); + + it('Removes trailing dashes from parts', () => { + const path = '/folder-/file--'; + const parts = sanitizePathParts(path); + expect(parts).to.deep.equal(['folder', 'file']); + }); + + it('Retains leading dashes in parts', () => { + const path = '/-folder/--file'; + const parts = sanitizePathParts(path); + expect(parts).to.deep.equal(['-folder', '-file']); + }); + + it('Retains underscores', () => { + const path = '/my_folder/file'; + const parts = sanitizePathParts(path); + expect(parts).to.deep.equal(['my_folder', 'file']); + }); + }); });