Skip to content

Commit

Permalink
fix(compartment-mapper): require isAbsolute for dynamic imports
Browse files Browse the repository at this point in the history
This adds a new prop to `ReadPowers`: `isAbsolute()`. This function is used to load such a module specifier as an exit module in `importNowHook`, since it could be pointing to anything.
  • Loading branch information
boneskull committed Jun 21, 2024
1 parent 45c147e commit 3861722
Show file tree
Hide file tree
Showing 12 changed files with 126 additions and 17 deletions.
19 changes: 18 additions & 1 deletion packages/compartment-mapper/src/import-hook.js
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,7 @@ export function makeImportNowHookMaker(
}
}

const { readSync } = readPowers;
const { readSync, isAbsolute } = readPowers;

/** @type {ImportNowHook} */
const importNowHook = moduleSpecifier => {
Expand All @@ -504,6 +504,23 @@ export function makeImportNowHookMaker(
);
}

if (isAbsolute(moduleSpecifier)) {
if (exitModuleImportNowHook) {
// this hook is responsible for ensuring that the moduleSpecifier actually refers to an exit module
const record = exitModuleImportNowHook(
moduleSpecifier,
packageLocation,
);

if (!record) {
throw new Error(`Could not import module: ${moduleSpecifier}`);
}
return record;
}

throw new Error(`Could not import module: ${moduleSpecifier}`);
}

// Collate candidate locations for the moduleSpecifier,
// to support Node.js conventions and similar.
const candidates = [moduleSpecifier];
Expand Down
45 changes: 31 additions & 14 deletions packages/compartment-mapper/src/node-powers.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,28 @@

// @ts-check

/** @import {CanonicalFn} from './types.js' */
/** @import {CryptoAPI} from './types.js' */
/** @import {FileURLToPathFn} from './types.js' */
/** @import {FsAPI} from './types.js' */
/** @import {HashFn} from './types.js' */
/** @import {IsAbsoluteFn} from './types.js' */
/** @import {MaybeReadFn} from './types.js' */
/** @import {MaybeReadPowers} from './types.js' */
/** @import {PathAPI} from './types.js' */
/** @import {PathToFileURLFn} from './types.js' */
/** @import {ReadFn} from './types.js' */
/** @import {ReadPowers} from './types.js' */
/** @import {ReadSyncFn} from './types.js' */
/** @import {FsAPI} from './types.js' */
/** @import {CryptoAPI} from './types.js' */
/** @import {UrlAPI} from './types.js' */
/** @import {MaybeReadPowers} from './types.js' */
/** @import {MaybeReadFn} from './types.js' */
/** @import {RequireResolveFn} from './types.js' */
/** @import {CanonicalFn} from './types.js' */
/** @import {SyncReadPowers} from './types.js' */
/** @import {HashFn} from './types.js' */
/** @import {UrlAPI} from './types.js' */
/** @import {WritePowers} from './types.js' */

import { createRequire } from 'module';

/**
* @param {string} location
* @type {FileURLToPathFn}
*/
const fakeFileURLToPath = location => {
const url = new URL(location);
Expand All @@ -36,12 +40,17 @@ const fakeFileURLToPath = location => {
};

/**
* @param {string} path
* @type {PathToFileURLFn} path
*/
const fakePathToFileURL = path => {
return new URL(path, 'file://').toString();
};

/**
* @type {IsAbsoluteFn}
*/
const fakeIsAbsolute = () => false;

/**
* The implementation of `makeReadPowers` and the deprecated
* `makeNodeReadPowers` handles the case when the `url` power is not provided,
Expand All @@ -51,13 +60,20 @@ const fakePathToFileURL = path => {
* @param {FsAPI} args.fs
* @param {UrlAPI} [args.url]
* @param {CryptoAPI} [args.crypto]
* @param {PathAPI} [args.path]
* @returns {MaybeReadPowers & SyncReadPowers}
*/
const makeReadPowersSloppy = ({ fs, url = undefined, crypto = undefined }) => {
const makeReadPowersSloppy = ({
fs,
url = undefined,
crypto = undefined,
path = undefined,
}) => {
const fileURLToPath =
url === undefined ? fakeFileURLToPath : url.fileURLToPath;
const pathToFileURL =
url === undefined ? fakePathToFileURL : url.pathToFileURL;
const isAbsolute = path === undefined ? fakeIsAbsolute : path.isAbsolute;

let readMutex = Promise.resolve(undefined);

Expand All @@ -72,11 +88,11 @@ const makeReadPowersSloppy = ({ fs, url = undefined, crypto = undefined }) => {
});
await promise;

const path = fileURLToPath(location);
const filepath = fileURLToPath(location);
try {
// We await here to ensure that we release the mutex only after
// completing the read.
return await fs.promises.readFile(path);
return await fs.promises.readFile(filepath);
} finally {
release(undefined);
}
Expand All @@ -86,8 +102,8 @@ const makeReadPowersSloppy = ({ fs, url = undefined, crypto = undefined }) => {
* @type {ReadSyncFn}
*/
const readSync = location => {
const path = fileURLToPath(location);
return fs.readFileSync(path);
const filepath = fileURLToPath(location);
return fs.readFileSync(filepath);
};

/**
Expand Down Expand Up @@ -157,6 +173,7 @@ const makeReadPowersSloppy = ({ fs, url = undefined, crypto = undefined }) => {
computeSha512,
requireResolve,
readSync,
isAbsolute,
};
};

Expand Down
5 changes: 4 additions & 1 deletion packages/compartment-mapper/src/powers.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export const unpackReadPowers = powers => {
*
* 1. `readSync` is a function
* 2. `fileURLToPath` is a function
* 3. `isAbsolute` is a function
*
* @param {ReadPowers|ReadFn} value
* @returns {value is SyncReadPowers}
Expand All @@ -67,4 +68,6 @@ export const isSyncReadPowers = value =>
'readSync' in value &&
typeof value.readSync === 'function' &&
'fileURLToPath' in value &&
typeof value.fileURLToPath === 'function';
typeof value.fileURLToPath === 'function' &&
'isAbsolute' in value &&
typeof value.isAbsolute === 'function';
20 changes: 19 additions & 1 deletion packages/compartment-mapper/src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,12 @@ export {};
* @returns {string}
*/

/**
* @callback IsAbsoluteFn
* @param {string} location
* @returns {boolean}
*/

/**
* Node.js' `url.pathToFileURL` only returns a {@link URL}.
* @callback PathToFileURLFn
Expand All @@ -198,12 +204,19 @@ export {};
* @property {FileURLToPathFn} [fileURLToPath]
* @property {PathToFileURLFn} [pathToFileURL]
* @property {RequireResolveFn} [requireResolve]
* @property {IsAbsoluteFn} [isAbsolute]
*/

/**
* These properties are necessary for dynamic require support
*
* @typedef {'fileURLToPath'|'readSync'|'isAbsolute'} SyncReadPowersProps
*/

/**
* {@link ReadPowers} supporting synchronous reads and dynamic requires
*
* @typedef {ReadPowers & {readSync: ReadSyncFn, fileURLToPath: FileURLToPathFn}} SyncReadPowers
* @typedef {Omit<ReadPowers, SyncReadPowersProps> & Required<Pick<ReadPowers, SyncReadPowersProps>>} SyncReadPowers
*/

/**
Expand Down Expand Up @@ -728,6 +741,11 @@ export {};
* @property {typeof import('crypto').createHash} createHash
*/

/**
* @typedef PathAPI
* @property {IsAbsoluteFn} isAbsolute
*/

/**
* Options for `compartmentMapForNodeModules`
*
Expand Down
29 changes: 29 additions & 0 deletions packages/compartment-mapper/test/dynamic-require.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,35 @@ test('intra-package dynamic require works', async t => {
);
});

test('intra-package dynamic require with absolute path works', async t => {
const fixture = new URL(
'fixtures-dynamic/node_modules/absolute-app/index.js',
import.meta.url,
).toString();
const { namespace } = await importLocation(readPowers, fixture, {
policy: {
entry: {
packages: 'any',
},
resources: {
absolute: {
dynamic: true,
},
},
},
});

t.deepEqual(
{
default: {
isOk: 1,
},
isOk: 1,
},
{ ...namespace },
);
});

test('dynamic require fails without sync read powers', async t => {
const fixture = new URL(
'fixtures-dynamic/node_modules/app/index.js',
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 3861722

Please sign in to comment.