Improve assertion validation and errors, add tests

nodejs · Oct 14, 2021 · 10ba639 · 10ba639
1 parent bd7c391
commit 10ba639
Show file tree

Hide file tree

Showing 8 changed files with 166 additions and 67 deletions.
diff --git a/lib/internal/errors.js b/lib/internal/errors.js
@@ -1083,13 +1083,13 @@ E('ERR_HTTP_SOCKET_ENCODING',
 E('ERR_HTTP_TRAILER_INVALID',
   'Trailers are invalid with this transfer encoding', Error);
 E('ERR_ILLEGAL_CONSTRUCTOR', 'Illegal constructor', TypeError);
-E('ERR_IMPORT_ASSERTION_DISALLOWED',
-  'Import assertions are not allowed for modules of format "%s"',
-  SyntaxError);
-E('ERR_IMPORT_ASSERTION_INVALID',
-  'Modules of format "%s" need an import assertion %s of "%s"', TypeError);
-E('ERR_IMPORT_ASSERTION_MISSING',
-  'Modules of format "%s" need an import assertion %s of "%s"', SyntaxError);
+E('ERR_IMPORT_ASSERTION_TYPE_FAILED',
+  'Modules of format "%s" need an import assertion type "%s"', TypeError);
+E('ERR_IMPORT_ASSERTION_TYPE_MISSING',
+  'Modules of format "%s" need an import assertion type "%s"', SyntaxError);
+E('ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED',
+  'Modules of format "%s" do not support import assertion type "%s"',
+  TypeError);
 E('ERR_INCOMPATIBLE_OPTION_PAIR',
   'Option "%s" cannot be used in combination with option "%s"', TypeError);
 E('ERR_INPUT_TYPE_NOT_ALLOWED', '--input-type can only be used with string ' +

diff --git a/lib/internal/modules/esm/assert.js b/lib/internal/modules/esm/assert.js
@@ -6,12 +6,17 @@ const {
 } = primordials;
 
 const {
-  ERR_IMPORT_ASSERTION_DISALLOWED,
-  ERR_IMPORT_ASSERTION_INVALID,
-  ERR_IMPORT_ASSERTION_MISSING,
-  ERR_UNKNOWN_MODULE_FORMAT,
+  ERR_IMPORT_ASSERTION_TYPE_FAILED,
+  ERR_IMPORT_ASSERTION_TYPE_MISSING,
+  ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED,
 } = require('internal/errors').codes;
 
+/**
+ * Define a map of module formats to import assertion types (the value of `type` in `assert { type: 'json' }`).
+ * A value of `false` means that the presence of an import assertion type for this format should throw an error.
+ * This matches browser behavior, where `import('data:text/javascript,export{}', { assert: { type: 'javascript' } })` throws.
+ * @type {Map<string, string | false}
+ */
 const formatTypeMap = new SafeMap([
   ['builtin', false],
   ['commonjs', false],
@@ -22,26 +27,24 @@ const formatTypeMap = new SafeMap([
 
 
 function validateAssertions(format, importAssertions) {
-  let validType;
-  try {
-    validType = formatTypeMap.get(format);
-  } catch {
-    throw new ERR_UNKNOWN_MODULE_FORMAT(format);
-  }
+  const validType = formatTypeMap.get(format);
 
-  if (validType === false) {
+  if (validType === undefined) {
+    // Ignore assertions for module types we don’t recognize, to allow new formats in the future
+    return true;
+  } else if (validType === false) {
     if (!importAssertions ||
         !ObjectPrototypeHasOwnProperty(importAssertions, 'type')) {
       return true;
     }
-    throw new ERR_IMPORT_ASSERTION_DISALLOWED(format);
+    throw new ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED(format, validType);
   } else {
     if (validType === importAssertions?.type) {
       return true;
     } else if (!ObjectPrototypeHasOwnProperty(importAssertions, 'type')) {
-      throw new ERR_IMPORT_ASSERTION_MISSING(format, 'type', validType);
+      throw new ERR_IMPORT_ASSERTION_TYPE_MISSING(format, validType);
     } else {
-      throw new ERR_IMPORT_ASSERTION_INVALID(format, 'type', validType);
+      throw new ERR_IMPORT_ASSERTION_TYPE_FAILED(format, validType);
     }
   }
 }

diff --git a/test/es-module/test-esm-dynamic-import-assertion.js b/test/es-module/test-esm-dynamic-import-assertion.js
@@ -1,31 +1,9 @@
 // Flags: --experimental-json-modules
 'use strict';
 const common = require('../common');
-const { rejects, strictEqual } = require('assert');
-
-const jsModuleDataUrl = 'data:text/javascript,export{}';
+const { strictEqual } = require('assert');
 
 async function test() {
-  await rejects(
-    import(`data:text/javascript,import${JSON.stringify(jsModuleDataUrl)}assert{type:"json"}`),
-    { code: 'ERR_IMPORT_ASSERTION_DISALLOWED' }
-  );
-
-  await rejects(
-    import(jsModuleDataUrl, { assert: { type: 'json' } }),
-    { code: 'ERR_IMPORT_ASSERTION_DISALLOWED' }
-  );
-
-  await rejects(
-    import('data:text/javascript,', { assert: { type: 'unsupported' } }),
-    { code: 'ERR_IMPORT_ASSERTION_DISALLOWED' }
-  );
-
-  await rejects(
-    import('data:text/javascript,', { assert: { type: 'undefined' } }),
-    { code: 'ERR_IMPORT_ASSERTION_DISALLOWED' }
-  );
-
   {
     const results = await Promise.allSettled([
       import('../fixtures/empty.js', { assert: { type: 'json' } }),
@@ -45,6 +23,26 @@ async function test() {
     strictEqual(results[0].status, 'fulfilled');
     strictEqual(results[1].status, 'rejected');
   }
+
+  {
+    const results = await Promise.allSettled([
+      import('../fixtures/empty.json', { assert: { type: 'json' } }),
+      import('../fixtures/empty.json'),
+    ]);
+
+    strictEqual(results[0].status, 'fulfilled');
+    strictEqual(results[1].status, 'rejected');
+  }
+
+  {
+    const results = await Promise.allSettled([
+      import('../fixtures/empty.json'),
+      import('../fixtures/empty.json', { assert: { type: 'json' } }),
+    ]);
+
+    strictEqual(results[0].status, 'rejected');
+    strictEqual(results[1].status, 'fulfilled');
+  }
 }
 
 test().then(common.mustCall());
diff --git a/test/es-module/test-esm-dynamic-import-assertion.mjs b/test/es-module/test-esm-dynamic-import-assertion.mjs
@@ -1,28 +1,6 @@
 // Flags: --experimental-json-modules
 import '../common/index.mjs';
-import { rejects, strictEqual } from 'assert';
-
-const jsModuleDataUrl = 'data:text/javascript,export{}';
-
-await rejects(
-  import(`data:text/javascript,import${JSON.stringify(jsModuleDataUrl)}assert{type:"json"}`),
-  { code: 'ERR_IMPORT_ASSERTION_DISALLOWED' }
-);
-
-await rejects(
-  import(jsModuleDataUrl, { assert: { type: 'json' } }),
-  { code: 'ERR_IMPORT_ASSERTION_DISALLOWED' }
-);
-
-await rejects(
-  import(import.meta.url, { assert: { type: 'unsupported' } }),
-  { code: 'ERR_IMPORT_ASSERTION_DISALLOWED' }
-);
-
-await rejects(
-  import(import.meta.url, { assert: { type: 'undefined' } }),
-  { code: 'ERR_IMPORT_ASSERTION_DISALLOWED' }
-);
+import { strictEqual } from 'assert';
 
 {
   const results = await Promise.allSettled([
@@ -43,3 +21,23 @@ await rejects(
   strictEqual(results[0].status, 'fulfilled');
   strictEqual(results[1].status, 'rejected');
 }
+
+{
+  const results = await Promise.allSettled([
+    import('../fixtures/empty.json', { assert: { type: 'json' } }),
+    import('../fixtures/empty.json'),
+  ]);
+
+  strictEqual(results[0].status, 'fulfilled');
+  strictEqual(results[1].status, 'rejected');
+}
+
+{
+  const results = await Promise.allSettled([
+    import('../fixtures/empty.json'),
+    import('../fixtures/empty.json', { assert: { type: 'json' } }),
+  ]);
+
+  strictEqual(results[0].status, 'rejected');
+  strictEqual(results[1].status, 'fulfilled');
+}
diff --git a/test/es-module/test-esm-import-assertion.mjs → ...es-module/test-esm-import-assertion-1.mjs b/test/es-module/test-esm-import-assertion.mjs → ...es-module/test-esm-import-assertion-1.mjs
diff --git a/test/es-module/test-esm-import-assertion-errors.js b/test/es-module/test-esm-import-assertion-errors.js
@@ -0,0 +1,52 @@
+// Flags: --experimental-json-modules
+'use strict';
+const common = require('../common');
+const { rejects, strictEqual } = require('assert');
+
+const jsModuleDataUrl = 'data:text/javascript,export{}';
+const jsonModuleDataUrl = 'data:application/json,""';
+
+async function test() {
+  await rejects(
+    // This rejects because of the unsupported MIME type, not because of the unsupported assertion
+    import(`data:text/css,`, { assert: { type: 'css' } }),
+    { code: 'ERR_INVALID_MODULE_SPECIFIER' }
+  )
+
+  await rejects(
+    import(`data:text/javascript,import${JSON.stringify(jsModuleDataUrl)}assert{type:"json"}`),
+    { code: 'ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED' }
+  );
+
+  await rejects(
+    import(jsModuleDataUrl, { assert: { type: 'json' } }),
+    { code: 'ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED' }
+  );
+
+  await rejects(
+    import('data:text/javascript,', { assert: { type: 'unsupported' } }),
+    { code: 'ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED' }
+  );
+
+  await rejects(
+    import(jsonModuleDataUrl),
+    { code: 'ERR_IMPORT_ASSERTION_TYPE_MISSING' }
+  );
+
+  await rejects(
+    import(jsonModuleDataUrl, { assert: {} }),
+    { code: 'ERR_IMPORT_ASSERTION_TYPE_MISSING' }
+  );
+
+  await rejects(
+    import(jsonModuleDataUrl, { assert: { foo: 'bar' } }),
+    { code: 'ERR_IMPORT_ASSERTION_TYPE_MISSING' }
+  );
+
+  await rejects(
+    import(jsonModuleDataUrl, { assert: { type: 'unsupported' }}),
+    { code: 'ERR_IMPORT_ASSERTION_TYPE_FAILED' }
+  );
+}
+
+test().then(common.mustCall());
diff --git a/test/es-module/test-esm-import-assertion-errors.mjs b/test/es-module/test-esm-import-assertion-errors.mjs
@@ -0,0 +1,47 @@
+// Flags: --experimental-json-modules
+import '../common/index.mjs';
+import { rejects } from 'assert';
+
+const jsModuleDataUrl = 'data:text/javascript,export{}';
+const jsonModuleDataUrl = 'data:application/json,""';
+
+await rejects(
+  // This rejects because of the unsupported MIME type, not because of the unsupported assertion
+  import(`data:text/css,`, { assert: { type: 'css' } }),
+  { code: 'ERR_INVALID_MODULE_SPECIFIER' }
+)
+
+await rejects(
+  import(`data:text/javascript,import${JSON.stringify(jsModuleDataUrl)}assert{type:"json"}`),
+  { code: 'ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED' }
+);
+
+await rejects(
+  import(jsModuleDataUrl, { assert: { type: 'json' } }),
+  { code: 'ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED' }
+);
+
+await rejects(
+  import(import.meta.url, { assert: { type: 'unsupported' } }),
+  { code: 'ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED' }
+);
+
+await rejects(
+  import(jsonModuleDataUrl),
+  { code: 'ERR_IMPORT_ASSERTION_TYPE_MISSING' }
+);
+
+await rejects(
+  import(jsonModuleDataUrl, { assert: {} }),
+  { code: 'ERR_IMPORT_ASSERTION_TYPE_MISSING' }
+);
+
+await rejects(
+  import(jsonModuleDataUrl, { assert: { foo: 'bar' } }),
+  { code: 'ERR_IMPORT_ASSERTION_TYPE_MISSING' }
+);
+
+await rejects(
+  import(jsonModuleDataUrl, { assert: { type: 'unsupported' }}),
+  { code: 'ERR_IMPORT_ASSERTION_TYPE_FAILED' }
+);
diff --git a/test/fixtures/empty.json b/test/fixtures/empty.json
@@ -0,0 +1 @@
+{}