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

esm module import with data scheme has bug with ternary operator in object #42890

Closed
mghahari opened this issue Apr 28, 2022 · 6 comments · Fixed by #54748
Closed

esm module import with data scheme has bug with ternary operator in object #42890

mghahari opened this issue Apr 28, 2022 · 6 comments · Fixed by #54748
Labels
esm Issues and PRs related to the ECMAScript Modules implementation.

Comments

@mghahari
Copy link

mghahari commented Apr 28, 2022

Version

v16.15.0

Platform

Linux mojighahar 5.8.0-63-generic #71-Ubuntu SMP Tue Jul 13 15:59:12 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

No response

What steps will reproduce the bug?

Running the following codes causes a SyntaxError: Unexpected end of input:

import(`data:text/javascript,
let a = {
  b: 1 == 1 ? "c" : "d";
};
`);

Or more complex:

import(`data:text/javascript,
let a = {
  b: function() { 1 == 1 ? "c" : "d"}
};
`);

But the following code works without error:

import(`data:text/javascript,
let a =  1 == 1 ? "c" : "d"
`);

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

No response

What is the expected behavior?

No response

What do you see instead?

SyntaxError: Unexpected end of input
    at ESMLoader.moduleStrategy (node:internal/modules/esm/translators:117:18)
    at ESMLoader.moduleProvider (node:internal/modules/esm/loader:337:14)
    at async link (node:internal/modules/esm/module_job:70:21)

Additional information

No response

@targos
Copy link
Member

targos commented Apr 28, 2022

/cc @nodejs/modules

It's not the first time an issue like this one is opened.
FWIW, Chrome doesn't error with it.

Here's the annotated stack:

data:text/javascript,let a = {  b: function() { 1 == 1 ?%20%22c%22%20:%20%22d%22}};:1
let a = {  b: function() { 1 == 1


SyntaxError: Unexpected end of input
    at ESMLoader.moduleStrategy (node:internal/modules/esm/translators:117:18)
    at ESMLoader.moduleProvider (node:internal/modules/esm/loader:361:14)
    at async link (node:internal/modules/esm/module_job:70:21)

@targos targos added the esm Issues and PRs related to the ECMAScript Modules implementation. label Apr 28, 2022
@aduh95
Copy link
Contributor

aduh95 commented Apr 28, 2022

FWIW, Firefox and Safari do error with this snippet.

I think this indeed already come up in another issue but I can't find it. From the top of my head, the gist of it was: the spec does not enforce a specific parsing algorithm for import specifiers, Chrome is the odd one out by using a more loose algorithm than others, Node.js is using the WHATWG URL parser and there are no reason to use another one (unless the spec changes). You should always encode the string you want to use in a URL:

import(`data:text/javascript,${encodeURIComponent(`
let a = {
  b: function() { 1 == 1 ? "c" : "d"}
};
`)}`);

// or using Base64
import(`data:text/javascript;base64,${Buffer.from(`
let a = {
  b: function() { 1 == 1 ? "c" : "d"}
};
`).toString('base64')}`);

@GeoffreyBooth
Copy link
Member

I think this indeed already come up in another issue but I can’t find it.

I assume you’re probably remembering #42504.

Should we close this one too per the resolution there? Or perhaps the solution is to try to print a better error message (if that’s even possible, since this seems to error at a parsing level)?

@targos
Copy link
Member

targos commented May 2, 2022

Or perhaps the solution is to try to print a better error message (if that’s even possible, since this seems to error at a parsing level)?

That would be nice, but I don't know if we can tell the difference between a real syntax error in the code and one introduced by URL encoding.

@aduh95
Copy link
Contributor

aduh95 commented May 2, 2022

const match = RegExpPrototypeExec(DATA_URL_PATTERN, parsed.pathname);
if (!match) {
throw new ERR_INVALID_URL(url);
}
const { 1: base64, 2: body } = match;
source = BufferFrom(decodeURIComponent(body), base64 ? 'base64' : 'utf8');

We could try to detect a ternary (if there's a : character that appears in parsed.search), and null coalescing (if parsed.search starts with ??), and display a runtime warning – although there would be some false positives.

@LongTengDao
Copy link
Contributor

But the following code works without error:

import(`data:text/javascript,
let a =  1 == 1 ? "c" : "d"
`);

This not really works, because only let a = 1 == 1 part runs.

FWIW, Firefox and Safari do error with this snippet.

Under my test (at year 2023), Firefox and Safari do same with Chrome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
esm Issues and PRs related to the ECMAScript Modules implementation.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants