Clipper: Avoid errors when downloading media files without protocol (#9241)

.eslintignore

@@ -826,6 +826,7 @@ packages/lib/services/rest/routes/events.test.js
 packages/lib/services/rest/routes/events.js
 packages/lib/services/rest/routes/folders.js
 packages/lib/services/rest/routes/master_keys.js
+packages/lib/services/rest/routes/notes.test.js
 packages/lib/services/rest/routes/notes.js
 packages/lib/services/rest/routes/ping.js
 packages/lib/services/rest/routes/resources.js

.gitignore

@@ -808,6 +808,7 @@ packages/lib/services/rest/routes/events.test.js
 packages/lib/services/rest/routes/events.js
 packages/lib/services/rest/routes/folders.js
 packages/lib/services/rest/routes/master_keys.js
+packages/lib/services/rest/routes/notes.test.js
 packages/lib/services/rest/routes/notes.js
 packages/lib/services/rest/routes/ping.js
 packages/lib/services/rest/routes/resources.js

packages/lib/services/rest/routes/notes.test.ts

@@ -0,0 +1,81 @@
+import Logger from '@joplin/utils/Logger';
+import shim from '../../../shim';
+import uuid from '../../../uuid';
+import { downloadMediaFile } from './notes';
+
+describe('routes/notes', () => {
+
+	beforeEach(() => {
+		jest.resetAllMocks();
+	});
+
+	test.each([
+		'/invalid/url',
+		'htp/asdfasf.com',
+		'https//joplinapp.org',
+	])('should not return a local file for invalid protocols', async (invalidUrl) => {
+		await expect(downloadMediaFile(invalidUrl)).resolves.toBe('');
+	});
+
+	test.each([
+		'https://joplinapp.org/valid/image_url.png',
+		'http://joplinapp.org/valid/image_url.png',
+	])('should try to download and return a local path to a valid URL', async (url) => {
+		const fetchBlobSpy = jest.fn();
+		jest.spyOn(shim, 'fetchBlob').mockImplementation(fetchBlobSpy);
+		jest.spyOn(uuid, 'create').mockReturnValue('mocked_uuid_value');
+
+		const response = await downloadMediaFile(url);
+
+		expect(response.endsWith('mocked_uuid_value.png')).toBe(true);
+		expect(fetchBlobSpy).toBeCalledTimes(1);
+	});
+
+	test('should get file from local drive if protocol allows it', async () => {
+		const url = 'file://valid/image.png';
+
+		const fsDriverCopySpy = jest.fn();
+		jest.spyOn(shim, 'fsDriver').mockImplementation(() => {
+			return {
+				copy: fsDriverCopySpy,
+			};
+		});
+		jest.spyOn(uuid, 'create').mockReturnValue('mocked_uuid_value');
+
+		const response = await downloadMediaFile(url);
+
+		expect(response.endsWith('mocked_uuid_value.png')).toBe(true);
+		expect(fsDriverCopySpy).toBeCalledTimes(1);
+	});
+
+	test('should be able to handle URLs with data', async () => {
+		const url = 'data:image/gif;base64,R0lGODlhEAAQAMQAAORHHOVSKudfOulrSOp3WOyDZu6QdvCchPGolfO0o/XBs/fNwfjZ0frl3/zy7////wAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAACH5BAkAABAALAAAAAAQABAAAAVVICSOZGlCQAosJ6mu7fiyZeKqNKToQGDsM8hBADgUXoGAiqhSvp5QAnQKGIgUhwFUYLCVDFCrKUE1lBavAViFIDlTImbKC5Gm2hB0SlBCBMQiB0UjIQA7';
+
+		const imageFromDataUrlSpy = jest.fn();
+		jest.spyOn(shim, 'imageFromDataUrl').mockImplementation(imageFromDataUrlSpy);
+		jest.spyOn(uuid, 'create').mockReturnValue('mocked_uuid_value');
+
+		const response = await downloadMediaFile(url);
+
+		expect(response.endsWith('mocked_uuid_value.gif')).toBe(true);
+		expect(imageFromDataUrlSpy).toBeCalledTimes(1);
+	});
+
+	test('should not process URLs with data that is not image type', async () => {
+		const url = 'data:application/octet-stream;base64,dGhpcyBpcyBhIG1lc3NhZ2UK';
+
+		Logger.globalLogger.enabled = false;
+		const response = await downloadMediaFile(url);
+		Logger.globalLogger.enabled = true;
+
+		expect(response).toBe('');
+	});
+
+	test('should not process URLs from cid: protocol', async () => {
+		const url = 'cid:ii_loq3d1100';
+
+		const response = await downloadMediaFile(url);
+
+		expect(response).toBe('');
+	});
+});

packages/lib/services/rest/routes/notes.ts

@@ -184,7 +184,7 @@ async function tryToGuessExtFromMimeType(response: any, mediaPath: string) {
 	return newMediaPath;
 }
 
-async function downloadMediaFile(url: string /* , allowFileProtocolImages */) {
+export async function downloadMediaFile(url: string /* , allowFileProtocolImages */) {
 	logger.info('Downloading media file', url);
 
 	const tempDir = Setting.value('tempDir');
@@ -200,6 +200,13 @@ async function downloadMediaFile(url: string /* , allowFileProtocolImages */) {
 		return '';
 	}
 
+	const invalidProtocols = ['cid:'];
+	const urlProtocol = urlUtils.urlProtocol(url)?.toLowerCase();
+
+	if (!urlProtocol || invalidProtocols.includes(urlProtocol)) {
+		return '';
+	}
+
 	const name = isDataUrl ? md5(`${Math.random()}_${Date.now()}`) : filename(url);
 	let fileExt = isDataUrl ? mimeUtils.toFileExtension(mimeUtils.fromDataUrl(url)) : safeFileExtension(fileExtension(url).toLowerCase());
 	if (!mimeUtils.fromFileExtension(fileExt)) fileExt = ''; // If the file extension is unknown - clear it.
@@ -212,7 +219,7 @@ async function downloadMediaFile(url: string /* , allowFileProtocolImages */) {
 	try {
 		if (isDataUrl) {
 			await shim.imageFromDataUrl(url, mediaPath);
-		} else if (urlUtils.urlProtocol(url).toLowerCase() === 'file:') {
+		} else if (urlProtocol === 'file:') {
 			// Can't think of any reason to disallow this at this point
 			// if (!allowFileProtocolImages) throw new Error('For security reasons, this URL with file:// protocol cannot be downloaded');
 			const localPath = fileUriToPath(url);