Desktop: Importing from OneNote: Enable stricter path sanitization on Windows (#14321)

packages/app-mobile/components/screens/Note/Note.test.tsx

@@ -364,4 +364,17 @@ describe('screens/Note', () => {
 
 		unmount();
 	});
+
+	it('should set the initial editor cursor location to the specified hash', async () => {
+		await openNewNote({ title: 'To be edited', body: 'a test\n\n# Test\n\n# Test 2\n\n# Test 3' });
+		store.dispatch({ type: 'NAV_GO', noteHash: 'test-2' });
+		const { unmount } = render(<WrappedNoteScreen />);
+
+		await openEditor();
+		const editor = await getMarkdownEditorControl();
+
+		expect(editor.getCursor().line).toBe(4);
+
+		unmount();
+	});
 });

packages/app-mobile/components/screens/Note/Note.tsx

@@ -236,12 +236,16 @@ class NoteScreenComponent extends BaseScreenComponent<ComponentProps, State> imp
 			multiline: false,
 		};
 
-		const initialCursorLocation = NotePositionService.instance().getCursorPosition(props.noteId, defaultWindowId).markdown;
-		if (initialCursorLocation) {
-			this.selection = { start: initialCursorLocation, end: initialCursorLocation };
-		}
 		const initialScroll = NotePositionService.instance().getScrollPercent(props.noteId, defaultWindowId);
-		this.lastBodyScroll = initialScroll;
+		const initialCursorLocation = NotePositionService.instance().getCursorPosition(props.noteId, defaultWindowId).markdown;
+		// Ignore the initial scroll and cursor location when there's a note hash. The editor/viewer should jump to
+		// the hash, rather than the last position.
+		if (!props.noteHash) {
+			if (initialCursorLocation) {
+				this.selection = { start: initialCursorLocation, end: initialCursorLocation };
+			}
+			this.lastBodyScroll = initialScroll;
+		}
 
 		this.titleTextFieldRef = React.createRef();
 

packages/lib/package.json

@@ -94,7 +94,7 @@
     "sqlite3": "5.1.6",
     "string-padding": "1.0.2",
     "string-to-stream": "3.0.1",
-    "tar": "6.2.1",
+    "tar": "7.5.7",
     "tcp-port-used": "1.0.2",
     "uglifycss": "0.0.29",
     "url-parse": "1.5.10",

packages/lib/services/interop/InteropService_Importer_OneNote.ts

@@ -2,7 +2,7 @@ import { ImportExportResult, ImportModuleOutputFormat, ImportOptions } from './t
 
 import InteropService_Importer_Base from './InteropService_Importer_Base';
 import { NoteEntity } from '../database/types';
-import { rtrimSlashes } from '../../path-utils';
+import { friendlySafeFilename, rtrimSlashes } from '../../path-utils';
 import InteropService_Importer_Md from './InteropService_Importer_Md';
 import { join, resolve, normalize, sep, dirname, extname, basename, relative } from 'path';
 import Logger from '@joplin/utils/Logger';
@@ -337,15 +337,15 @@ export default class InteropService_Importer_OneNote extends InteropService_Impo
 			const originalPath = join(basePath, fileName);
 			let newPath;
 
-			let fixedFileName = Buffer.from(fileName, 'latin1').toString('utf8');
-			if (fixedFileName !== fileName) {
-				// In general, the path shouldn't start with "."s or contain path separators.
-				// However, if it does, these characters might cause import errors, so remove them:
-				fixedFileName = fixedFileName.replace(/^\.+/, '');
-				fixedFileName = fixedFileName.replace(/[/\\]/g, ' ');
-
-				// Avoid path traversal: Ensure that the file path is contained within the base directory
-				const newFullPathSafe = shim.fsDriver().resolveRelativePathWithinDir(basePath, fixedFileName);
+			const fixedFileName = Buffer.from(fileName, 'latin1').toString('utf8');
+			// If the filename includes the Unicode replacement character, file name correction has failed.
+			// Use the original (incorrect) filename in that case:
+			const replacementCharacter = '\uFFFD';
+			if (fixedFileName !== fileName && !fixedFileName.includes(replacementCharacter)) {
+				const newFullPathSafe = shim.fsDriver().resolveRelativePathWithinDir(
+					basePath,
+					friendlySafeFilename(fixedFileName, 128, true),
+				);
 				await shim.fsDriver().move(originalPath, newFullPathSafe);
 
 				newPath = newFullPathSafe;

packages/onenote-converter/Cargo.lock

@@ -392,6 +392,7 @@ dependencies = [
  "lazy_static",
  "parser-macros",
  "paste",
+ "sanitize-filename",
  "thiserror",
  "uuid",
  "wasm-bindgen",
@@ -569,7 +570,6 @@ dependencies = [
  "paste",
  "percent-encoding",
  "regex",
- "sanitize-filename",
  "thiserror",
  "uuid",
  "wasm-bindgen",

packages/onenote-converter/parser-utils/Cargo.toml

@@ -11,6 +11,7 @@ widestring = "1.0.2"
 uuid = "1.1.2"
 lazy_static = "1.4"
 wasm-bindgen = "0.2"
+sanitize-filename = "0.3.0"
 parser-macros = { path = "../parser-macros" }
 
 [dependencies.web-sys]

packages/onenote-converter/parser-utils/node_functions.js

@@ -28,10 +28,15 @@ function normalizeAndWriteFile(filePath, data) {
 	fs.writeFileSync(filePath, data);
 }
 
+function isWindows() {
+	return process.platform === 'win32';
+}
+
 module.exports = {
 	mkdirSyncRecursive,
 	isDirectory,
 	readDir,
 	removePrefix,
 	normalizeAndWriteFile,
+	isWindows,
 };

packages/onenote-converter/parser-utils/src/file_api/api.rs

@@ -1,6 +1,9 @@
+use sanitize_filename::{ sanitize_with_options, Options as SanitizeOptions };
+
 pub type ApiResult<T> = std::result::Result<T, std::io::Error>;
 
 pub trait FileApiDriver: Send + Sync {
+    fn is_windows(&self) -> bool;
     fn is_directory(&self, path: &str) -> ApiResult<bool>;
     fn read_dir(&self, path: &str) -> ApiResult<Vec<String>>;
     fn read_file(&self, path: &str) -> ApiResult<Vec<u8>>;
@@ -19,6 +22,21 @@ pub trait FileApiDriver: Send + Sync {
     /// `path_2` is still appended to `path_1`.
     fn join(&self, path_1: &str, path_2: &str) -> String;
 
+    fn sanitize_file_name(&self, file_name: &str) -> String {
+        sanitize_with_options(
+            file_name.trim(),
+            SanitizeOptions {
+                // Override "windows". By default, sanitize_filename can
+                // incorrectly detect the host OS when compiled to WASM.
+                windows: self.is_windows(),
+
+                // Otherwise, match the default sanitize_filename options:
+                truncate: true,
+                replacement: "",
+            },
+        )
+    }
+
     /// Splits filename into (base, extension).
     fn split_file_name(&self, filename: &str) -> (String, String) {
         let ext = self.get_file_extension(filename);

packages/onenote-converter/parser-utils/src/file_api/mod.rs

@@ -21,3 +21,32 @@ lazy_static! {
 pub fn fs_driver() -> Arc<dyn FileApiDriver> {
     FS_DRIVER.clone()
 }
+
+#[cfg(test)]
+mod test {
+    use super::fs_driver;
+
+    #[test]
+    fn sanitize_simple() {
+        assert_eq!(
+            fs_driver().sanitize_file_name("a.txt"),
+            "a.txt"
+        );
+        assert_eq!(
+            fs_driver().sanitize_file_name("a/b.txt"),
+            "a_b.txt"
+        );
+        assert_eq!(
+            fs_driver().sanitize_file_name("a\0a/b.txt"),
+            "a_a_b.txt"
+        );
+        assert_eq!(
+            fs_driver().sanitize_file_name("a\\b\\.txt "),
+            "a_b_.txt"
+        );
+        assert_eq!(
+            fs_driver().sanitize_file_name("/"),
+            "_"
+        );
+    }
+}

packages/onenote-converter/parser-utils/src/file_api/native_driver.rs

@@ -7,6 +7,16 @@ use std::path::Path;
 pub struct FileApiDriverImpl {}
 
 impl FileApiDriver for FileApiDriverImpl {
+    #[cfg(target_os = "windows")]
+    fn is_windows(&self) -> bool {
+        true
+    }
+
+    #[cfg(not(target_os = "windows"))]
+    fn is_windows(&self) -> bool {
+        false
+    }
+
     fn is_directory(&self, path: &str) -> ApiResult<bool> {
         let metadata = fs::metadata(path)?;
         let file_type = metadata.file_type();

packages/onenote-converter/parser-utils/src/file_api/wasm_driver.rs

@@ -31,6 +31,9 @@ extern "C" {
 
     #[wasm_bindgen(js_name = readDir, catch)]
     fn read_dir_js(path: &str) -> std::result::Result<JsValue, JsValue>;
+
+    #[wasm_bindgen(js_name = isWindows)]
+    fn is_windows() -> bool;
 }
 
 #[wasm_bindgen(module = "fs")]
@@ -73,6 +76,10 @@ fn handle_error(error: JsValue, source: &str) -> std::io::Error {
 pub struct FileApiDriverImpl {}
 
 impl FileApiDriver for FileApiDriverImpl {
+    fn is_windows(&self) -> bool {
+        is_windows()
+    }
+
     fn is_directory(&self, path: &str) -> ApiResult<bool> {
         match is_directory(path) {
             Ok(is_dir) => Ok(is_dir),

packages/onenote-converter/renderer/Cargo.toml

@@ -17,7 +17,6 @@ mime_guess = "2.0.3"
 once_cell = "1.4.1"
 palette = "0.5.0"
 regex = "1"
-sanitize-filename = "0.3.0"
 console_error_panic_hook = "0.1.7"
 bytes = "1.2.0"
 encoding_rs = "0.8.31"

packages/onenote-converter/renderer/src/notebook.rs

@@ -38,7 +38,7 @@ impl Renderer {
                     )?));
                 }
                 SectionEntry::SectionGroup(group) => {
-                    let dir_name = sanitize_filename::sanitize(group.display_name());
+                    let dir_name = fs_driver().sanitize_file_name(group.display_name());
                     let section_group_dir =
                         fs_driver().join(notebook_dir.as_str(), dir_name.as_str());
 

packages/onenote-converter/renderer/src/section.rs

@@ -28,7 +28,7 @@ impl Renderer {
     pub fn render(&mut self, section: &Section, output_dir: String) -> Result<RenderedSection> {
         let section_dir = fs_driver().join(
             output_dir.as_str(),
-            sanitize_filename::sanitize(section.display_name()).as_str(),
+            fs_driver().sanitize_file_name(section.display_name()).as_str(),
         );
         log!(
             "section_dir: {:?} \n output_dir: {:?}",
@@ -168,7 +168,7 @@ impl Renderer {
         let filename = filename_base.trim().replace("/", "_");
         let mut i = 0;
         let mut current_filename =
-            sanitize_filename::sanitize(format!("{}{}", filename, extension));
+            fs_driver().sanitize_file_name(&format!("{}{}", filename, extension));
 
         loop {
             let current_full_path = fs_driver().join(parent_dir, &current_filename);
@@ -179,7 +179,7 @@ impl Renderer {
 
             i += 1;
             current_filename =
-                sanitize_filename::sanitize(format!("{}_{}{}", filename, i, extension));
+                fs_driver().sanitize_file_name(&format!("{}_{}{}", filename, i, extension));
         }
 
         Ok(current_filename)

yarn.lock

@@ -10860,7 +10860,7 @@ __metadata:
     sqlite3: "npm:5.1.6"
     string-padding: "npm:1.0.2"
     string-to-stream: "npm:3.0.1"
-    tar: "npm:6.2.1"
+    tar: "npm:7.5.7"
     tcp-port-used: "npm:1.0.2"
     tesseract.js: "npm:6.0.1"
     typescript: "npm:5.8.3"
@@ -38732,6 +38732,15 @@ __metadata:
   languageName: node
   linkType: hard
 
+"minizlib@npm:^3.1.0":
+  version: 3.1.0
+  resolution: "minizlib@npm:3.1.0"
+  dependencies:
+    minipass: "npm:^7.1.2"
+  checksum: 10/f47365cc2cb7f078cbe7e046eb52655e2e7e97f8c0a9a674f4da60d94fb0624edfcec9b5db32e8ba5a99a5f036f595680ae6fe02a262beaa73026e505cc52f99
+  languageName: node
+  linkType: hard
+
 "mississippi@npm:^3.0.0":
   version: 3.0.0
   resolution: "mississippi@npm:3.0.0"
@@ -49589,17 +49598,16 @@ __metadata:
   languageName: node
   linkType: hard
 
-"tar@npm:6.2.1, tar@npm:^6.2.1":
-  version: 6.2.1
-  resolution: "tar@npm:6.2.1"
+"tar@npm:7.5.7":
+  version: 7.5.7
+  resolution: "tar@npm:7.5.7"
   dependencies:
-    chownr: "npm:^2.0.0"
-    fs-minipass: "npm:^2.0.0"
-    minipass: "npm:^5.0.0"
-    minizlib: "npm:^2.1.1"
-    mkdirp: "npm:^1.0.3"
-    yallist: "npm:^4.0.0"
-  checksum: 10/bfbfbb2861888077fc1130b84029cdc2721efb93d1d1fb80f22a7ac3a98ec6f8972f29e564103bbebf5e97be67ebc356d37fa48dbc4960600a1eb7230fbd1ea0
+    "@isaacs/fs-minipass": "npm:^4.0.0"
+    chownr: "npm:^3.0.0"
+    minipass: "npm:^7.1.2"
+    minizlib: "npm:^3.1.0"
+    yallist: "npm:^5.0.0"
+  checksum: 10/0d6938dd32fe5c0f17c8098d92bd9889ee0ed9d11f12381b8146b6e8c87bb5aa49feec7abc42463f0597503d8e89e4c4c0b42bff1a5a38444e918b4878b7fd21
   languageName: node
   linkType: hard
 
@@ -49646,6 +49654,20 @@ __metadata:
   languageName: node
   linkType: hard
 
+"tar@npm:^6.2.1":
+  version: 6.2.1
+  resolution: "tar@npm:6.2.1"
+  dependencies:
+    chownr: "npm:^2.0.0"
+    fs-minipass: "npm:^2.0.0"
+    minipass: "npm:^5.0.0"
+    minizlib: "npm:^2.1.1"
+    mkdirp: "npm:^1.0.3"
+    yallist: "npm:^4.0.0"
+  checksum: 10/bfbfbb2861888077fc1130b84029cdc2721efb93d1d1fb80f22a7ac3a98ec6f8972f29e564103bbebf5e97be67ebc356d37fa48dbc4960600a1eb7230fbd1ea0
+  languageName: node
+  linkType: hard
+
 "tar@npm:^7.4.3":
   version: 7.4.3
   resolution: "tar@npm:7.4.3"