diff --git a/.eslintignore b/.eslintignore index 94485166b27..34c2be0f773 100644 --- a/.eslintignore +++ b/.eslintignore @@ -146,6 +146,7 @@ packages/app-desktop/InteropServiceHelper.js packages/app-desktop/app.reducer.test.js packages/app-desktop/app.reducer.js packages/app-desktop/app.js +packages/app-desktop/bridge.test.js packages/app-desktop/bridge.js packages/app-desktop/checkForUpdates.js packages/app-desktop/commands/copyDevCommand.js diff --git a/.gitignore b/.gitignore index 5db4a4268da..f09eb25634c 100644 --- a/.gitignore +++ b/.gitignore @@ -122,6 +122,7 @@ packages/app-desktop/InteropServiceHelper.js packages/app-desktop/app.reducer.test.js packages/app-desktop/app.reducer.js packages/app-desktop/app.js +packages/app-desktop/bridge.test.js packages/app-desktop/bridge.js packages/app-desktop/checkForUpdates.js packages/app-desktop/commands/copyDevCommand.js diff --git a/packages/app-desktop/bridge.test.ts b/packages/app-desktop/bridge.test.ts new file mode 100644 index 00000000000..2ebe9152717 --- /dev/null +++ b/packages/app-desktop/bridge.test.ts @@ -0,0 +1,53 @@ +import { createTempDir } from '@joplin/lib/testing/test-utils'; +import bridge, { initBridge } from './bridge'; +import { join } from 'path'; +import { mkdirp } from 'fs-extra'; +import { dialog, shell } from 'electron'; +import ElectronAppWrapper from './ElectronAppWrapper'; + +jest.mock('@sentry/electron/main', () => ({ + init: () => { }, +})); +jest.mock('electron', () => ({ + dialog: { + showMessageBox: jest.fn(() => Promise.resolve()), + }, + shell: { + openExternal: jest.fn(), + openPath: jest.fn(), + }, +})); +describe('bridge', () => { + + beforeAll(async () => { + await initBridge({ + activeWindow: () => {}, + } as ElectronAppWrapper, '', '', '', false); + }); + + it('should open directory if name has no extension', async () => { + const unsafeExtension = 'name'; + + const tempDir = await createTempDir(); + const fullPath = join(tempDir, unsafeExtension); + await mkdirp(fullPath); + + await bridge().openItem(fullPath); + expect(dialog.showMessageBox).toHaveBeenCalledTimes(0); + expect(shell.openPath).toHaveBeenCalledTimes(1); + expect(shell.openPath).toHaveBeenLastCalledWith(fullPath); + }); + + it('should show warning if opening directory has a unsafe extension', async () => { + const unsafeExtension = 'name.unsafe'; + dialog.showMessageBox = jest.fn().mockReturnValue({ response: 1 }); + + const tempDir = await createTempDir(); + const fullPath = join(tempDir, unsafeExtension); + await mkdirp(fullPath); + + await bridge().openItem(fullPath); + expect(dialog.showMessageBox).toHaveBeenCalledTimes(1); + expect(dialog.showMessageBox).toHaveBeenCalledWith(undefined, { 'buttons': ['Cancel', 'Learn more', 'Open it'], 'checkboxLabel': 'Always open ".unsafe" files without asking.', 'message': 'This file seems to be a directory, but Joplin doesn\'t recognise the ".unsafe" extension. Opening this could be dangerous. What would you like to do?', 'title': 'Unknown file type', 'type': 'warning' }); + }); +}); diff --git a/packages/app-desktop/bridge.ts b/packages/app-desktop/bridge.ts index f6f31e0361a..da9bd57e2c2 100644 --- a/packages/app-desktop/bridge.ts +++ b/packages/app-desktop/bridge.ts @@ -9,7 +9,7 @@ import * as Sentry from '@sentry/electron/main'; import { ErrorEvent } from '@sentry/types/types'; import { homedir } from 'os'; import { msleep } from '@joplin/utils/time'; -import { pathExists, pathExistsSync, writeFileSync } from 'fs-extra'; +import { pathExists, pathExistsSync, stat, writeFileSync } from 'fs-extra'; import { extname, normalize } from 'path'; import isSafeToOpen from './utils/isSafeToOpen'; import { closeSync, openSync, readSync, statSync } from 'fs'; @@ -431,16 +431,26 @@ export class Bridge { if (await pathExists(fullPath)) { const fileExtension = extname(fullPath); const userAllowedExtension = this.extraAllowedOpenExtensions.includes(fileExtension); - if (userAllowedExtension || await isSafeToOpen(fullPath)) { + + if (userAllowedExtension || (await isSafeToOpen(fullPath))) { return shell.openPath(fullPath); } else { const allowOpenId = 2; const learnMoreId = 1; const fileExtensionDescription = JSON.stringify(fileExtension); + + let warningMessage = _('Joplin doesn\'t recognise the %s extension. Opening this file could be dangerous. What would you like to do?', fileExtensionDescription); + + // We recheck if it is a directory because there is a possibility that the file could be dangerous and a directory + // but we would like to not confuse users about what link is being open + const isDirectory = (await stat(fullPath)).isDirectory(); + if (isDirectory) { + warningMessage = _('This file seems to be a directory, but Joplin doesn\'t recognise the %s extension. Opening this could be dangerous. What would you like to do?', fileExtensionDescription); + } + const result = await dialog.showMessageBox(this.activeWindow(), { title: _('Unknown file type'), - message: - _('Joplin doesn\'t recognise the %s extension. Opening this file could be dangerous. What would you like to do?', fileExtensionDescription), + message: warningMessage, type: 'warning', checkboxLabel: _('Always open %s files without asking.', fileExtensionDescription), buttons: [