From 8874769b317e6e364c881039c0c9308826f49cff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Klein?= Date: Tue, 25 Feb 2020 19:12:11 +0100 Subject: [PATCH] feat: sanitize media filenames according to global slug setting (#3315) --- .../actions/__tests__/mediaLibrary.spec.js | 116 +++++++++++++----- .../src/actions/mediaLibrary.ts | 15 ++- website/content/docs/configuration-options.md | 4 +- 3 files changed, 101 insertions(+), 34 deletions(-) diff --git a/packages/netlify-cms-core/src/actions/__tests__/mediaLibrary.spec.js b/packages/netlify-cms-core/src/actions/__tests__/mediaLibrary.spec.js index fcffdd56..701efb09 100644 --- a/packages/netlify-cms-core/src/actions/__tests__/mediaLibrary.spec.js +++ b/packages/netlify-cms-core/src/actions/__tests__/mediaLibrary.spec.js @@ -4,9 +4,7 @@ import { List, Map } from 'immutable'; import { insertMedia, persistMedia, deleteMedia } from '../mediaLibrary'; jest.mock('coreSrc/backend'); -jest.mock('ValueObjects/AssetProxy'); jest.mock('../waitUntil'); -jest.mock('../../lib/urlHelper'); jest.mock('netlify-cms-lib-util', () => { const lib = jest.requireActual('netlify-cms-lib-util'); return { @@ -62,7 +60,6 @@ describe('mediaLibrary', () => { }); const { currentBackend } = require('coreSrc/backend'); - const { createAssetProxy } = require('ValueObjects/AssetProxy'); const backend = { persistMedia: jest.fn(() => ({ id: 'id' })), @@ -83,12 +80,14 @@ describe('mediaLibrary', () => { getBlobSHA.mockReturnValue('000000000000000'); - const { sanitizeSlug } = require('../../lib/urlHelper'); - sanitizeSlug.mockReturnValue('name.png'); - const store = mockStore({ config: Map({ media_folder: 'static/media', + slug: Map({ + encoding: 'unicode', + clean_accents: false, + sanitize_replacement: '-', + }), }), collections: Map({ posts: Map({ name: 'posts' }), @@ -103,27 +102,27 @@ describe('mediaLibrary', () => { }); const file = new File([''], 'name.png'); - const assetProxy = { path: 'static/media/name.png' }; - createAssetProxy.mockReturnValue(assetProxy); return store.dispatch(persistMedia(file)).then(() => { const actions = store.getActions(); expect(actions).toHaveLength(2); - expect(actions[0]).toEqual({ - type: 'ADD_ASSET', - payload: { path: 'static/media/name.png' }, - }); - expect(actions[1]).toEqual({ - type: 'ADD_DRAFT_ENTRY_MEDIA_FILE', - payload: { + expect(actions[0].type).toEqual('ADD_ASSET'); + expect(actions[0].payload).toEqual( + expect.objectContaining({ + path: 'static/media/name.png', + }), + ); + expect(actions[1].type).toEqual('ADD_DRAFT_ENTRY_MEDIA_FILE'); + expect(actions[1].payload).toEqual( + expect.objectContaining({ draft: true, id: '000000000000000', path: 'static/media/name.png', size: file.size, name: file.name, - }, - }); + }), + ); expect(getBlobSHA).toHaveBeenCalledTimes(1); expect(getBlobSHA).toHaveBeenCalledWith(file); @@ -135,6 +134,11 @@ describe('mediaLibrary', () => { const store = mockStore({ config: Map({ media_folder: 'static/media', + slug: Map({ + encoding: 'unicode', + clean_accents: false, + sanitize_replacement: '-', + }), }), collections: Map({ posts: Map({ name: 'posts' }), @@ -149,8 +153,6 @@ describe('mediaLibrary', () => { }); const file = new File([''], 'name.png'); - const assetProxy = { path: 'static/media/name.png' }; - createAssetProxy.mockReturnValue(assetProxy); return store.dispatch(persistMedia(file)).then(() => { const actions = store.getActions(); @@ -159,10 +161,12 @@ describe('mediaLibrary', () => { expect(actions).toHaveLength(3); expect(actions[0]).toEqual({ type: 'MEDIA_PERSIST_REQUEST' }); - expect(actions[1]).toEqual({ - type: 'ADD_ASSET', - payload: { path: 'static/media/name.png' }, - }); + expect(actions[1].type).toEqual('ADD_ASSET'); + expect(actions[1].payload).toEqual( + expect.objectContaining({ + path: 'static/media/name.png', + }), + ); expect(actions[2]).toEqual({ type: 'MEDIA_PERSIST_SUCCESS', payload: { @@ -171,7 +175,67 @@ describe('mediaLibrary', () => { }); expect(backend.persistMedia).toHaveBeenCalledTimes(1); - expect(backend.persistMedia).toHaveBeenCalledWith(store.getState().config, assetProxy); + expect(backend.persistMedia).toHaveBeenCalledWith( + store.getState().config, + expect.objectContaining({ + path: 'static/media/name.png', + }), + ); + }); + }); + + it('should sanitize media name if needed when persisting', () => { + const store = mockStore({ + config: Map({ + media_folder: 'static/media', + slug: Map({ + encoding: 'ascii', + clean_accents: true, + sanitize_replacement: '_', + }), + }), + collections: Map({ + posts: Map({ name: 'posts' }), + }), + integrations: Map(), + mediaLibrary: Map({ + files: List(), + }), + entryDraft: Map({ + entry: Map(), + }), + }); + + const file = new File([''], 'abc DEF éâçÖ $;, .png'); + + return store.dispatch(persistMedia(file)).then(() => { + const actions = store.getActions(); + + expect(actions).toHaveLength(3); + + expect(actions[0]).toEqual({ type: 'MEDIA_PERSIST_REQUEST' }); + + expect(actions[1].type).toEqual('ADD_ASSET'); + expect(actions[1].payload).toEqual( + expect.objectContaining({ + path: 'static/media/abc_def_eaco_.png', + }), + ); + + expect(actions[2]).toEqual({ + type: 'MEDIA_PERSIST_SUCCESS', + payload: { + file: { id: 'id' }, + }, + }); + + expect(backend.persistMedia).toHaveBeenCalledTimes(1); + expect(backend.persistMedia).toHaveBeenCalledWith( + store.getState().config, + expect.objectContaining({ + path: 'static/media/abc_def_eaco_.png', + }), + ); }); }); }); @@ -197,8 +261,6 @@ describe('mediaLibrary', () => { }); const file = { name: 'name.png', id: 'id', path: 'static/media/name.png', draft: false }; - const assetProxy = { path: 'static/media/name.png' }; - createAssetProxy.mockReturnValue(assetProxy); return store.dispatch(deleteMedia(file)).then(() => { const actions = store.getActions(); @@ -242,8 +304,6 @@ describe('mediaLibrary', () => { }); const file = { name: 'name.png', id: 'id', path: 'static/media/name.png', draft: true }; - const assetProxy = { path: 'static/media/name.png' }; - createAssetProxy.mockReturnValue(assetProxy); return store.dispatch(deleteMedia(file)).then(() => { const actions = store.getActions(); diff --git a/packages/netlify-cms-core/src/actions/mediaLibrary.ts b/packages/netlify-cms-core/src/actions/mediaLibrary.ts index 9bf2943e..553ce6e5 100644 --- a/packages/netlify-cms-core/src/actions/mediaLibrary.ts +++ b/packages/netlify-cms-core/src/actions/mediaLibrary.ts @@ -1,6 +1,6 @@ import { Map } from 'immutable'; import { actions as notifActions } from 'redux-notifications'; -import { getBlobSHA, ImplementationMediaFile } from 'netlify-cms-lib-util'; +import { basename, getBlobSHA, ImplementationMediaFile } from 'netlify-cms-lib-util'; import { currentBackend } from '../backend'; import AssetProxy, { createAssetProxy } from '../valueObjects/AssetProxy'; import { selectIntegration } from '../reducers'; @@ -195,7 +195,7 @@ function createMediaFileFromAsset({ }): ImplementationMediaFile { const mediaFile = { id, - name: file.name, + name: basename(assetProxy.path), displayURL: assetProxy.url, draft, size: file.size, @@ -253,7 +253,7 @@ export function persistMedia(file: File, opts: MediaOptions = {}) { } catch (error) { assetProxy = createAssetProxy({ file, - path: file.name, + path: fileName, }); } } else if (privateUpload) { @@ -261,7 +261,7 @@ export function persistMedia(file: File, opts: MediaOptions = {}) { } else { const entry = state.entryDraft.get('entry'); const collection = state.collections.get(entry?.get('collection')); - const path = selectMediaFilePath(state.config, collection, entry, file.name, field); + const path = selectMediaFilePath(state.config, collection, entry, fileName, field); assetProxy = createAssetProxy({ file, path, @@ -278,7 +278,12 @@ export function persistMedia(file: File, opts: MediaOptions = {}) { mediaFile = createMediaFileFromAsset({ id, file, assetProxy, draft: false }); } else if (editingDraft) { const id = await getBlobSHA(file); - mediaFile = createMediaFileFromAsset({ id, file, assetProxy, draft: editingDraft }); + mediaFile = createMediaFileFromAsset({ + id, + file, + assetProxy, + draft: editingDraft, + }); return dispatch(addDraftEntryMediaFile(mediaFile)); } else { mediaFile = await backend.persistMedia(state.config, assetProxy); diff --git a/website/content/docs/configuration-options.md b/website/content/docs/configuration-options.md index 415aa4b0..63298561 100644 --- a/website/content/docs/configuration-options.md +++ b/website/content/docs/configuration-options.md @@ -156,7 +156,9 @@ show_preview_links: false ## Slug Type -The `slug` option allows you to change how filenames for entries are created and sanitized. For modifying the actual data in a slug, see the per-collection option below. +The `slug` option allows you to change how filenames for entries are created and sanitized. +It also applies to filenames of media inserted via the default media library. +For modifying the actual data in a slug, see the per-collection option below. `slug` accepts multiple options: