Skip to content

Commit

Permalink
lib,src: drop --experimental-network-imports
Browse files Browse the repository at this point in the history
  • Loading branch information
RafaelGSS committed Jul 12, 2024
1 parent 24648b5 commit d76edeb
Show file tree
Hide file tree
Showing 10 changed files with 3 additions and 219 deletions.
13 changes: 0 additions & 13 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -986,18 +986,6 @@ changes:
Specify the `module` containing exported [module customization hooks][].
`module` may be any string accepted as an [`import` specifier][].

### `--experimental-network-imports`

<!-- YAML
added:
- v17.6.0
- v16.15.0
-->

> Stability: 1 - Experimental
Enable experimental support for the `https:` protocol in `import` specifiers.

### `--experimental-permission`

<!-- YAML
Expand Down Expand Up @@ -2854,7 +2842,6 @@ one is included in the list below.
* `--experimental-json-modules`
* `--experimental-loader`
* `--experimental-modules`
* `--experimental-network-imports`
* `--experimental-permission`
* `--experimental-print-required-tla`
* `--experimental-require-module`
Expand Down
17 changes: 0 additions & 17 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -3583,23 +3583,6 @@ removed: v10.0.0

Used by the `Node-API` when `Constructor.prototype` is not an object.

<a id="ERR_NETWORK_IMPORT_BAD_RESPONSE"></a>

### `ERR_NETWORK_IMPORT_BAD_RESPONSE`

> Stability: 1 - Experimental
Response was received but was invalid when importing a module over the network.

<a id="ERR_NETWORK_IMPORT_DISALLOWED"></a>

### `ERR_NETWORK_IMPORT_DISALLOWED`

> Stability: 1 - Experimental
A network module attempted to load another module that it is not allowed to
load. Likely this restriction is for security reasons.

<a id="ERR_NO_LONGER_SUPPORTED"></a>

### `ERR_NO_LONGER_SUPPORTED`
Expand Down
68 changes: 1 addition & 67 deletions doc/api/esm.md
Original file line number Diff line number Diff line change
Expand Up @@ -697,71 +697,6 @@ spawn(execPath, [
});
```
## HTTPS and HTTP imports
> Stability: 1 - Experimental
Importing network based modules using `https:` and `http:` is supported under
the `--experimental-network-imports` flag. This allows web browser-like imports
to work in Node.js with a few differences due to application stability and
security concerns that are different when running in a privileged environment
instead of a browser sandbox.
### Imports are limited to HTTP/1
Automatic protocol negotiation for HTTP/2 and HTTP/3 is not yet supported.
### HTTP is limited to loopback addresses
`http:` is vulnerable to man-in-the-middle attacks and is not allowed to be
used for addresses outside of the IPv4 address `127.0.0.0/8` (`127.0.0.1` to
`127.255.255.255`) and the IPv6 address `::1`. Support for `http:` is intended
to be used for local development.
### Authentication is never sent to the destination server.
`Authorization`, `Cookie`, and `Proxy-Authorization` headers are not sent to the
server. Avoid including user info in parts of imported URLs. A security model
for safely using these on the server is being worked on.
### CORS is never checked on the destination server
CORS is designed to allow a server to limit the consumers of an API to a
specific set of hosts. This is not supported as it does not make sense for a
server-based implementation.
### Cannot load non-network dependencies
These modules cannot access other modules that are not over `http:` or `https:`.
To still access local modules while avoiding the security concern, pass in
references to the local dependencies:
```mjs
// file.mjs
import worker_threads from 'node:worker_threads';
import { configure, resize } from 'https://example.com/imagelib.mjs';
configure({ worker_threads });
```
```mjs
// https://example.com/imagelib.mjs
let worker_threads;
export function configure(opts) {
worker_threads = opts.worker_threads;
}
export function resize(img, size) {
// Perform resizing in worker_thread to avoid main thread blocking
}
```
### Network-based loading is not enabled by default
For now, the `--experimental-network-imports` flag is required to enable loading
resources over `http:` or `https:`. In the future, a different mechanism will be
used to enforce this. Opt-in is required to prevent transitive dependencies
inadvertently using potentially mutable state that could affect reliability
of Node.js applications.
<i id="esm_experimental_loaders"></i>
## Loaders
Expand Down Expand Up @@ -804,8 +739,7 @@ does not determine whether the resolved URL protocol can be loaded,
or whether the file extensions are permitted, instead these validations
are applied by Node.js during the load phase
(for example, if it was asked to load a URL that has a protocol that is
not `file:`, `data:`, `node:`, or if `--experimental-network-imports`
is enabled, `https:`).
not `file:`, `data:` or `node:`.
The algorithm also tries to determine the format of the file based
on the extension (see `ESM_FILE_FORMAT` algorithm below). If it does
Expand Down
3 changes: 0 additions & 3 deletions doc/node.1
Original file line number Diff line number Diff line change
Expand Up @@ -173,9 +173,6 @@ Specify the
.Ar module
to use as a custom module loader.
.
.It Fl -experimental-network-imports
Enable experimental support for loading modules using `import` over `https:`.
.
.It Fl -experimental-permission
Enable the experimental permission model.
.
Expand Down
4 changes: 0 additions & 4 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1591,10 +1591,6 @@ E('ERR_NAPI_INVALID_TYPEDARRAY_ALIGNMENT',
'start offset of %s should be a multiple of %s', RangeError);
E('ERR_NAPI_INVALID_TYPEDARRAY_LENGTH',
'Invalid typed array length', RangeError);
E('ERR_NETWORK_IMPORT_BAD_RESPONSE',
"import '%s' received a bad response: %s", Error);
E('ERR_NETWORK_IMPORT_DISALLOWED',
"import of '%s' by %s is not supported: %s", Error);
E('ERR_NOT_BUILDING_SNAPSHOT',
'Operation cannot be invoked when not building startup snapshot', Error);
E('ERR_NOT_IN_SINGLE_EXECUTABLE_APPLICATION',
Expand Down
23 changes: 0 additions & 23 deletions lib/internal/modules/esm/get_format.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

const {
ObjectPrototypeHasOwnProperty,
PromisePrototypeThen,
PromiseResolve,
RegExpPrototypeExec,
SafeSet,
StringPrototypeCharCodeAt,
Expand All @@ -17,8 +15,6 @@ const {
mimeToFormat,
} = require('internal/modules/esm/formats');

const experimentalNetworkImports =
getOptionValue('--experimental-network-imports');
const { containsModuleSyntax } = internalBinding('contextify');
const { getPackageScopeConfig, getPackageType } = require('internal/modules/package_json_reader');
const { fileURLToPath } = require('internal/url');
Expand All @@ -28,8 +24,6 @@ const protocolHandlers = {
'__proto__': null,
'data:': getDataProtocolModuleFormat,
'file:': getFileProtocolModuleFormat,
'http:': getHttpProtocolModuleFormat,
'https:': getHttpProtocolModuleFormat,
'node:'() { return 'builtin'; },
};

Expand Down Expand Up @@ -176,23 +170,6 @@ function getFileProtocolModuleFormat(url, context = { __proto__: null }, ignoreE
throw new ERR_UNKNOWN_FILE_EXTENSION(ext, filepath);
}

/**
* @param {URL} url
* @param {{parentURL: string}} context
* @returns {Promise<string> | undefined} only works when enabled
*/
function getHttpProtocolModuleFormat(url, context) {
if (experimentalNetworkImports) {
const { fetchModule } = require('internal/modules/esm/fetch_module');
return PromisePrototypeThen(
PromiseResolve(fetchModule(url, context)),
(entry) => {
return mimeToFormat(entry.headers['content-type']);
},
);
}
}

/**
* @param {URL} url
* @param {{parentURL: string}} context
Expand Down
88 changes: 2 additions & 86 deletions lib/internal/modules/esm/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ const { getOptionValue } = require('internal/options');
const { sep, posix: { relative: relativePosixPath }, resolve } = require('path');
const preserveSymlinks = getOptionValue('--preserve-symlinks');
const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main');
const experimentalNetworkImports =
getOptionValue('--experimental-network-imports');
const inputTypeFlag = getOptionValue('--input-type');
const { URL, pathToFileURL, fileURLToPath, isURL, URLParse } = require('internal/url');
const { getCWDURL, setOwnProperty } = require('internal/util');
Expand All @@ -48,7 +46,6 @@ const {
ERR_PACKAGE_PATH_NOT_EXPORTED,
ERR_UNSUPPORTED_DIR_IMPORT,
ERR_UNSUPPORTED_RESOLVE_REQUEST,
ERR_NETWORK_IMPORT_DISALLOWED,
} = require('internal/errors').codes;

const { Module: CJSModule } = require('internal/modules/cjs/loader');
Expand Down Expand Up @@ -886,10 +883,6 @@ function moduleResolve(specifier, base, conditions, preserveSymlinks) {
StringPrototypeSlice(base, 0, StringPrototypeIndexOf(base, ':') + 1) :
base.protocol;
const isData = protocol === 'data:';
const isRemote =
isData ||
protocol === 'http:' ||
protocol === 'https:';
// Order swapped from spec for minor perf gain.
// Ok since relative URLs cannot parse as URLs.
let resolved;
Expand All @@ -907,7 +900,7 @@ function moduleResolve(specifier, base, conditions, preserveSymlinks) {
try {
resolved = new URL(specifier);
} catch (cause) {
if (isRemote && !BuiltinModule.canBeRequiredWithoutScheme(specifier)) {
if (isData && !BuiltinModule.canBeRequiredWithoutScheme(specifier)) {
const error = new ERR_UNSUPPORTED_RESOLVE_REQUEST(specifier, base);
setOwnProperty(error, 'cause', cause);
throw error;
Expand Down Expand Up @@ -976,57 +969,6 @@ function resolveAsCommonJS(specifier, parentURL) {
}
}

/**
* Throw an error if an import is not allowed.
* TODO(@JakobJingleheimer): de-dupe `specifier` & `parsed`
* @param {string} specifier - The import specifier.
* @param {URL} parsed - The parsed URL of the import specifier.
* @param {URL} parsedParentURL - The parsed URL of the parent module.
* @throws {ERR_NETWORK_IMPORT_DISALLOWED} - If the import is disallowed.
*/
function checkIfDisallowedImport(specifier, parsed, parsedParentURL) {
if (parsedParentURL) {
// Avoid accessing the `protocol` property due to the lazy getters.
const parentProtocol = parsedParentURL.protocol;
if (
parentProtocol === 'http:' ||
parentProtocol === 'https:'
) {
if (shouldBeTreatedAsRelativeOrAbsolutePath(specifier)) {
// Avoid accessing the `protocol` property due to the lazy getters.
const parsedProtocol = parsed?.protocol;
// data: and blob: disallowed due to allowing file: access via
// indirection
if (parsedProtocol &&
parsedProtocol !== 'https:' &&
parsedProtocol !== 'http:'
) {
throw new ERR_NETWORK_IMPORT_DISALLOWED(
specifier,
parsedParentURL,
'remote imports cannot import from a local location.',
);
}

return { url: parsed.href };
}
if (BuiltinModule.canBeRequiredWithoutScheme(specifier)) {
throw new ERR_NETWORK_IMPORT_DISALLOWED(
specifier,
parsedParentURL,
'remote imports cannot import from a local location.',
);
}

throw new ERR_NETWORK_IMPORT_DISALLOWED(
specifier,
parsedParentURL,
'only relative and absolute specifiers are supported.',
);
}
}
}

/**
* Validate user-input in `context` supplied by a custom loader.
* @param {string | URL | undefined} parentURL - The parent URL.
Expand Down Expand Up @@ -1068,36 +1010,10 @@ function defaultResolve(specifier, context = {}) {
// Avoid accessing the `protocol` property due to the lazy getters.
protocol = parsed.protocol;

if (protocol === 'data:' &&
parsedParentURL.protocol !== 'file:' &&
experimentalNetworkImports) {
throw new ERR_NETWORK_IMPORT_DISALLOWED(
specifier,
parsedParentURL,
'import data: from a non file: is not allowed',
);
}
if (protocol === 'data:' ||
(experimentalNetworkImports &&
(
protocol === 'https:' ||
protocol === 'http:'
)
)
) {
if (protocol === 'data:' ) {

Check failure on line 1013 in lib/internal/modules/esm/resolve.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

There should be no space before this paren
return { __proto__: null, url: parsed.href };
}
}
// There are multiple deep branches that can either throw or return; instead
// of duplicating that deeply nested logic for the possible returns, DRY and
// check for a return. This seems the least gnarly.
const maybeReturn = checkIfDisallowedImport(
specifier,
parsed,
parsedParentURL,
);

if (maybeReturn) { return maybeReturn; }

// This must come after checkIfDisallowedImport
protocol ??= parsed?.protocol;
Expand Down
4 changes: 0 additions & 4 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -435,10 +435,6 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
kAllowedInEnvvar);
AddAlias("--loader", "--experimental-loader");
AddOption("--experimental-modules", "", NoOp{}, kAllowedInEnvvar);
AddOption("--experimental-network-imports",
"experimental https: support for the ES Module loader",
&EnvironmentOptions::experimental_https_modules,
kAllowedInEnvvar);
AddOption("--experimental-wasm-modules",
"experimental ES Module support for webassembly modules",
&EnvironmentOptions::experimental_wasm_modules,
Expand Down
1 change: 0 additions & 1 deletion src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@ class EnvironmentOptions : public Options {
std::string localstorage_file;
bool experimental_global_navigator = true;
bool experimental_global_web_crypto = true;
bool experimental_https_modules = false;
bool experimental_wasm_modules = false;
bool experimental_import_meta_resolve = false;
std::string input_type; // Value of --input-type
Expand Down
1 change: 0 additions & 1 deletion test/fixtures/es-modules/network-import.mjs

This file was deleted.

0 comments on commit d76edeb

Please sign in to comment.