From 0d1845f857a52decd4e164b74afefcde1b0e2356 Mon Sep 17 00:00:00 2001 From: Jack Westbrook Date: Thu, 7 Sep 2023 14:30:35 +0200 Subject: [PATCH] Plugins: Fix applying default extension and AMD detection (#74471) * fix(plugins): support edge cases where default extension was not added * fix(plugins): only apply AMD wrapper to AMD format plugins * refactor(plugins): update regex for codeql warnings * refactor(plugins): remove check for commented out dependency strings in AMD regex * test(plugins): add tests for systemjs hook amd detection --- .../app/features/plugins/loader/constants.ts | 4 +- .../plugins/loader/pluginLoader.mock.ts | 104 +++++++++++- .../plugins/loader/systemjsHooks.test.ts | 155 ++++++++++++++++-- .../features/plugins/loader/systemjsHooks.ts | 20 +-- 4 files changed, 251 insertions(+), 32 deletions(-) diff --git a/public/app/features/plugins/loader/constants.ts b/public/app/features/plugins/loader/constants.ts index 9c247ae4d07..daf840dff2c 100644 --- a/public/app/features/plugins/loader/constants.ts +++ b/public/app/features/plugins/loader/constants.ts @@ -1,5 +1,5 @@ export const SHARED_DEPENDENCY_PREFIX = 'package'; export const LOAD_PLUGIN_CSS_REGEX = /^plugins.+\.css$/i; export const JS_CONTENT_TYPE_REGEX = /^(text|application)\/(x-)?javascript(;|$)/; -export const ENDS_WITH_FILE_EXTENSION_REGEX = /\/?\.[a-zA-Z]{2,}$/; -export const IS_SYSTEM_MODULE_REGEX = /System\.register\(/; +export const AMD_MODULE_REGEX = + /(?:^\uFEFF?|[^$_a-zA-Z\xA0-\uFFFF.])define\s*\(\s*("[^"]+"\s*,\s*|'[^']+'\s*,\s*)?\s*(\[(\s*(("[^"]+"|'[^']+')\s*,|\/\/.*\r?\n))*(\s*("[^"]+"|'[^']+')\s*,?)?(\s*(\/\/.*\r?\n|\/\*\/))*\s*\]|function\s*|{|[_$a-zA-Z\xA0-\uFFFF][_$a-zA-Z0-9\xA0-\uFFFF]*\))/; diff --git a/public/app/features/plugins/loader/pluginLoader.mock.ts b/public/app/features/plugins/loader/pluginLoader.mock.ts index c2332bdcc6a..530b245a592 100644 --- a/public/app/features/plugins/loader/pluginLoader.mock.ts +++ b/public/app/features/plugins/loader/pluginLoader.mock.ts @@ -1,13 +1,6 @@ import { rest } from 'msw'; import { setupServer } from 'msw/node'; -export const mockAmdModule = `define([], function() { - return function() { - console.log('AMD module loaded'); - var pluginPath = "/public/plugins/"; - } -});`; - export const mockSystemModule = `System.register(['./dependencyA'], function (_export, _context) { "use strict"; @@ -22,15 +15,108 @@ export const mockSystemModule = `System.register(['./dependencyA'], function (_e }; });`; +export const mockAmdModule = `define([], function() { + return function() { + console.log('AMD module loaded'); + var pluginPath = "/public/plugins/"; + } +});`; + +export const mockAmdModuleNamedNoDeps = `define("named", function() { + return function() { + console.log('AMD module loaded'); + var pluginPath = "/public/plugins/"; + } +});`; + +export const mockAmdModuleNamedWithDeps = `define("named", ["dep"], function(dep) { + return function() { + console.log('AMD module loaded'); + var pluginPath = "/public/plugins/"; + } +});`; + +export const mockAmdModuleNamedWithDeps2 = `define("named", ["dep", "dep2"], function(dep, dep2) { + return function() { + console.log('AMD module loaded'); + var pluginPath = "/public/plugins/"; + } +});`; + +export const mockAmdModuleNamedWithDeps3 = `define("named", ["dep", +"dep2" +], function(dep, dep2) { + return function() { + console.log('AMD module loaded'); + var pluginPath = "/public/plugins/"; + } +});`; + +export const mockAmdModuleOnlyFunction = `define(function() { + return function() { + console.log('AMD module loaded'); + var pluginPath = "/public/plugins/"; + } +});`; + +export const mockAmdModuleWithComments = `/*! For license information please see module.js.LICENSE.txt */ +define(function(react) { + return function() { + console.log('AMD module loaded'); + var pluginPath = "/public/plugins/"; + } +});`; + +export const mockAmdModuleWithComments2 = `/*! This is a commment */ +define(["dep"], + /*! This is a commment */ + function(dep) { + return function() { + console.log('AMD module loaded'); + var pluginPath = "/public/plugins/"; + } +});`; + +export const mockModuleWithDefineMethod = `ace.define(function() { + return function() { + console.log('AMD module loaded'); + var pluginPath = "/public/plugins/"; + } +});`; + const server = setupServer( - rest.get('/public/plugins/my-amd-plugin/module.js', async (_req, res, ctx) => + rest.get('/public/plugins/mockAmdModule/module.js', async (_req, res, ctx) => res(ctx.status(200), ctx.set('Content-Type', 'text/javascript'), ctx.body(mockAmdModule)) ), - rest.get('/public/plugins/my-system-plugin/module.js', async (_req, res, ctx) => + rest.get('/public/plugins/mockSystemModule/module.js', async (_req, res, ctx) => res(ctx.status(200), ctx.set('Content-Type', 'text/javascript'), ctx.body(mockSystemModule)) ), rest.get('http://my-cdn.com/plugins/my-plugin/v1.0.0/public/plugins/my-plugin/module.js', async (_req, res, ctx) => res(ctx.status(200), ctx.set('Content-Type', 'text/javascript'), ctx.body(mockAmdModule)) + ), + rest.get('/public/plugins/mockAmdModuleNamedNoDeps/module.js', async (_req, res, ctx) => + res(ctx.status(200), ctx.set('Content-Type', 'text/javascript'), ctx.body(mockAmdModuleNamedNoDeps)) + ), + rest.get('/public/plugins/mockAmdModuleNamedWithDeps/module.js', async (_req, res, ctx) => + res(ctx.status(200), ctx.set('Content-Type', 'text/javascript'), ctx.body(mockAmdModuleNamedWithDeps)) + ), + rest.get('/public/plugins/mockAmdModuleNamedWithDeps2/module.js', async (_req, res, ctx) => + res(ctx.status(200), ctx.set('Content-Type', 'text/javascript'), ctx.body(mockAmdModuleNamedWithDeps2)) + ), + rest.get('/public/plugins/mockAmdModuleNamedWithDeps3/module.js', async (_req, res, ctx) => + res(ctx.status(200), ctx.set('Content-Type', 'text/javascript'), ctx.body(mockAmdModuleNamedWithDeps3)) + ), + rest.get('/public/plugins/mockAmdModuleOnlyFunction/module.js', async (_req, res, ctx) => + res(ctx.status(200), ctx.set('Content-Type', 'text/javascript'), ctx.body(mockAmdModuleOnlyFunction)) + ), + rest.get('/public/plugins/mockAmdModuleWithComments/module.js', async (_req, res, ctx) => + res(ctx.status(200), ctx.set('Content-Type', 'text/javascript'), ctx.body(mockAmdModuleWithComments)) + ), + rest.get('/public/plugins/mockAmdModuleWithComments2/module.js', async (_req, res, ctx) => + res(ctx.status(200), ctx.set('Content-Type', 'text/javascript'), ctx.body(mockAmdModuleWithComments2)) + ), + rest.get('/public/plugins/mockModuleWithDefineMethod/module.js', async (_req, res, ctx) => + res(ctx.status(200), ctx.set('Content-Type', 'text/javascript'), ctx.body(mockModuleWithDefineMethod)) ) ); diff --git a/public/app/features/plugins/loader/systemjsHooks.test.ts b/public/app/features/plugins/loader/systemjsHooks.test.ts index bc58292aa54..08d33e2c644 100644 --- a/public/app/features/plugins/loader/systemjsHooks.test.ts +++ b/public/app/features/plugins/loader/systemjsHooks.test.ts @@ -7,7 +7,19 @@ jest.mock('./cache', () => ({ resolveWithCache: (url: string) => `${url}?_cache=1234`, })); -import { server, mockAmdModule, mockSystemModule } from './pluginLoader.mock'; +import { + server, + mockAmdModule, + mockSystemModule, + mockAmdModuleNamedNoDeps, + mockAmdModuleNamedWithDeps, + mockAmdModuleNamedWithDeps2, + mockAmdModuleNamedWithDeps3, + mockAmdModuleOnlyFunction, + mockAmdModuleWithComments, + mockModuleWithDefineMethod, + mockAmdModuleWithComments2, +} from './pluginLoader.mock'; import { decorateSystemJSFetch, decorateSystemJSResolve } from './systemjsHooks'; import { SystemJSWithLoaderHooks } from './types'; @@ -30,23 +42,112 @@ describe('SystemJS Loader Hooks', () => { }); describe('decorateSystemJSFetch', () => { - it('wraps amd module plugins for define function', async () => { - const url = '/public/plugins/my-amd-plugin/module.js'; - const result = await decorateSystemJSFetch(originalFetch, url, {}); - const source = await result.text(); - const expected = `(function(define) { + it('wraps AMD modules in an AMD iife', async () => { + const basicResult = await decorateSystemJSFetch(originalFetch, '/public/plugins/mockAmdModule/module.js', {}); + const basicSource = await basicResult.text(); + const basicExpected = `(function(define) { ${mockAmdModule} })(window.__grafana_amd_define);`; + expect(basicSource).toBe(basicExpected); - expect(source).toBe(expected); + const mockAmdModuleNamedNoDepsResult = await decorateSystemJSFetch( + originalFetch, + '/public/plugins/mockAmdModuleNamedNoDeps/module.js', + {} + ); + const mockAmdModuleNamedNoDepsSource = await mockAmdModuleNamedNoDepsResult.text(); + const mockAmdModuleNamedNoDepsExpected = `(function(define) { + ${mockAmdModuleNamedNoDeps} +})(window.__grafana_amd_define);`; + + expect(mockAmdModuleNamedNoDepsSource).toBe(mockAmdModuleNamedNoDepsExpected); + + const mockAmdModuleNamedWithDepsResult = await decorateSystemJSFetch( + originalFetch, + '/public/plugins/mockAmdModuleNamedWithDeps/module.js', + {} + ); + const mockAmdModuleNamedWithDepsSource = await mockAmdModuleNamedWithDepsResult.text(); + const mockAmdModuleNamedWithDepsExpected = `(function(define) { + ${mockAmdModuleNamedWithDeps} +})(window.__grafana_amd_define);`; + + expect(mockAmdModuleNamedWithDepsSource).toBe(mockAmdModuleNamedWithDepsExpected); + + const mockAmdModuleNamedWithDeps2Result = await decorateSystemJSFetch( + originalFetch, + '/public/plugins/mockAmdModuleNamedWithDeps2/module.js', + {} + ); + const mockAmdModuleNamedWithDeps2Source = await mockAmdModuleNamedWithDeps2Result.text(); + const mockAmdModuleNamedWithDeps2Expected = `(function(define) { + ${mockAmdModuleNamedWithDeps2} +})(window.__grafana_amd_define);`; + + expect(mockAmdModuleNamedWithDeps2Source).toBe(mockAmdModuleNamedWithDeps2Expected); + + const mockAmdModuleNamedWithDeps3Result = await decorateSystemJSFetch( + originalFetch, + '/public/plugins/mockAmdModuleNamedWithDeps3/module.js', + {} + ); + const mockAmdModuleNamedWithDeps3Source = await mockAmdModuleNamedWithDeps3Result.text(); + const mockAmdModuleNamedWithDeps3Expected = `(function(define) { + ${mockAmdModuleNamedWithDeps3} +})(window.__grafana_amd_define);`; + + expect(mockAmdModuleNamedWithDeps3Source).toBe(mockAmdModuleNamedWithDeps3Expected); + + const mockAmdModuleOnlyFunctionResult = await decorateSystemJSFetch( + originalFetch, + '/public/plugins/mockAmdModuleOnlyFunction/module.js', + {} + ); + const mockAmdModuleOnlyFunctionSource = await mockAmdModuleOnlyFunctionResult.text(); + const mockAmdModuleOnlyFunctionExpected = `(function(define) { + ${mockAmdModuleOnlyFunction} +})(window.__grafana_amd_define);`; + + expect(mockAmdModuleOnlyFunctionSource).toBe(mockAmdModuleOnlyFunctionExpected); + + const mockAmdModuleWithCommentsResult = await decorateSystemJSFetch( + originalFetch, + '/public/plugins/mockAmdModuleWithComments/module.js', + {} + ); + const mockAmdModuleWithCommentsSource = await mockAmdModuleWithCommentsResult.text(); + const mockAmdModuleWithCommentsExpected = `(function(define) { + ${mockAmdModuleWithComments} +})(window.__grafana_amd_define);`; + + expect(mockAmdModuleWithCommentsSource).toBe(mockAmdModuleWithCommentsExpected); + + const mockAmdModuleWithComments2Result = await decorateSystemJSFetch( + originalFetch, + '/public/plugins/mockAmdModuleWithComments2/module.js', + {} + ); + const mockAmdModuleWithComments2Source = await mockAmdModuleWithComments2Result.text(); + const mockAmdModuleWithComments2Expected = `(function(define) { + ${mockAmdModuleWithComments2} +})(window.__grafana_amd_define);`; + + expect(mockAmdModuleWithComments2Source).toBe(mockAmdModuleWithComments2Expected); }); - it("doesn't wrap system module plugins with define function", async () => { - const url = '/public/plugins/my-system-plugin/module.js'; + it("doesn't wrap system modules in an AMD iife", async () => { + const url = '/public/plugins/mockSystemModule/module.js'; const result = await decorateSystemJSFetch(originalFetch, url, {}); const source = await result.text(); expect(source).toBe(mockSystemModule); }); + it("doesn't wrap modules with a define method in an AMD iife", async () => { + const url = '/public/plugins/mockModuleWithDefineMethod/module.js'; + const result = await decorateSystemJSFetch(originalFetch, url, {}); + const source = await result.text(); + + expect(source).toBe(mockModuleWithDefineMethod); + }); it('only transforms plugin source code hosted on cdn with cdn paths', async () => { config.pluginsCDNBaseURL = 'http://my-cdn.com/plugins'; const cdnUrl = 'http://my-cdn.com/plugins/my-plugin/v1.0.0/public/plugins/my-plugin/module.js'; @@ -55,7 +156,7 @@ describe('SystemJS Loader Hooks', () => { expect(cdnSource).toContain('var pluginPath = "http://my-cdn.com/plugins/my-plugin/v1.0.0/public/plugins/";'); - const url = '/public/plugins/my-amd-plugin/module.js'; + const url = '/public/plugins/mockAmdModule/module.js'; const result = await decorateSystemJSFetch(originalFetch, url, {}); const source = await result.text(); expect(source).toContain('var pluginPath = "/public/plugins/";'); @@ -69,10 +170,44 @@ describe('SystemJS Loader Hooks', () => { expect(result).toBe('http://localhost/public/plugins/my-datasource/styles.css'); }); it('adds default js extension to resolved url', () => { + // test against missing extension const id = '/public/plugins/my-plugin/traffic_light'; const result = decorateSystemJSResolve.bind(systemJSPrototype)(originalResolve, id); expect(result).toBe('http://localhost/public/plugins/my-plugin/traffic_light.js'); + + // test against missing extension with periods in filename + const id2 = '/public/plugins/my-plugin/lib/flot/jquery.flot.gauge'; + const result2 = decorateSystemJSResolve.bind(systemJSPrototype)(originalResolve, id2); + + expect(result2).toBe('http://localhost/public/plugins/my-plugin/lib/flot/jquery.flot.gauge.js'); + + // test against bare specifiers + const id3 = 'package:lodash'; + const result3 = decorateSystemJSResolve.bind(systemJSPrototype)(originalResolve, id3); + + expect(result3).toBe('package:lodash'); + + // test against file extensions systemjs can load + const id4 = '/public/plugins/my-plugin/traffic_light.js'; + const result4 = decorateSystemJSResolve.bind(systemJSPrototype)(originalResolve, id4); + + expect(result4).toBe('http://localhost/public/plugins/my-plugin/traffic_light.js'); + + const id5 = '/public/plugins/my-plugin/traffic_light.css'; + const result5 = decorateSystemJSResolve.bind(systemJSPrototype)(originalResolve, id5); + + expect(result5).toBe('http://localhost/public/plugins/my-plugin/traffic_light.css'); + + const id6 = '/public/plugins/my-plugin/traffic_light.json'; + const result6 = decorateSystemJSResolve.bind(systemJSPrototype)(originalResolve, id6); + + expect(result6).toBe('http://localhost/public/plugins/my-plugin/traffic_light.json'); + + const id7 = '/public/plugins/my-plugin/traffic_light.wasm'; + const result7 = decorateSystemJSResolve.bind(systemJSPrototype)(originalResolve, id7); + + expect(result7).toBe('http://localhost/public/plugins/my-plugin/traffic_light.wasm'); }); it('resolves loadPluginCSS urls correctly', () => { const id = 'plugins/my-plugin/dark.css'; diff --git a/public/app/features/plugins/loader/systemjsHooks.ts b/public/app/features/plugins/loader/systemjsHooks.ts index 08023a2ebcd..8bdbd723b11 100644 --- a/public/app/features/plugins/loader/systemjsHooks.ts +++ b/public/app/features/plugins/loader/systemjsHooks.ts @@ -3,13 +3,7 @@ import { config, SystemJS } from '@grafana/runtime'; import { transformPluginSourceForCDN } from '../cdn/utils'; import { resolveWithCache } from './cache'; -import { - LOAD_PLUGIN_CSS_REGEX, - JS_CONTENT_TYPE_REGEX, - IS_SYSTEM_MODULE_REGEX, - SHARED_DEPENDENCY_PREFIX, - ENDS_WITH_FILE_EXTENSION_REGEX, -} from './constants'; +import { LOAD_PLUGIN_CSS_REGEX, JS_CONTENT_TYPE_REGEX, AMD_MODULE_REGEX, SHARED_DEPENDENCY_PREFIX } from './constants'; import { SystemJSWithLoaderHooks } from './types'; import { isHostedOnCDN } from './utils'; @@ -24,7 +18,8 @@ export async function decorateSystemJSFetch( if (JS_CONTENT_TYPE_REGEX.test(contentType)) { const source = await res.text(); let transformedSrc = source; - if (!IS_SYSTEM_MODULE_REGEX.test(transformedSrc)) { + + if (AMD_MODULE_REGEX.test(transformedSrc)) { transformedSrc = preventAMDLoaderCollision(source); } @@ -78,13 +73,16 @@ export function decorateSystemJsOnload(err: unknown, id: string) { // - strips legacy loader wildcard from urls // - support config.defaultExtension for System.register deps that lack an extension (e.g. './my_ctrl') function getBackWardsCompatibleUrl(url: string) { + if (url.startsWith(`${SHARED_DEPENDENCY_PREFIX}:`)) { + return url; + } if (url.endsWith('!')) { url = url.slice(0, -1); } - const shouldAddDefaultExtension = - !url.startsWith(`${SHARED_DEPENDENCY_PREFIX}:`) && !ENDS_WITH_FILE_EXTENSION_REGEX.test(url); + const systemJSFileExtensions = ['css', 'js', 'json', 'wasm']; + const hasValidFileExtension = systemJSFileExtensions.some((extensionName) => url.endsWith(extensionName)); - return shouldAddDefaultExtension ? url + '.js' : url; + return hasValidFileExtension ? url : url + '.js'; } // This transform prevents a conflict between systemjs and requirejs which Monaco Editor