Merge pull request #347 from webpack/fix/issue-346

fix imports field

Author: sokra

lib/ExportsFieldPlugin.js

@@ -10,7 +10,7 @@ const DescriptionFileUtils = require("./DescriptionFileUtils");
 const forEachBail = require("./forEachBail");
 const { processExportsField } = require("./util/entrypoints");
 const { parseIdentifier } = require("./util/identifier");
-const { checkExportsFieldTarget } = require("./util/path");
+const { checkImportsExportsFieldTarget } = require("./util/path");
 
 /** @typedef {import("./Resolver")} Resolver */
 /** @typedef {import("./Resolver").ResolveStepHook} ResolveStepHook */
@@ -116,7 +116,7 @@ module.exports = class ExportsFieldPlugin {
 
 						const [relativePath, query, fragment] = parsedIdentifier;
 
-						const error = checkExportsFieldTarget(relativePath);
+						const error = checkImportsExportsFieldTarget(relativePath);
 
 						if (error) {
 							return callback(error);

lib/ImportsFieldPlugin.js

@@ -10,6 +10,7 @@ const DescriptionFileUtils = require("./DescriptionFileUtils");
 const forEachBail = require("./forEachBail");
 const { processImportsField } = require("./util/entrypoints");
 const { parseIdentifier } = require("./util/identifier");
+const { checkImportsExportsFieldTarget } = require("./util/path");
 
 /** @typedef {import("./Resolver")} Resolver */
 /** @typedef {import("./Resolver").ResolveStepHook} ResolveStepHook */
@@ -118,6 +119,12 @@ module.exports = class ImportsFieldPlugin {
 
 						const [path_, query, fragment] = parsedIdentifier;
 
+						const error = checkImportsExportsFieldTarget(path_);
+
+						if (error) {
+							return callback(error);
+						}
+
 						switch (path_.charCodeAt(0)) {
 							// should be relative
 							case dotCode: {

lib/util/path.js

@@ -192,9 +192,9 @@ const cachedJoin = (rootPath, request) => {
 };
 exports.cachedJoin = cachedJoin;
 
-const checkExportsFieldTarget = relativePath => {
-	let lastNonSlashIndex = 2;
-	let slashIndex = relativePath.indexOf("/", 2);
+const checkImportsExportsFieldTarget = relativePath => {
+	let lastNonSlashIndex = 0;
+	let slashIndex = relativePath.indexOf("/", 1);
 	let cd = 0;
 
 	while (slashIndex !== -1) {
@@ -209,6 +209,8 @@ const checkExportsFieldTarget = relativePath => {
 					);
 				break;
 			}
+			case ".":
+				break;
 			default:
 				cd++;
 				break;
@@ -218,4 +220,4 @@ const checkExportsFieldTarget = relativePath => {
 		slashIndex = relativePath.indexOf("/", lastNonSlashIndex);
 	}
 };
-exports.checkExportsFieldTarget = checkExportsFieldTarget;
+exports.checkImportsExportsFieldTarget = checkImportsExportsFieldTarget;

test/exportsField.js

@@ -10,6 +10,7 @@ const CachedInputFileSystem = require("../lib/CachedInputFileSystem");
 const fixture = path.resolve(__dirname, "fixtures", "exports-field");
 const fixture2 = path.resolve(__dirname, "fixtures", "exports-field2");
 const fixture3 = path.resolve(__dirname, "fixtures", "exports-field3");
+const fixture4 = path.resolve(__dirname, "fixtures", "exports-field-error");
 
 describe("Process exports field", function exportsField() {
 	/** @type {Array<{name: string, expect: string[]|Error, suite: [ExportsField, string, string[]]}>} */
@@ -2391,6 +2392,15 @@ describe("ExportsFieldPlugin", () => {
 		});
 	});
 
+	it("should throw error if target is invalid", done => {
+		resolver.resolve({}, fixture4, "exports-field", {}, (err, result) => {
+			if (!err) throw new Error(`expect error, got ${result}`);
+			err.should.be.instanceof(Error);
+			err.message.should.match(/Trying to access out of package scope/);
+			done();
+		});
+	});
+
 	it("throw error if exports field is invalid", done => {
 		resolver.resolve(
 			{},

test/fixtures/exports-field-error/node_modules/exports-field/package.json

@@ -1211,11 +1211,11 @@ describe("ImportsFieldPlugin", () => {
 		);
 	});
 
-	it("should resolve out of package scope", done => {
+	it("should disallow resolve out of package scope", done => {
 		resolver.resolve({}, fixture, "#b", {}, (err, result) => {
-			if (err) return done(err);
-			if (!result) throw new Error("No result");
-			result.should.equal(path.resolve(fixture, "../b.js"));
+			if (!err) throw new Error(`expect error, got ${result}`);
+			err.should.be.instanceof(Error);
+			err.message.should.match(/Trying to access out of package scope/);
 			done();
 		});
 	});
@@ -1229,10 +1229,10 @@ describe("ImportsFieldPlugin", () => {
 			conditionNames: ["webpack"]
 		});
 
-		resolver.resolve({}, fixture, "#b", {}, (err, result) => {
+		resolver.resolve({}, fixture, "#imports-field", {}, (err, result) => {
 			if (err) return done(err);
 			if (!result) throw new Error("No result");
-			result.should.equal(path.resolve(fixture, "../b.js"));
+			result.should.equal(path.resolve(fixture, "./b.js"));
 			done();
 		});
 	});

test/fixtures/exports-field-error/node_modules/pack1/index.js

@@ -0,0 +1,30 @@
+require("should");
+
+const { checkImportsExportsFieldTarget } = require("../lib/util/path");
+
+describe("checkImportsExportsFieldTarget", () => {
+	/**
+	 * @type {string[]}
+	 */
+	const errorCases = [
+		"../a.js",
+		"../",
+		"./a/b/../../../c.js",
+		"./a/b/../../../",
+		"./../../c.js",
+		"./../../",
+		"./a/../b/../../c.js",
+		"./a/../b/../../",
+		"./././../"
+	];
+
+	errorCases.forEach(case_ => {
+		it(case_, done => {
+			const error = checkImportsExportsFieldTarget(case_);
+			if (!error) return done("expect error");
+			error.should.be.instanceof(Error);
+			error.message.should.match(/Trying to access out of package scope/);
+			done();
+		});
+	});
+});