feat: sanitize media filenames according to global slug setting (#3315)

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({
+        expect(actions[0].type).toEqual('ADD_ASSET');
-          type: 'ADD_ASSET',
+        expect(actions[0].payload).toEqual(
-          payload: { path: 'static/media/name.png' },
+          expect.objectContaining({
-        });
+            path: 'static/media/name.png',
-        expect(actions[1]).toEqual({
+          }),
-          type: 'ADD_DRAFT_ENTRY_MEDIA_FILE',
+        );
-          payload: {
+        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({
+        expect(actions[1].type).toEqual('ADD_ASSET');
-          type: 'ADD_ASSET',
+        expect(actions[1].payload).toEqual(
-          payload: { path: 'static/media/name.png' },
+          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();

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);

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: