Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(endo): Search for index.js and implied package exports #424

Closed
wants to merge 13 commits into from

Conversation

kriskowal
Copy link
Member

When asked for a module named "x", Node.js will search for "x.js" then "x/index.js".

For packages that do not supply an "exports" property in their "package.json", any module contained by that package is a valid exported module.

To achieve parity with these two features, Endo uses different techniques when loading from the file system and loading from an archive.

When reading from the file system, Endo will search for a satsifactory candidate in each compartment's asynchronous importHook. Endo also uses the compartment's new moduleMapHook to search for dependency compartments in the "scope" of a module identifier prefix. These allow Endo to operate from an incomplete compartment map for the initial load.

The Endo archiver instead creates a more complete compartment map, with every discovered module. This introduces a new kind of module to the compartment map module descriptor type union: modules with known locations and corresponding parsers. The archiver erases the "scopes", "types", and "parsers" on each compartment description since they are no longer necessary.

When reading from an archive, Endo uses an importHook that consults the compartment map directly for the locations of all contained modules, and creates a complete moduleMap up front.

@kriskowal kriskowal mentioned this pull request Aug 22, 2020
36 tasks
Copy link
Member

@michaelfig michaelfig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand all the details, but here are some comments.


// ExitName is the name of a built-in module, to be threaded in from the
// modules passed to the module executor.
type ExitName string;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
type ExitName string;
type ExitName = string;

// In Node.js, an absolute specifier always indicates a built-in or
// third-party dependency.
// The `moduleMapHook` captures all third-party dependencies.
if (moduleSpecifier !== "." && !moduleSpecifier.startsWith("./")) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you also need to handle

&& moduleSpecifier !== '..' && !moduleSpecifier.startsWith('../')

?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good in general to validate the moduleSpecifier. Any specifier that starts with ../ is invalid because it escapes the package root. I have a utility that throws if a module specifier escapes the package root in general, which I could use as a guard here.

const types = {};

Object.assign(result, {
label: `${name}${version ? `-v${version}` : ""}`,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would @version be better, to help avoid naming collisions.

Suggested change
label: `${name}${version ? `-v${version}` : ""}`,
label: `${name}${version ? `@${version}` : ""}`,

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was previously using name@version#number for compartment labels in archives, and quickly got in trouble when using URL math with them. I might try again, but they would end up % encoded.

When asked for a module named "x", Node.js will search for "x.js" then "x/index.js".

For packages that do not supply an "exports" property in their "package.json", any module contained by that package is a valid exported module.

To achieve parity with these two features, Endo uses different techniques when loading from the file system and loading from an archive.

When reading from the file system, Endo will search for a satsifactory candidate in each compartment's asynchronous importHook.  Endo also uses the compartment's new moduleMapHook to search for dependency compartments in the "scope" of a module identifier prefix.  These allow Endo to operate from an incomplete compartment map for the initial load.

The Endo archiver instead creates a more complete compartment map, with every discovered module.  This introduces a new kind of module to the compartment map module descriptor type union: modules with known locations and corresponding parsers.  The archiver erases the "scopes", "types", and "parsers" on each compartment description since they are no longer necessary.

When reading from an archive, Endo uses an importHook that consults the compartment map directly for the locations of all contained modules, and creates a complete moduleMap up front.
@kriskowal kriskowal force-pushed the kris/endo-mitm branch 10 times, most recently from cb1fcad to bf310ac Compare August 26, 2020 18:25
@kriskowal kriskowal marked this pull request as draft August 26, 2020 18:27
@kriskowal
Copy link
Member Author

Looks like I messed up the stack and that this was a duplicate of #434

@kriskowal kriskowal closed this Aug 26, 2020
@kriskowal kriskowal deleted the kris/endo-probe branch August 26, 2020 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants