Skip to content

Commit

Permalink
module: refine package name validation
Browse files Browse the repository at this point in the history
PR-URL: #28965
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Jan Krems <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
  • Loading branch information
guybedford authored and targos committed Aug 19, 2019
1 parent 7b7b8f2 commit 491b46d
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 18 deletions.
20 changes: 11 additions & 9 deletions doc/api/esm.md
Original file line number Diff line number Diff line change
Expand Up @@ -700,15 +700,17 @@ _isMain_ is **true** when resolving the Node.js application entry point.
> 1. Throw an _Invalid Specifier_ error.
> 1. Set _packageName_ to the substring of _packageSpecifier_
> until the second _"/"_ separator or the end of the string.
> 1. Let _packageSubpath_ be the substring of _packageSpecifier_ from the
> position at the length of _packageName_ plus one, if any.
> 1. Assert: _packageName_ is a valid package name or scoped package name.
> 1. Assert: _packageSubpath_ is either empty, or a path without a leading
> separator.
> 1. If _packageName_ starts with _"."_ or contains _"\\"_ or _"%"_, then
> 1. Throw an _Invalid Specifier_ error.
> 1. Let _packageSubpath_ be _undefined_.
> 1. If the length of _packageSpecifier_ is greater than the length of
> _packageName_, then
> 1. Set _packageSubpath_ to _"."_ concatenated with the substring of
> _packageSpecifier_ from the position at the length of _packageName_.
> 1. If _packageSubpath_ contains any _"."_ or _".."_ segments or percent
> encoded strings for _"/"_ or _"\\"_ then,
> 1. Throw an _Invalid Specifier_ error.
> 1. If _packageSubpath_ is empty and _packageName_ is a Node.js builtin
> 1. If _packageSubpath_ is _undefined_ and _packageName_ is a Node.js builtin
> module, then
> 1. Return the string _"node:"_ concatenated with _packageSpecifier_.
> 1. While _parentURL_ is not the file system root,
Expand All @@ -719,16 +721,16 @@ _isMain_ is **true** when resolving the Node.js application entry point.
> 1. Set _parentURL_ to the parent URL path of _parentURL_.
> 1. Continue the next loop iteration.
> 1. Let _pjson_ be the result of **READ_PACKAGE_JSON**(_packageURL_).
> 1. If _packageSubpath_ is empty, then
> 1. If _packageSubpath_ is _undefined__, then
> 1. Return the result of **PACKAGE_MAIN_RESOLVE**(_packageURL_,
> _pjson_).
> 1. Otherwise,
> 1. If _pjson_ is not **null** and _pjson_ has an _"exports"_ key, then
> 1. Let _exports_ be _pjson.exports_.
> 1. If _exports_ is not **null** or **undefined**, then
> 1. Return **PACKAGE_EXPORTS_RESOLVE**(_packageURL_,
> _packagePath_, _pjson.exports_).
> 1. Return the URL resolution of _packagePath_ in _packageURL_.
> _packageSubpath_, _pjson.exports_).
> 1. Return the URL resolution of _packageSubpath_ in _packageURL_.
> 1. Throw a _Module Not Found_ error.
**PACKAGE_MAIN_RESOLVE**(_packageURL_, _pjson_)
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ function findLongestRegisteredExtension(filename) {
// This only applies to requests of a specific form:
// 1. name/.*
// 2. @scope/name/.*
const EXPORTS_PATTERN = /^((?:@[^./@\\][^/@\\]*\/)?[^@./\\][^/\\]*)(\/.*)$/;
const EXPORTS_PATTERN = /^((?:@[^/\\%]+\/)?[^./\\%][^/\\%]*)(\/.*)$/;
function resolveExports(nmPath, request, absoluteRequest) {
// The implementation's behavior is meant to mirror resolution in ESM.
if (experimentalExports && !absoluteRequest) {
Expand Down
31 changes: 23 additions & 8 deletions src/module_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -867,20 +867,35 @@ Maybe<URL> PackageResolve(Environment* env,
const std::string& specifier,
const URL& base) {
size_t sep_index = specifier.find('/');
if (specifier[0] == '@' && (sep_index == std::string::npos ||
specifier.length() == 0)) {
std::string msg = "Invalid package name '" + specifier +
"' imported from " + base.ToFilePath();
node::THROW_ERR_INVALID_MODULE_SPECIFIER(env, msg.c_str());
return Nothing<URL>();
}
bool valid_package_name = true;
bool scope = false;
if (specifier[0] == '@') {
scope = true;
sep_index = specifier.find('/', sep_index + 1);
if (sep_index == std::string::npos || specifier.length() == 0) {
valid_package_name = false;
} else {
sep_index = specifier.find('/', sep_index + 1);
}
} else if (specifier[0] == '.') {
valid_package_name = false;
}
std::string pkg_name = specifier.substr(0,
sep_index == std::string::npos ? std::string::npos : sep_index);
// Package name cannot have leading . and cannot have percent-encoding or
// separators.
for (size_t i = 0; i < pkg_name.length(); i++) {
char c = pkg_name[i];
if (c == '%' || c == '\\') {
valid_package_name = false;
break;
}
}
if (!valid_package_name) {
std::string msg = "Invalid package name '" + specifier +
"' imported from " + base.ToFilePath();
node::THROW_ERR_INVALID_MODULE_SPECIFIER(env, msg.c_str());
return Nothing<URL>();
}
std::string pkg_subpath;
if ((sep_index == std::string::npos ||
sep_index == specifier.length() - 1)) {
Expand Down
18 changes: 18 additions & 0 deletions test/es-module/test-esm-pkgname.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Flags: --experimental-modules

import { mustCall } from '../common/index.mjs';
import { strictEqual } from 'assert';

import { importFixture } from '../fixtures/pkgexports.mjs';

importFixture('as%2Ff').catch(mustCall((err) => {
strictEqual(err.code, 'ERR_INVALID_MODULE_SPECIFIER');
}));

importFixture('as\\df').catch(mustCall((err) => {
strictEqual(err.code, 'ERR_INVALID_MODULE_SPECIFIER');
}));

importFixture('@as@df').catch(mustCall((err) => {
strictEqual(err.code, 'ERR_INVALID_MODULE_SPECIFIER');
}));

0 comments on commit 491b46d

Please sign in to comment.