Desktop: Fixes #8706: Pasting a resource in Rich Text editor breaks the resource link

packages/app-desktop/gui/MainScreen/commands/openItem.ts

@@ -3,7 +3,8 @@ import shim from '@joplin/lib/shim';
 import { _ } from '@joplin/lib/locale';
 import bridge from '../../../services/bridge';
 import { openItemById } from '../../NoteEditor/utils/contextMenu';
-const { parseResourceUrl, urlProtocol, fileUriToPath } = require('@joplin/lib/urlUtils');
+const { parseResourceUrl, urlProtocol } = require('@joplin/lib/urlUtils');
+import { fileUriToPath } from '@joplin/utils/url';
 const { urlDecode } = require('@joplin/lib/string-utils');
 
 export const declaration: CommandDeclaration = {

packages/app-desktop/gui/NoteEditor/utils/resourceHandling.test.ts

@@ -1,10 +1,15 @@
+import Setting from '@joplin/lib/models/Setting';
 import { processPastedHtml } from './resourceHandling';
 
 describe('resourceHandling', () => {
 	it('should sanitize pasted HTML', async () => {
+		Setting.setConstant('resourceDir', '/home/.config/joplin/resources');
+
 		const testCases = [
 			['Test: <style onload="evil()"></style>', 'Test: <style></style>'],
 			['<a href="javascript: alert()">test</a>', '<a href="#">test</a>'],
+			['<a href="file:///home/.config/joplin/resources/test.pdf">test</a>', '<a href="file:///home/.config/joplin/resources/test.pdf">test</a>'],
+			['<a href="file:///etc/passwd">evil.pdf</a>', '<a href="#">evil.pdf</a>'],
 			['<script >evil()</script>', ''],
 			['<script>evil()</script>', ''],
 			[

packages/app-desktop/gui/NoteEditor/utils/resourceHandling.ts

@@ -8,7 +8,7 @@ import ResourceFetcher from '@joplin/lib/services/ResourceFetcher';
 import htmlUtils from '@joplin/lib/htmlUtils';
 import rendererHtmlUtils from '@joplin/renderer/htmlUtils';
 import Logger from '@joplin/utils/Logger';
-const { fileUriToPath } = require('@joplin/lib/urlUtils');
+import { fileUriToPath } from '@joplin/utils/url';
 const joplinRendererUtils = require('@joplin/renderer').utils;
 const { clipboard } = require('electron');
 const mimeUtils = require('@joplin/lib/mime-utils.js').mime;
@@ -179,6 +179,8 @@ export async function processPastedHtml(html: string) {
 	return rendererHtmlUtils.sanitizeHtml(
 		htmlUtils.replaceImageUrls(html, (src: string) => {
 			return mappedResources[src];
-		})
+		}), {
+			allowedFilePrefixes: [Setting.value('resourceDir')],
+		}
 	);
 }

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

@@ -24,9 +24,9 @@ import * as ArrayUtils from '../../../ArrayUtils';
 import Logger from '@joplin/utils/Logger';
 const { mimeTypeFromHeaders } = require('../../../net-utils');
 const { fileExtension, safeFileExtension, safeFilename, filename } = require('../../../path-utils');
-const { fileUriToPath } = require('../../../urlUtils');
 const { MarkupToHtml } = require('@joplin/renderer');
 const { ErrorNotFound } = require('../utils/errors');
+import { fileUriToPath } from '@joplin/utils/url';
 
 const logger = Logger.create('routes/notes');
 

packages/lib/urlUtils.js

@@ -105,101 +105,4 @@ urlUtils.objectToQueryString = function(query) {
 	return queryString;
 };
 
-// This is a modified version of the file-uri-to-path package:
-//
-// - It removes the dependency to the "path" package, which wouldn't work with
-//   React Native.
-//
-// - It always returns paths with forward slashes "/". This is normally handled
-//   properly everywhere.
-//
-// - Adds the "platform" parameter to optionall return paths with "\" for win32
-function fileUriToPath_(uri, platform) {
-	const sep = '/';
-
-	if (
-		typeof uri !== 'string' ||
-		uri.length <= 7 ||
-		uri.substring(0, 7) !== 'file://'
-	) {
-		throw new TypeError(
-			'must pass in a file:// URI to convert to a file path'
-		);
-	}
-
-	const rest = decodeURI(uri.substring(7));
-	const firstSlash = rest.indexOf('/');
-	let host = rest.substring(0, firstSlash);
-	let path = rest.substring(firstSlash + 1);
-
-	// 2.  Scheme Definition
-	// As a special case, <host> can be the string "localhost" or the empty
-	// string; this is interpreted as "the machine from which the URL is
-	// being interpreted".
-	if (host === 'localhost') {
-		host = '';
-	}
-
-	if (host) {
-		host = sep + sep + host;
-	}
-
-	// 3.2  Drives, drive letters, mount points, file system root
-	// Drive letters are mapped into the top of a file URI in various ways,
-	// depending on the implementation; some applications substitute
-	// vertical bar ("|") for the colon after the drive letter, yielding
-	// "file:///c|/tmp/test.txt".  In some cases, the colon is left
-	// unchanged, as in "file:///c:/tmp/test.txt".  In other cases, the
-	// colon is simply omitted, as in "file:///c/tmp/test.txt".
-	path = path.replace(/^(.+)\|/, '$1:');
-
-	// for Windows, we need to invert the path separators from what a URI uses
-	// if (sep === '\\') {
-	// 	path = path.replace(/\//g, '\\');
-	// }
-
-	if (/^.+:/.test(path)) {
-		// has Windows drive at beginning of path
-	} else {
-		// unix path…
-		path = sep + path;
-	}
-
-	if (platform === 'win32') {
-		return (host + path).replace(/\//g, '\\');
-	} else {
-		return host + path;
-	}
-}
-
-urlUtils.fileUriToPath = (path, platform = 'linux') => {
-	const output = fileUriToPath_(path, platform);
-
-	// The file-uri-to-path module converts Windows path such as
-	//
-	// file://c:/autoexec.bat => \\c:\autoexec.bat
-	//
-	// Probably because a file:// that starts with only two slashes is not
-	// quite valid. If we use three slashes, it works:
-	//
-	// file:///c:/autoexec.bat => c:\autoexec.bat
-	//
-	// However there are various places in the app where we can find
-	// paths with only two slashes because paths are often constructed
-	// as `file://${resourcePath}` - which works in all OSes except
-	// Windows.
-	//
-	// So here we introduce a special case - if we detect that we have
-	// an invalid Windows path that starts with \\x:, we just remove
-	// the first two backslashes.
-	//
-	// https://github.com/laurent22/joplin/issues/5693
-
-	if (output.match(/^\/\/[a-zA-Z]:/)) {
-		return output.substr(2);
-	}
-
-	return output;
-};
-
 module.exports = urlUtils;

packages/lib/urlUtils.test.js

@@ -71,30 +71,4 @@ describe('urlUtils', () => {
 		}
 	}));
 
-	it('should convert a file URI to a file path', (async () => {
-		// Tests imported from https://github.com/TooTallNate/file-uri-to-path/tree/master/test
-		const testCases = {
-			'file://host/path': '//host/path',
-			'file://localhost/etc/fstab': '/etc/fstab',
-			'file:///etc/fstab': '/etc/fstab',
-			'file:///c:/WINDOWS/clock.avi': 'c:/WINDOWS/clock.avi',
-			'file://localhost/c|/WINDOWS/clock.avi': 'c:/WINDOWS/clock.avi',
-			'file:///c|/WINDOWS/clock.avi': 'c:/WINDOWS/clock.avi',
-			'file://localhost/c:/WINDOWS/clock.avi': 'c:/WINDOWS/clock.avi',
-			'file://hostname/path/to/the%20file.txt': '//hostname/path/to/the file.txt',
-			'file:///c:/path/to/the%20file.txt': 'c:/path/to/the file.txt',
-			'file:///C:/Documents%20and%20Settings/davris/FileSchemeURIs.doc': 'C:/Documents and Settings/davris/FileSchemeURIs.doc',
-			'file:///C:/caf%C3%A9/%C3%A5r/d%C3%BCnn/%E7%89%9B%E9%93%83/Ph%E1%BB%9F/%F0%9F%98%B5.exe': 'C:/café/år/dünn/牛铃/Phở/😵.exe',
-		};
-
-		for (const [input, expected] of Object.entries(testCases)) {
-			const actual = urlUtils.fileUriToPath(input);
-			expect(actual).toBe(expected);
-		}
-
-		expect(urlUtils.fileUriToPath('file://c:/not/quite/right')).toBe('c:/not/quite/right');
-		expect(urlUtils.fileUriToPath('file:///d:/better')).toBe('d:/better');
-		expect(urlUtils.fileUriToPath('file:///c:/AUTOEXEC.BAT', 'win32')).toBe('c:\\AUTOEXEC.BAT');
-	}));
-
 });

packages/renderer/htmlUtils.ts

@@ -1,5 +1,5 @@
-const Entities = require('html-entities').AllHtmlEntities;
+import { htmlentities } from '@joplin/utils/html';
-const htmlentities = new Entities().encode;
+import { fileUriToPath } from '@joplin/utils/url';
 const htmlparser2 = require('@joplin/fork-htmlparser2');
 
 // [\s\S] instead of . for multiline matching
@@ -31,7 +31,8 @@ const selfClosingElements = [
 ];
 
 interface SanitizeHtmlOptions {
-	addNoMdConvClass: boolean;
+	addNoMdConvClass?: boolean;
+	allowedFilePrefixes?: string[];
 }
 
 class HtmlUtils {
@@ -157,20 +158,36 @@ class HtmlUtils {
 			.replace(/</g, '&lt;');
 	}
 
-	private isAcceptedUrl(url: string): boolean {
+	private isAcceptedUrl(url: string, allowedFilePrefixes: string[]): boolean {
 		url = url.toLowerCase();
-		return url.startsWith('https://') ||
+		if (url.startsWith('https://') ||
 			url.startsWith('http://') ||
 			url.startsWith('mailto://') ||
 			// We also allow anchors but only with a specific set of a characters.
 			// Fixes https://github.com/laurent22/joplin/issues/8286
-			!!url.match(/^#[a-zA-Z0-9-]+$/);
+			!!url.match(/^#[a-zA-Z0-9-]+$/)) return true;
+
+		if (url.startsWith('file://')) {
+			// We need to do a case insensitive comparison because the URL we
+			// get appears to be converted to lowercase somewhere. To be
+			// completely sure, we make it lowercase explicitely.
+			const filePath = fileUriToPath(url).toLowerCase();
+			for (const filePrefix of allowedFilePrefixes) {
+				if (filePath.startsWith(filePrefix.toLowerCase())) return true;
+			}
+		}
+
+		return false;
 	}
 
 	public sanitizeHtml(html: string, options: SanitizeHtmlOptions = null) {
-		options = { // If true, adds a "jop-noMdConv" class to all the tags.
+		options = {
+			// If true, adds a "jop-noMdConv" class to all the tags.
 			// It can be used afterwards to restore HTML tags in Markdown.
-			addNoMdConvClass: false, ...options };
+			addNoMdConvClass: false,
+			allowedFilePrefixes: [],
+			...options,
+		};
 
 		const output: string[] = [];
 
@@ -247,7 +264,7 @@ class HtmlUtils {
 				// particular we want to exclude `javascript:` URLs. This
 				// applies to A tags, and also AREA ones but to be safe we don't
 				// filter on the tag name and process all HREF attributes.
-				if ('href' in attrs && !this.isAcceptedUrl(attrs['href'])) {
+				if ('href' in attrs && !this.isAcceptedUrl(attrs['href'], options.allowedFilePrefixes)) {
 					attrs['href'] = '#';
 				}
 

packages/renderer/package.json

@@ -28,6 +28,7 @@
   "dependencies": {
     "@joplin/fork-htmlparser2": "^4.1.46",
     "@joplin/fork-uslug": "^1.0.11",
+    "@joplin/utils": "~2.12",
     "font-awesome-filetypes": "2.1.0",
     "fs-extra": "11.1.1",
     "highlight.js": "11.8.0",

packages/utils/package.json

@@ -8,6 +8,7 @@
     "./net": "./dist/net.js",
     "./fs": "./dist/fs.js",
     "./env": "./dist/env.js",
+    "./url": "./dist/url.js",
     "./Logger": "./dist/Logger.js"
   },
   "publishConfig": {

yarn.lock

@@ -4842,6 +4842,7 @@ __metadata:
   dependencies:
     "@joplin/fork-htmlparser2": ^4.1.46
     "@joplin/fork-uslug": ^1.0.11
+    "@joplin/utils": ~2.12
     "@types/jest": 29.5.3
     "@types/node": 18.16.18
     font-awesome-filetypes: 2.1.0