Mobile: Fixes #9069: Fix writing UTF-8 data to a file replaces non-ASCII characters with ?s (#9076)

.eslintignore

@@ -499,7 +499,8 @@ packages/app-mobile/utils/autodetectTheme.js
 packages/app-mobile/utils/checkPermissions.js
 packages/app-mobile/utils/createRootStyle.js
 packages/app-mobile/utils/debounce.js
-packages/app-mobile/utils/fs-driver-rn.js
+packages/app-mobile/utils/fs-driver/fs-driver-rn.js
+packages/app-mobile/utils/fs-driver/runOnDeviceTests.js
 packages/app-mobile/utils/setupNotifications.js
 packages/app-mobile/utils/shareHandler.js
 packages/app-mobile/utils/types.js

.gitignore

@@ -481,7 +481,8 @@ packages/app-mobile/utils/autodetectTheme.js
 packages/app-mobile/utils/checkPermissions.js
 packages/app-mobile/utils/createRootStyle.js
 packages/app-mobile/utils/debounce.js
-packages/app-mobile/utils/fs-driver-rn.js
+packages/app-mobile/utils/fs-driver/fs-driver-rn.js
+packages/app-mobile/utils/fs-driver/runOnDeviceTests.js
 packages/app-mobile/utils/setupNotifications.js
 packages/app-mobile/utils/shareHandler.js
 packages/app-mobile/utils/types.js

packages/app-mobile/root.tsx

@@ -97,7 +97,7 @@ SyncTargetRegistry.addClass(SyncTargetAmazonS3);
 SyncTargetRegistry.addClass(SyncTargetJoplinServer);
 SyncTargetRegistry.addClass(SyncTargetJoplinCloud);
 
-import FsDriverRN from './utils/fs-driver-rn';
+import FsDriverRN from './utils/fs-driver/fs-driver-rn';
 import DecryptionWorker from '@joplin/lib/services/DecryptionWorker';
 import EncryptionService from '@joplin/lib/services/e2ee/EncryptionService';
 import MigrationService from '@joplin/lib/services/MigrationService';
@@ -109,7 +109,7 @@ import { loadMasterKeysFromSettings, migrateMasterPassword } from '@joplin/lib/s
 import SyncTargetNone from '@joplin/lib/SyncTargetNone';
 import { setRSA } from '@joplin/lib/services/e2ee/ppk';
 import RSA from './services/e2ee/RSA.react-native';
-import { runIntegrationTests } from '@joplin/lib/services/e2ee/ppkTestUtils';
+import { runIntegrationTests as runRsaIntegrationTests } from '@joplin/lib/services/e2ee/ppkTestUtils';
 import { Theme, ThemeAppearance } from '@joplin/lib/themes/type';
 import { AppState } from './utils/types';
 import ProfileSwitcher from './components/ProfileSwitcher/ProfileSwitcher';
@@ -121,6 +121,7 @@ import userFetcher, { initializeUserFetcher } from '@joplin/lib/utils/userFetche
 import { ReactNode } from 'react';
 import { parseShareCache } from '@joplin/lib/services/share/reducer';
 import autodetectTheme, { onSystemColorSchemeChange } from './utils/autodetectTheme';
+import runOnDeviceFsDriverTests from './utils/fs-driver/runOnDeviceTests';
 
 type SideMenuPosition = 'left' | 'right';
 
@@ -749,7 +750,10 @@ async function initialize(dispatch: Function) {
 	// call will throw an error, alerting us of the issue. Otherwise it will
 	// just print some messages in the console.
 	// ----------------------------------------------------------------------------
-	if (Setting.value('env') === 'dev') await runIntegrationTests();
+	if (Setting.value('env') === 'dev') {
+		await runRsaIntegrationTests();
+		await runOnDeviceFsDriverTests();
+	}
 
 	reg.logger().info('Application initialized');
 }

packages/app-mobile/utils/fs-driver/fs-driver-rn.ts

@@ -3,7 +3,7 @@ const RNFetchBlob = require('rn-fetch-blob').default;
 import * as RNFS from 'react-native-fs';
 const DocumentPicker = require('react-native-document-picker').default;
 import { openDocument } from '@joplin/react-native-saf-x';
-import RNSAF, { Encoding, DocumentFileDetail, openDocumentTree } from '@joplin/react-native-saf-x';
+import RNSAF, { DocumentFileDetail, openDocumentTree } from '@joplin/react-native-saf-x';
 import { Platform } from 'react-native';
 import * as tar from 'tar-stream';
 import { resolve } from 'path';
@@ -18,24 +18,63 @@ function isScopedUri(path: string) {
 	return path.includes(ANDROID_URI_PREFIX);
 }
 
+// Encodings supported by rn-fetch-blob, RNSAF, and
+// RNFS.
+// See also
+// - https://github.com/itinance/react-native-fs#readfilefilepath-string-encoding-string-promisestring
+// - https://github.com/joltup/rn-fetch-blob/blob/cf9e8843599de92031df2660d5a1da18491fa3c0/android/src/main/java/com/RNFetchBlob/RNFetchBlobFS.java#L1049
+export enum SupportedEncoding {
+	Utf8 = 'utf8',
+	Ascii = 'ascii',
+	Base64 = 'base64',
+}
+const supportedEncodings = Object.values<string>(SupportedEncoding);
+
+// Converts some encodings specifiers that work with NodeJS into encodings
+// that work with RNSAF, RNFetchBlob.fs, and RNFS.
+//
+// Throws if an encoding can't be normalized.
+const normalizeEncoding = (encoding: string): SupportedEncoding => {
+	encoding = encoding.toLowerCase();
+
+	// rn-fetch-blob and RNSAF require the exact string "utf8", but NodeJS (and thus
+	// fs-driver-node) support variants on this like "UtF-8" and "utf-8". Convert them:
+	if (encoding === 'utf-8') {
+		encoding = 'utf8';
+	}
+
+	if (!supportedEncodings.includes(encoding)) {
+		throw new Error(`Unsupported encoding: ${encoding}.`);
+	}
+
+	return encoding as SupportedEncoding;
+};
+
 export default class FsDriverRN extends FsDriverBase {
 	public appendFileSync() {
 		throw new Error('Not implemented');
 	}
 
-	// Encoding can be either "utf8" or "base64"
-	public appendFile(path: string, content: any, encoding = 'base64') {
+	// Requires that the file already exists.
+	// TODO: Update for compatibility with fs-driver-node's appendFile (which does not
+	//       require that the file exists).
+	public appendFile(path: string, content: any, rawEncoding = 'base64') {
+		const encoding = normalizeEncoding(rawEncoding);
+
 		if (isScopedUri(path)) {
-			return RNSAF.writeFile(path, content, { encoding: encoding as Encoding, append: true });
+			return RNSAF.writeFile(path, content, { encoding, append: true });
 		}
 		return RNFS.appendFile(path, content, encoding);
 	}
 
-	// Encoding can be either "utf8" or "base64"
-	public writeFile(path: string, content: any, encoding = 'base64') {
+	// Encoding can be either "utf8", "utf-8", or "base64"
+	public writeFile(path: string, content: any, rawEncoding = 'base64') {
+		const encoding = normalizeEncoding(rawEncoding);
+
 		if (isScopedUri(path)) {
-			return RNSAF.writeFile(path, content, { encoding: encoding as Encoding });
+			return RNSAF.writeFile(path, content, { encoding: encoding });
 		}
+
 		// We need to use rn-fetch-blob here due to this bug:
 		// https://github.com/itinance/react-native-fs/issues/700
 		return RNFetchBlob.fs.writeFile(path, content, encoding);
@@ -195,10 +234,11 @@ export default class FsDriverRN extends FsDriverBase {
 		return null;
 	}
 
-	public readFile(path: string, encoding = 'utf8') {
-		if (encoding === 'Buffer') throw new Error('Raw buffer output not supported for FsDriverRN.readFile');
+	public readFile(path: string, rawEncoding = 'utf8') {
+		const encoding = normalizeEncoding(rawEncoding);
+
 		if (isScopedUri(path)) {
-			return RNSAF.readFile(path, { encoding: encoding as Encoding });
+			return RNSAF.readFile(path, { encoding: encoding });
 		}
 		return RNFS.readFile(path, encoding);
 	}
@@ -244,7 +284,9 @@ export default class FsDriverRN extends FsDriverBase {
 		}
 	}
 
-	public async readFileChunk(handle: any, length: number, encoding = 'base64') {
+	public async readFileChunk(handle: any, length: number, rawEncoding = 'base64') {
+		const encoding = normalizeEncoding(rawEncoding);
+
 		if (handle.offset + length > handle.stat.size) {
 			length = handle.stat.size - handle.offset;
 		}

packages/app-mobile/utils/fs-driver/runOnDeviceTests.ts

@@ -0,0 +1,249 @@
+import Setting from '@joplin/lib/models/Setting';
+import shim from '@joplin/lib/shim';
+import uuid from '@joplin/lib/uuid';
+import { join } from 'path';
+import FsDriverBase from '@joplin/lib/fs-driver-base';
+import Logger from '@joplin/utils/Logger';
+import { Buffer } from 'buffer';
+
+const logger = Logger.create('fs-driver-tests');
+
+const expectToBe = async <T> (actual: T, expected: T) => {
+	if (actual !== expected) {
+		throw new Error(`Integration test failure: ${actual} was expected to be ${expected}`);
+	}
+};
+
+const testExpect = async () => {
+	// Verify that expect is working
+	await expectToBe(1, 1);
+	await expectToBe(true, true);
+
+	let failed = false;
+	try {
+		await expectToBe('a', 'test');
+		failed = true;
+	} catch (_error) {
+		failed = false;
+	}
+
+	if (failed) {
+		throw new Error('expectToBe should throw when given non-equal inputs');
+	}
+};
+
+const testAppendFile = async (tempDir: string) => {
+	logger.info('Testing fsDriver.appendFile...');
+
+	const targetFile = join(tempDir, uuid.createNano());
+
+	const fsDriver: FsDriverBase = shim.fsDriver();
+
+	// For fs-driver-rn's appendFile to work, we first need to create the file.
+	// TODO: This is different from the requirements of fs-driver-node.
+	await fsDriver.writeFile(targetFile, '');
+
+	const firstChunk = 'A 𝓊𝓃𝒾𝒸𝓸𝒹𝓮 test\n...';
+	await fsDriver.appendFile(targetFile, firstChunk, 'utf-8');
+	await expectToBe(await fsDriver.readFile(targetFile), firstChunk);
+
+	const secondChunk = '▪️  More unicode ▪️';
+	await fsDriver.appendFile(targetFile, secondChunk, 'utf8');
+	await expectToBe(await fsDriver.readFile(targetFile), firstChunk + secondChunk);
+
+	const thirdChunk = 'ASCII';
+	await fsDriver.appendFile(targetFile, thirdChunk, 'ascii');
+	await expectToBe(await fsDriver.readFile(targetFile), firstChunk + secondChunk + thirdChunk);
+
+	const lastChunk = 'Test...';
+	await fsDriver.appendFile(
+		targetFile, Buffer.from(lastChunk, 'utf8').toString('base64'), 'base64',
+	);
+	await expectToBe(
+		await fsDriver.readFile(targetFile), firstChunk + secondChunk + thirdChunk + lastChunk,
+	);
+
+	// Should throw if given an invalid encoding
+	let didThrow = false;
+	try {
+		await fsDriver.appendFile(targetFile, 'test', 'bad-encoding');
+	} catch (_error) {
+		didThrow = true;
+	}
+	await expectToBe(didThrow, true);
+};
+
+const testReadWriteFileUtf8 = async (tempDir: string) => {
+	logger.info('Testing fsDriver.writeFile and fsDriver.readFile with utf-8...');
+
+	const filePath = join(tempDir, uuid.createNano());
+
+	const testStrings = [
+		// ASCII
+		'test',
+
+		// Special characters
+		'𝐴 𝒕𝐞𝑺𝒕',
+
+		// Emojis
+		'✅ Test. 🕳️',
+	];
+
+	const testEncodings = ['utf-8', 'utf8', 'UtF-8'];
+
+	// Use the same file for all tests to test overwriting
+	for (const encoding of testEncodings) {
+		for (const testString of testStrings) {
+			const fsDriver: FsDriverBase = shim.fsDriver();
+			await fsDriver.writeFile(filePath, testString, encoding);
+
+			const fileData = await fsDriver.readFile(filePath, encoding);
+			await expectToBe(fileData, testString);
+		}
+	}
+};
+
+const testReadFileChunkUtf8 = async (tempDir: string) => {
+	logger.info('Testing fsDriver.readFileChunk...');
+
+	const filePath = join(tempDir, `${uuid.createNano()}.txt`);
+
+	const fsDriver: FsDriverBase = shim.fsDriver();
+
+	// 🕳️ is 7 bytes when utf-8 encoded
+	// à,á,â, and ã are each 2 bytes
+	const expectedFileContent = '01234567\nàáâã\n🕳️🕳️🕳️\ntēst...';
+	await fsDriver.writeFile(filePath, expectedFileContent, 'utf8');
+
+	const testEncodings = ['utf-8', 'utf8', 'UtF-8'];
+
+	for (const encoding of testEncodings) {
+		const handle = await fsDriver.open(filePath, 'r');
+
+		await expectToBe(
+			await fsDriver.readFileChunk(handle, 8, encoding), '01234567',
+		);
+
+		await expectToBe(
+			await fsDriver.readFileChunk(handle, 1, encoding), '\n',
+		);
+
+		await expectToBe(
+			await fsDriver.readFileChunk(handle, 8, encoding), 'àáâã',
+		);
+
+		await expectToBe(
+			await fsDriver.readFileChunk(handle, 8, encoding), '\n🕳️',
+		);
+
+		await expectToBe(
+			await fsDriver.readFileChunk(handle, 15, encoding), '🕳️🕳️\n',
+		);
+
+		// A 0 length should return null and not advance
+		await expectToBe(
+			await fsDriver.readFileChunk(handle, 0, encoding), null,
+		);
+
+		// Reading a different encoding (then switching back to the original)
+		// should be supported
+		await expectToBe(
+			await fsDriver.readFileChunk(handle, 3, 'base64'),
+			Buffer.from('tē', 'utf-8').toString('base64'),
+		);
+
+		await expectToBe(
+			await fsDriver.readFileChunk(handle, 100, encoding), 'st...',
+		);
+
+		// Should not be able to read past the end
+		await expectToBe(
+			await fsDriver.readFileChunk(handle, 10, encoding), null,
+		);
+
+		await expectToBe(
+			await fsDriver.readFileChunk(handle, 1, encoding), null,
+		);
+
+		await fsDriver.close(filePath);
+	}
+};
+
+const testTarCreate = async (tempDir: string) => {
+	logger.info('Testing fsDriver.tarCreate...');
+
+	const directoryToPack = join(tempDir, uuid.createNano());
+
+	const fsDriver: FsDriverBase = shim.fsDriver();
+
+	// Add test files to the directory
+	const fileContents: Record<string, string> = {};
+
+	// small utf-8 encoded files
+	for (let i = 0; i < 10; i ++) {
+		const testFilePath = join(directoryToPack, uuid.createNano());
+
+		const fileContent = `✅ Testing... ä ✅ File #${i}`;
+		await fsDriver.writeFile(testFilePath, fileContent, 'utf-8');
+
+		fileContents[testFilePath] = fileContent;
+	}
+
+	// larger utf-8 encoded files
+	for (let i = 0; i < 3; i ++) {
+		const testFilePath = join(directoryToPack, uuid.createNano());
+
+		let fileContent = `✅ Testing... ä ✅ File #${i}`;
+
+		for (let j = 0; j < 8; j ++) {
+			fileContent += fileContent;
+		}
+
+		await fsDriver.writeFile(testFilePath, fileContent, 'utf-8');
+
+		fileContents[testFilePath] = fileContent;
+	}
+
+	// Pack the files
+	const pathsToTar = Object.keys(fileContents);
+	const tarOutputPath = join(tempDir, 'test-tar.tar');
+	await fsDriver.tarCreate({
+		cwd: tempDir,
+		file: tarOutputPath,
+	}, pathsToTar);
+
+	// Read the tar file as utf-8 and search for the written file contents
+	// (which should work).
+	const rawTarData: string = await fsDriver.readFile(tarOutputPath, 'utf8');
+
+	for (const fileContent of Object.values(fileContents)) {
+		await expectToBe(rawTarData.includes(fileContent), true);
+	}
+};
+
+// In the past, some fs-driver functionality has worked correctly on some devices and not others.
+// As such, we need to be able to run some tests on-device.
+const runOnDeviceTests = async () => {
+	const tempDir = join(Setting.value('tempDir'), uuid.createNano());
+
+	if (await shim.fsDriver().exists(tempDir)) {
+		await shim.fsDriver().remove(tempDir);
+	}
+
+	try {
+		await testExpect();
+		await testAppendFile(tempDir);
+		await testReadWriteFileUtf8(tempDir);
+		await testReadFileChunkUtf8(tempDir);
+		await testTarCreate(tempDir);
+	} catch (error) {
+		const errorMessage = `On-device testing failed with an exception: ${error}.`;
+
+		logger.error(errorMessage, error);
+		alert(errorMessage);
+	} finally {
+		await shim.fsDriver().remove(tempDir);
+	}
+};
+
+export default runOnDeviceTests;

packages/app-mobile/utils/shim-init-react.js

@@ -3,7 +3,7 @@ const { GeolocationReact } = require('./geolocation-react.js');
 const PoorManIntervals = require('@joplin/lib/PoorManIntervals').default;
 const RNFetchBlob = require('rn-fetch-blob').default;
 const { generateSecureRandom } = require('react-native-securerandom');
-const FsDriverRN = require('./fs-driver-rn').default;
+const FsDriverRN = require('./fs-driver/fs-driver-rn').default;
 const { Buffer } = require('buffer');
 const { Linking, Platform } = require('react-native');
 const mimeUtils = require('@joplin/lib/mime-utils.js').mime;

packages/lib/fs-driver-base.ts

@@ -25,6 +25,10 @@ export default class FsDriverBase {
 		throw new Error('Not implemented');
 	}
 
+	public async appendFile(_path: string, _content: string, _encoding = 'base64'): Promise<any> {
+		throw new Error('Not implemented');
+	}
+
 	public async copy(_source: string, _dest: string) {
 		throw new Error('Not implemented');
 	}