Validate the full extracted payload before skipping extraction

packages/lib/services/plugins/PluginService.ts

@@ -19,6 +19,11 @@ const uslug = require('@joplin/fork-uslug');
 
 const logger = Logger.create('PluginService');
 
+interface PluginExtractionState {
+	size: number;
+	timestamp: number;
+}
+
 // Plugin data is split into two:
 //
 // - First there's the service `plugins` property, which contains the
@@ -105,6 +110,7 @@ export default class PluginService extends BaseService {
 	private startedPlugins_: Record<string, boolean> = {};
 	private isSafeMode_ = false;
 	private pluginsChangeListeners_: LoadedPluginsChangeListener[] = [];
+	private extractionStates_: Record<string, PluginExtractionState> = null;
 
 	// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
 	public initialize(appVersion: string, platformImplementation: any, runner: BasePluginRunner, store: any) {
@@ -197,6 +203,31 @@ export default class PluginService extends BaseService {
 		await shim.fsDriver().remove(plugin.baseDir);
 	}
 
+	private extractionStatePath(): string {
+		return `${Setting.value('cacheDir')}/plugin-extraction-state.json`;
+	}
+
+	private async loadExtractionStates(): Promise<Record<string, PluginExtractionState>> {
+		if (!this.extractionStates_) {
+			try {
+				const text = await shim.fsDriver().readFile(this.extractionStatePath(), 'utf8');
+				this.extractionStates_ = JSON.parse(text);
+			} catch {
+				this.extractionStates_ = {};
+			}
+		}
+		return this.extractionStates_;
+	}
+
+	private async saveExtractionStates(states: Record<string, PluginExtractionState>): Promise<void> {
+		this.extractionStates_ = states;
+		try {
+			await shim.fsDriver().writeFile(this.extractionStatePath(), JSON.stringify(states), 'utf8');
+		} catch (error) {
+			logger.error('Failed to save extraction states:', error);
+		}
+	}
+
 	public pluginById(id: string): Plugin {
 		if (!this.plugins_[id]) throw new Error(`Plugin not found: ${id}`);
 
@@ -291,15 +322,24 @@ export default class PluginService extends BaseService {
 		baseDir = rtrimSlashes(baseDir);
 
 		const fname = filename(path);
-		const hash = await shim.fsDriver().md5File(path);
-
 		const unpackDir = `${Setting.value('cacheDir')}/${fname}`;
 		const manifestFilePath = `${unpackDir}/manifest.json`;
 
-		// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
-		let manifest: any = await this.loadManifestToObject(manifestFilePath);
+		// Use file size + mtime to check if the .jpl has changed, to
+		// avoid computing an MD5 hash of the full file on every startup.
+		const stat = await shim.fsDriver().stat(path);
+		const extractionStates = await this.loadExtractionStates();
+		const extractionState = extractionStates[fname];
+		const scriptFilePath = `${unpackDir}/index.js`;
+		const extractionValid = extractionState
+			&& extractionState.size === stat.size
+			&& extractionState.timestamp === stat.mtime.getTime()
+			&& await shim.fsDriver().exists(manifestFilePath)
+			&& await shim.fsDriver().exists(scriptFilePath);
+
+		if (!extractionValid) {
+			logger.info(`Extracting plugin: ${fname}`);
 
-		if (!manifest || manifest._package_hash !== hash) {
 			await shim.fsDriver().remove(unpackDir);
 			await shim.fsDriver().mkdir(unpackDir);
 
@@ -310,12 +350,16 @@ export default class PluginService extends BaseService {
 				cwd: unpackDir,
 			});
 
-			manifest = await this.loadManifestToObject(manifestFilePath);
+			const manifest = await this.loadManifestToObject(manifestFilePath);
 			if (!manifest) throw new Error(`Missing manifest file at: ${manifestFilePath}`);
 
-			manifest._package_hash = hash;
-
-			await shim.fsDriver().writeFile(manifestFilePath, JSON.stringify(manifest, null, '\t'), 'utf8');
+			extractionStates[fname] = {
+				size: stat.size,
+				timestamp: stat.mtime.getTime(),
+			};
+			await this.saveExtractionStates(extractionStates);
+		} else {
+			logger.info(`Using already extracted plugin: ${fname}`);
 		}
 
 		return this.loadPluginFromPath(unpackDir);
@@ -495,6 +539,7 @@ export default class PluginService extends BaseService {
 				logger.error(`Could not load plugin: ${pluginPath}`, error);
 			}
 		}
+
 	}
 
 	public async loadAndRunDevPlugins(settings: PluginSettings) {

packages/lib/services/plugins/loadPlugins.test.ts

@@ -8,6 +8,7 @@ import { Action, createStore } from 'redux';
 import MockPlatformImplementation from './testing/MockPlatformImplementation';
 import createTestPlugin from '../../testing/plugins/createTestPlugin';
 import Plugin from './Plugin';
+import shim from '../../shim';
 
 const createMockReduxStore = () => {
 	return createStore((state: State = defaultState, action: Action<string>) => {
@@ -136,6 +137,44 @@ describe('loadPlugins', () => {
 		expect([...pluginRunner.runningPluginIds].sort()).toMatchObject(expectedRunningIds);
 	});
 
+	test('should skip extraction when jpl has not changed', async () => {
+		const pluginId = 'joplin.test.plugin.packed';
+		await createTestPlugin({
+			...defaultManifestProperties,
+			id: pluginId,
+			name: 'Test JPL Plugin',
+		}, { format: 'jpl' });
+
+		const pluginRunner = new MockPluginRunner();
+		const store = createMockReduxStore();
+		const service = PluginService.instance();
+		service.initialize('2.3.4', platformImplementation, pluginRunner, store);
+
+		const tarExtractSpy = jest.spyOn(shim.fsDriver(), 'tarExtract');
+
+		// First load should extract
+		await service.loadAndRunPlugins(Setting.value('pluginDir'), Setting.value('plugins.states'));
+		expect(tarExtractSpy).toHaveBeenCalledTimes(1);
+
+		// Second load with same file should skip extraction
+		await service.unloadPlugin(pluginId);
+		await service.loadAndRunPlugins(Setting.value('pluginDir'), Setting.value('plugins.states'));
+		expect(tarExtractSpy).toHaveBeenCalledTimes(1);
+
+		// Recreating the jpl (different mtime/size) should trigger re-extraction
+		await service.unloadPlugin(pluginId);
+		await createTestPlugin({
+			...defaultManifestProperties,
+			id: pluginId,
+			name: 'Test JPL Plugin',
+		}, { format: 'jpl', onStart: '/* changed */' });
+
+		await service.loadAndRunPlugins(Setting.value('pluginDir'), Setting.value('plugins.states'));
+		expect(tarExtractSpy).toHaveBeenCalledTimes(2);
+
+		tarExtractSpy.mockRestore();
+	});
+
 	test('should not block allPluginsStarted when a plugin fails to start', async () => {
 		// This tests the fix for https://github.com/laurent22/joplin/issues/12793
 		// When a plugin crashes before calling register(), it should not block

packages/lib/testing/plugins/createTestPlugin.ts

@@ -1,8 +1,9 @@
 import { join } from 'path';
 import { PluginManifest } from '../../services/plugins/utils/types';
 import Setting from '../../models/Setting';
-import { writeFile } from 'fs-extra';
+import { mkdirp, writeFile } from 'fs-extra';
 import { defaultPluginSetting } from '../../services/plugins/PluginService';
+import shim from '../../shim';
 
 
 const setPluginEnabled = (id: string, enabled: boolean) => {
@@ -19,22 +20,35 @@ const setPluginEnabled = (id: string, enabled: boolean) => {
 interface Options {
 	onStart?: string;
 	enabled?: boolean;
+	format?: 'js' | 'jpl';
 }
 
-const createTestPlugin = async (manifest: PluginManifest, { onStart = '', enabled = true }: Options = {}) => {
-	const pluginSource = `
+const createTestPlugin = async (manifest: PluginManifest, { onStart = '', enabled = true, format = 'js' }: Options = {}) => {
+	const scriptSource = `joplin.plugins.register({
+		onStart: async function() {
+			${onStart}
+		},
+	});`;
+
+	if (format === 'jpl') {
+		const tempDir = join(Setting.value('tempDir'), `plugin-build-${manifest.id}`);
+		await mkdirp(tempDir);
+		await writeFile(join(tempDir, 'manifest.json'), JSON.stringify(manifest), 'utf-8');
+		await writeFile(join(tempDir, 'index.js'), scriptSource, 'utf-8');
+
+		const jplPath = join(Setting.value('pluginDir'), `${manifest.id}.jpl`);
+		await shim.fsDriver().tarCreate({ cwd: tempDir, file: jplPath }, ['manifest.json', 'index.js']);
+	} else {
+		const pluginSource = `
 		/* joplin-manifest:
 		${JSON.stringify(manifest)}
 		*/
 
-		joplin.plugins.register({
-			onStart: async function() {
-				${onStart}
-			},
-		});
+		${scriptSource}
 	`;
-	const pluginPath = join(Setting.value('pluginDir'), `${manifest.id}.js`);
-	await writeFile(pluginPath, pluginSource, 'utf-8');
+		const pluginPath = join(Setting.value('pluginDir'), `${manifest.id}.js`);
+		await writeFile(pluginPath, pluginSource, 'utf-8');
+	}
 
 	setPluginEnabled(manifest.id, enabled);