Skip to content

Commit

Permalink
[BREAKING] Discontinue bundling of JavaScript modules as string
Browse files Browse the repository at this point in the history
The UI5 bundler packages JavaScript files that are identified as "requiresTopLevelScope" as a string, to be evaluated via "eval" at runtime. This behavior ensures that the script works as expected, e.g. with regards to implicit globals.
This "eval" runtime feature with be discontinued in UI5 2.0 because of security best practices (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/eval) and to comply with stricter CSP settings (unsafe-eval).
By using specVersion 4.0 an error is thrown when bundling a module as string and the module is not included in the bundle at all.

JIRA: CPOUI5FOUNDATION-794
  • Loading branch information
flovogt committed Jul 4, 2024
1 parent 934956a commit 88bcad2
Show file tree
Hide file tree
Showing 13 changed files with 450 additions and 124 deletions.
66 changes: 48 additions & 18 deletions lib/lbt/bundle/Builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,13 @@ function isEmptyBundle(resolvedBundle) {
}

class BundleBuilder {
constructor(pool, targetUi5CoreVersion) {
constructor(pool, targetUi5CoreVersion, allowStringBundling) {
this.pool = pool;
this.resolver = new BundleResolver(pool);
this.splitter = new BundleSplitter(pool, this.resolver);
this.targetUi5CoreVersion = targetUi5CoreVersion;
this.targetUi5CoreVersionMajor = undefined;
this.allowStringBundling = allowStringBundling;
}

getUi5MajorVersion() {
Expand Down Expand Up @@ -172,7 +173,7 @@ class BundleBuilder {

this.closeModule(resolvedModule);

const bundleInfo = await resolvedModule.createModuleInfo(this.pool);
const bundleInfo = await resolvedModule.createModuleInfo(this.pool, this.allowStringBundling);
bundleInfo.size = this.outW.length;

return {
Expand Down Expand Up @@ -231,23 +232,23 @@ class BundleBuilder {
}
}

addSection(section) {
async addSection(section) {
this.ensureRawDeclarations();

switch (section.mode) {
case SectionType.Provided:
// do nothing
return undefined; // nothing to wait for
case SectionType.Raw:
return this.writeRaw(section);
return await this.writeRaw(section);
case SectionType.Preload:
return this.writePreloadFunction(section);
return await this.writePreloadFunction(section);
case SectionType.BundleInfo:
return this.writeBundleInfos([section]);
return await this.writeBundleInfos([section]);
case SectionType.Require:
return this.writeRequires(section);
return await this.writeRequires(section);
case SectionType.DepCache:
return this.writeDepCache(section);
return await this.writeDepCache(section);
default:
throw new Error("unknown section mode " + section.mode);
}
Expand Down Expand Up @@ -330,7 +331,6 @@ class BundleBuilder {
if ( i>0 ) {
outW.writeln(",");
}
// this.beforeWritePreloadModule(module, resource.info, resource);
outW.write(`\t"${module.toString()}":`);
outW.startSegment(module);
await this.writePreloadModule(module, resource.info, resource);
Expand Down Expand Up @@ -397,8 +397,16 @@ class BundleBuilder {
const remaining = [];
for ( const moduleName of sequence ) {
if ( /\.js$/.test(moduleName) ) {
// console.log("Processing " + moduleName);
const resource = await this.pool.findResourceWithInfo(moduleName);

if (resource.info?.requiresTopLevelScope && !this.allowStringBundling) {
log.error(
"Module " + moduleName + " requires top level scope and can only be embedded as a string " +
"(requires 'eval'), which is not supported with specVersion 4.0 and newer");

continue;
}

let moduleContent = (await resource.buffer()).toString();
moduleContent = removeHashbang(moduleContent);
let moduleSourceMap;
Expand Down Expand Up @@ -472,14 +480,15 @@ class BundleBuilder {
outW.write(`}`);
} else if ( /\.js$/.test(moduleName) /* implicitly: && info != null && info.requiresTopLevelScope */ ) {
log.warn(
`Module ${moduleName} requires top level scope and can only be embedded as a string (requires 'eval')`);
"Module " + moduleName + " requires top level scope and can only be embedded as a string " +
"(requires 'eval')");
let moduleContent = (await resource.buffer()).toString();
moduleContent = removeHashbang(moduleContent);
if (this.options.sourceMap) {
// We are actually not interested in the source map this module might contain,
// but we should make sure to remove any "sourceMappingURL" from the module content before
// writing it to the bundle. Otherwise browser dev-tools might create unnecessary (and likely incorrect)
// requests for any referenced .map files
// writing it to the bundle. Otherwise browser dev-tools might create unnecessary
// (and likely incorrect) requests for any referenced .map files
({moduleContent} =
await this.getSourceMapForModule({
moduleName,
Expand Down Expand Up @@ -542,15 +551,36 @@ class BundleBuilder {
});
}

writeBundleInfos(sections) {
async checkForGlobals(moduleName) {
if (!this.allowStringBundling && /\.js$/.test(moduleName)) {
const resource = await this.pool.findResourceWithInfo(moduleName);
if (resource.info?.requiresTopLevelScope) {
log.error(
"Module " + moduleName + " requires top level scope and can only be embedded as a string " +
"(requires 'eval'), which is not supported with specVersion 4.0 and newer");
return null;
}
}
return moduleName;
}

async writeBundleInfos(sections) {
this.outW.ensureNewLine();

let bundleInfoStr = "";
if ( sections.length > 0 ) {
bundleInfoStr = "sap.ui.loader.config({bundlesUI5:{\n";
sections.forEach((section, idx) => {
if ( idx > 0 ) {
let initial = true;
for (let idx=0; idx < sections.length; idx++) {
const section = sections[idx];

let modules = await Promise.all(section.modules.map(this.checkForGlobals.bind(this)));
modules = modules.filter(($) => $) || [];

if (!initial) {
bundleInfoStr += ",\n";
} else {
initial=false;
}

if (!section.name) {
Expand All @@ -561,8 +591,8 @@ class BundleBuilder {
`The info might not work as expected. ` +
`The name must match the bundle filename (incl. extension such as '.js')`);
}
bundleInfoStr += `"${section.name}":[${section.modules.map(makeStringLiteral).join(",")}]`;
});
bundleInfoStr += `"${section.name}":[${modules.map(makeStringLiteral).join(",")}]`;
}
bundleInfoStr += "\n}});\n";

this.writeWithSourceMap(bundleInfoStr);
Expand Down
19 changes: 4 additions & 15 deletions lib/lbt/bundle/ResolvedBundleDefinition.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class ResolvedBundleDefinition {
);
}

createModuleInfo(pool) {
createModuleInfo(pool, allowStringBundling) {
const bundleInfo = new ModuleInfo();
bundleInfo.name = this.name;

Expand Down Expand Up @@ -66,7 +66,9 @@ class ResolvedBundleDefinition {
modules.map( (submodule) => {
return pool.getModuleInfo(submodule).then(
(subinfo) => {
if (!bundleInfo.subModules.includes(subinfo.name)) {
if (!bundleInfo.subModules.includes(subinfo.name) &&
(!subinfo.requiresTopLevelScope ||
(subinfo.requiresTopLevelScope && allowStringBundling))) {
bundleInfo.addSubModule(subinfo);
}
}
Expand All @@ -78,19 +80,6 @@ class ResolvedBundleDefinition {

return promise.then( () => bundleInfo );
}

/*
public JSModuleDefinition getDefinition() {
return moduleDefinition;
}
public Configuration getConfiguration() {
return moduleDefinition.getConfiguration();
}
}
*/
}

class ResolvedSection {
Expand Down
5 changes: 3 additions & 2 deletions lib/processors/bundlers/moduleBundler.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,13 @@ const log = getLogger("builder:processors:bundlers:moduleBundler");
* @param {string} [parameters.options.targetUi5CoreVersion] Optional semver compliant sap.ui.core project version, e.g '2.0.0'.
This allows the bundler to make assumptions on available runtime APIs.
Omit if the ultimate UI5 version at runtime is unknown or can't be determined.
* @param {boolean} [parameters.options.allowStringBundling=false] Optional flag to allow bundling of modules as a string.
* @returns {Promise<module:@ui5/builder/processors/bundlers/moduleBundler~ModuleBundlerResult[]>}
* Promise resolving with module bundle resources
*/
/* eslint-enable max-len */
export default function({resources, options: {
bundleDefinition, bundleOptions, moduleNameMapping, targetUi5CoreVersion
bundleDefinition, bundleOptions, moduleNameMapping, targetUi5CoreVersion, allowStringBundling = false
}}) {
// Apply defaults without modifying the passed object
bundleOptions = Object.assign({}, {
Expand All @@ -164,7 +165,7 @@ export default function({resources, options: {
const pool = new LocatorResourcePool({
ignoreMissingModules: bundleOptions.ignoreMissingModules
});
const builder = new BundleBuilder(pool, targetUi5CoreVersion);
const builder = new BundleBuilder(pool, targetUi5CoreVersion, allowStringBundling);

if (log.isLevelEnabled("verbose")) {
log.verbose(`Generating bundle:`);
Expand Down
3 changes: 2 additions & 1 deletion lib/tasks/bundlers/generateBundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,9 @@ export default async function({
});
}
const coreVersion = taskUtil?.getProject("sap.ui.core")?.getVersion();
const allowStringBundling = taskUtil?.getProject().getSpecVersion().lt("4.0");
return combo.byGlob("/resources/**/*.{js,json,xml,html,properties,library,js.map}").then((resources) => {
const options = {bundleDefinition, bundleOptions};
const options = {bundleDefinition, bundleOptions, allowStringBundling};
if (!optimize && taskUtil) {
options.moduleNameMapping = createModuleNameMapping({resources, taskUtil});
}
Expand Down
4 changes: 3 additions & 1 deletion lib/tasks/bundlers/generateComponentPreload.js
Original file line number Diff line number Diff line change
Expand Up @@ -145,14 +145,16 @@ export default async function({
});
}
const coreVersion = taskUtil?.getProject("sap.ui.core")?.getVersion();
const allowStringBundling = taskUtil?.getProject().getSpecVersion().lt("4.0");
return Promise.all(bundleDefinitions.filter(Boolean).map((bundleDefinition) => {
log.verbose(`Generating ${bundleDefinition.name}...`);
const options = {
bundleDefinition,
bundleOptions: {
ignoreMissingModules: true,
optimize: true
}
},
allowStringBundling
};
if (coreVersion) {
options.targetUi5CoreVersion = coreVersion;
Expand Down
3 changes: 2 additions & 1 deletion lib/tasks/bundlers/generateLibraryPreload.js
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ export default async function({workspace, taskUtil, options: {skipBundles = [],
});
}
const coreVersion = taskUtil?.getProject("sap.ui.core")?.getVersion();

const allowStringBundling = taskUtil?.getProject().getSpecVersion().lt("4.0");
const execModuleBundlerIfNeeded = ({options, resources}) => {
if (skipBundles.includes(options.bundleDefinition.name)) {
log.verbose(`Skipping generation of bundle ${options.bundleDefinition.name}`);
Expand All @@ -265,6 +265,7 @@ export default async function({workspace, taskUtil, options: {skipBundles = [],
if (coreVersion) {
options.targetUi5CoreVersion = coreVersion;
}
options.allowStringBundling = allowStringBundling;
return moduleBundler({options, resources});
};

Expand Down
7 changes: 5 additions & 2 deletions lib/tasks/bundlers/generateStandaloneAppBundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,13 +138,15 @@ export default async function({workspace, dependencies, taskUtil, options}) {
});
}

const allowStringBundling = taskUtil?.getProject().getSpecVersion().lt("4.0");
const bundleOptions = {
bundleDefinition: getBundleDefinition({
name: "sap-ui-custom.js",
filters,
namespace,
preloadSection: true
})
}),
allowStringBundling
};

const bundleDbgOptions = {
Expand All @@ -156,7 +158,8 @@ export default async function({workspace, dependencies, taskUtil, options}) {
bundleOptions: {
optimize: false
},
moduleNameMapping: unoptimizedModuleNameMapping
moduleNameMapping: unoptimizedModuleNameMapping,
allowStringBundling
};

if (coreVersion) {
Expand Down
Loading

0 comments on commit 88bcad2

Please sign in to comment.