Skip to content

Commit 491b46d

Browse files
guybedfordtargos
authored andcommitted
module: refine package name validation
PR-URL: #28965 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Jan Krems <[email protected]> Reviewed-By: Rich Trott <[email protected]>
1 parent 7b7b8f2 commit 491b46d

File tree

4 files changed

+53
-18
lines changed

4 files changed

+53
-18
lines changed

doc/api/esm.md

+11-9
Original file line numberDiff line numberDiff line change
@@ -700,15 +700,17 @@ _isMain_ is **true** when resolving the Node.js application entry point.
700700
> 1. Throw an _Invalid Specifier_ error.
701701
> 1. Set _packageName_ to the substring of _packageSpecifier_
702702
> until the second _"/"_ separator or the end of the string.
703-
> 1. Let _packageSubpath_ be the substring of _packageSpecifier_ from the
704-
> position at the length of _packageName_ plus one, if any.
705-
> 1. Assert: _packageName_ is a valid package name or scoped package name.
706-
> 1. Assert: _packageSubpath_ is either empty, or a path without a leading
707-
> separator.
703+
> 1. If _packageName_ starts with _"."_ or contains _"\\"_ or _"%"_, then
704+
> 1. Throw an _Invalid Specifier_ error.
705+
> 1. Let _packageSubpath_ be _undefined_.
706+
> 1. If the length of _packageSpecifier_ is greater than the length of
707+
> _packageName_, then
708+
> 1. Set _packageSubpath_ to _"."_ concatenated with the substring of
709+
> _packageSpecifier_ from the position at the length of _packageName_.
708710
> 1. If _packageSubpath_ contains any _"."_ or _".."_ segments or percent
709711
> encoded strings for _"/"_ or _"\\"_ then,
710712
> 1. Throw an _Invalid Specifier_ error.
711-
> 1. If _packageSubpath_ is empty and _packageName_ is a Node.js builtin
713+
> 1. If _packageSubpath_ is _undefined_ and _packageName_ is a Node.js builtin
712714
> module, then
713715
> 1. Return the string _"node:"_ concatenated with _packageSpecifier_.
714716
> 1. While _parentURL_ is not the file system root,
@@ -719,16 +721,16 @@ _isMain_ is **true** when resolving the Node.js application entry point.
719721
> 1. Set _parentURL_ to the parent URL path of _parentURL_.
720722
> 1. Continue the next loop iteration.
721723
> 1. Let _pjson_ be the result of **READ_PACKAGE_JSON**(_packageURL_).
722-
> 1. If _packageSubpath_ is empty, then
724+
> 1. If _packageSubpath_ is _undefined__, then
723725
> 1. Return the result of **PACKAGE_MAIN_RESOLVE**(_packageURL_,
724726
> _pjson_).
725727
> 1. Otherwise,
726728
> 1. If _pjson_ is not **null** and _pjson_ has an _"exports"_ key, then
727729
> 1. Let _exports_ be _pjson.exports_.
728730
> 1. If _exports_ is not **null** or **undefined**, then
729731
> 1. Return **PACKAGE_EXPORTS_RESOLVE**(_packageURL_,
730-
> _packagePath_, _pjson.exports_).
731-
> 1. Return the URL resolution of _packagePath_ in _packageURL_.
732+
> _packageSubpath_, _pjson.exports_).
733+
> 1. Return the URL resolution of _packageSubpath_ in _packageURL_.
732734
> 1. Throw a _Module Not Found_ error.
733735
734736
**PACKAGE_MAIN_RESOLVE**(_packageURL_, _pjson_)

lib/internal/modules/cjs/loader.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,7 @@ function findLongestRegisteredExtension(filename) {
360360
// This only applies to requests of a specific form:
361361
// 1. name/.*
362362
// 2. @scope/name/.*
363-
const EXPORTS_PATTERN = /^((?:@[^./@\\][^/@\\]*\/)?[^@./\\][^/\\]*)(\/.*)$/;
363+
const EXPORTS_PATTERN = /^((?:@[^/\\%]+\/)?[^./\\%][^/\\%]*)(\/.*)$/;
364364
function resolveExports(nmPath, request, absoluteRequest) {
365365
// The implementation's behavior is meant to mirror resolution in ESM.
366366
if (experimentalExports && !absoluteRequest) {

src/module_wrap.cc

+23-8
Original file line numberDiff line numberDiff line change
@@ -867,20 +867,35 @@ Maybe<URL> PackageResolve(Environment* env,
867867
const std::string& specifier,
868868
const URL& base) {
869869
size_t sep_index = specifier.find('/');
870-
if (specifier[0] == '@' && (sep_index == std::string::npos ||
871-
specifier.length() == 0)) {
872-
std::string msg = "Invalid package name '" + specifier +
873-
"' imported from " + base.ToFilePath();
874-
node::THROW_ERR_INVALID_MODULE_SPECIFIER(env, msg.c_str());
875-
return Nothing<URL>();
876-
}
870+
bool valid_package_name = true;
877871
bool scope = false;
878872
if (specifier[0] == '@') {
879873
scope = true;
880-
sep_index = specifier.find('/', sep_index + 1);
874+
if (sep_index == std::string::npos || specifier.length() == 0) {
875+
valid_package_name = false;
876+
} else {
877+
sep_index = specifier.find('/', sep_index + 1);
878+
}
879+
} else if (specifier[0] == '.') {
880+
valid_package_name = false;
881881
}
882882
std::string pkg_name = specifier.substr(0,
883883
sep_index == std::string::npos ? std::string::npos : sep_index);
884+
// Package name cannot have leading . and cannot have percent-encoding or
885+
// separators.
886+
for (size_t i = 0; i < pkg_name.length(); i++) {
887+
char c = pkg_name[i];
888+
if (c == '%' || c == '\\') {
889+
valid_package_name = false;
890+
break;
891+
}
892+
}
893+
if (!valid_package_name) {
894+
std::string msg = "Invalid package name '" + specifier +
895+
"' imported from " + base.ToFilePath();
896+
node::THROW_ERR_INVALID_MODULE_SPECIFIER(env, msg.c_str());
897+
return Nothing<URL>();
898+
}
884899
std::string pkg_subpath;
885900
if ((sep_index == std::string::npos ||
886901
sep_index == specifier.length() - 1)) {

test/es-module/test-esm-pkgname.mjs

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// Flags: --experimental-modules
2+
3+
import { mustCall } from '../common/index.mjs';
4+
import { strictEqual } from 'assert';
5+
6+
import { importFixture } from '../fixtures/pkgexports.mjs';
7+
8+
importFixture('as%2Ff').catch(mustCall((err) => {
9+
strictEqual(err.code, 'ERR_INVALID_MODULE_SPECIFIER');
10+
}));
11+
12+
importFixture('as\\df').catch(mustCall((err) => {
13+
strictEqual(err.code, 'ERR_INVALID_MODULE_SPECIFIER');
14+
}));
15+
16+
importFixture('@as@df').catch(mustCall((err) => {
17+
strictEqual(err.code, 'ERR_INVALID_MODULE_SPECIFIER');
18+
}));

0 commit comments

Comments
 (0)