Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

regression file:/// import within a data: url no longer works #42860

Closed
Pyrolistical opened this issue Apr 25, 2022 · 2 comments · Fixed by #42881
Closed

regression file:/// import within a data: url no longer works #42860

Pyrolistical opened this issue Apr 25, 2022 · 2 comments · Fixed by #42881

Comments

@Pyrolistical
Copy link

Pyrolistical commented Apr 25, 2022

Version

v18.0.0

Platform

Darwin Ronalds-iMac.local 20.6.0 Darwin Kernel Version 20.6.0: Tue Feb 22 21:10:41 PST 2022; root:xnu-7195.141.26~1/RELEASE_X86_64 x86_64

Subsystem

url or module

What steps will reproduce the bug?

  1. import a data: url that itself javascript that esm imports an absolute file: url

example

import "data:text/javascript,import%20%22file:///Users/rchen/dev/projects/github.com/Pyrolistical/esm-import-file-scheme-bug/inner.js%22;";

How often does it reproduce? Is there a required condition?

always

What is the expected behavior?

inner.js is imported which prints 'inner'

What do you see instead?

crashes with

node:internal/errors:465
    ErrorCaptureStackTrace(err);
    ^

TypeError [ERR_INVALID_URL_SCHEME]: The URL must be of scheme file
    at new NodeError (node:internal/errors:372:5)
    at fileURLToPath (node:internal/url:1484:11)
    at checkIfDisallowedImport (node:internal/modules/esm/resolve:1018:23)
    at defaultResolve (node:internal/modules/esm/resolve:1147:23)
    at ESMLoader.resolve (node:internal/modules/esm/loader:605:30)
    at ESMLoader.getModuleJob (node:internal/modules/esm/loader:318:18)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/module_job:80:40)
    at link (node:internal/modules/esm/module_job:78:36) {
  code: 'ERR_INVALID_URL_SCHEME'
}

Additional information

According to the documentation, esm imports within a data: url must contain absolute paths.

It is unclear what absolute paths means as there is a contradiction in the documentation:

  1. According to the section on file: absolute paths are defined as either starting with /, // or file:///.
  2. But according to esm terminology section, absolute paths start with file:///

I tested both / and file:/// within a data:, neither worked.

I tested data: url by itself with no import and there is no issue there.

This is a regression because when I tested against node 16.14.2, esm import of file: within a data: did work!

Using file: directly does work in v18. The bug only seems to be when file: is used within data:

// works
import "file:///Users/rchen/dev/projects/github.com/Pyrolistical/esm-import-file-scheme-bug/inner.js";

// fails
import "data:text/javascript,import%20%22file:///Users/rchen/dev/projects/github.com/Pyrolistical/esm-import-file-scheme-bug/inner.js%22;";

Repo where I tried various cases. See branches for different versions including v12 and v14.
https://github.com/Pyrolistical/esm-import-file-scheme-bug

@Pyrolistical Pyrolistical changed the title regression file: import within a data: url no longer works regression file: import within a data: url no longer works Apr 25, 2022
@Pyrolistical Pyrolistical changed the title regression file: import within a data: url no longer works regression file:/// import within a data: url no longer works Apr 25, 2022
@Pyrolistical
Copy link
Author

I've bitsected the node versions, the regression was introduced in 17.5.0. it worked fine in 17.4.0

@Pyrolistical
Copy link
Author

Pyrolistical commented Apr 25, 2022

i've traced the code and i believe by the time the url is passed into https://github.com/nodejs/node/blob/master/lib/internal/modules/esm/module_job.js#L56 it is already missing file:///.

aduh95 added a commit to aduh95/node that referenced this issue Apr 27, 2022
nodejs-github-bot pushed a commit that referenced this issue Apr 29, 2022
Fixes: #42860

PR-URL: #42881
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Guy Bedford <[email protected]>
targos pushed a commit that referenced this issue May 2, 2022
Fixes: #42860

PR-URL: #42881
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Guy Bedford <[email protected]>
juanarbol pushed a commit that referenced this issue May 31, 2022
Fixes: #42860

PR-URL: #42881
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Guy Bedford <[email protected]>
danielleadams pushed a commit that referenced this issue Jun 27, 2022
Fixes: #42860

PR-URL: #42881
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Guy Bedford <[email protected]>
targos pushed a commit that referenced this issue Jul 12, 2022
Fixes: #42860

PR-URL: #42881
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Guy Bedford <[email protected]>
targos pushed a commit that referenced this issue Jul 31, 2022
Fixes: #42860

PR-URL: #42881
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Guy Bedford <[email protected]>
guangwong pushed a commit to noslate-project/node that referenced this issue Oct 10, 2022
Fixes: nodejs/node#42860

PR-URL: nodejs/node#42881
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Guy Bedford <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant